Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-12-05 Thread GunChleoc
The reason that the warehouse statistics don't list them is that people might try to set a policy on them. Players might still be interested in the inventory though. -- https://code.launchpad.net/~widelands-dev/widelands/casern_workersqueue/+merge/309763 Your team Widelands Developers is request

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-12-04 Thread Notabilis
Good question! In the case of workers, this is not required. Now that you are mentioning it I remembered (and testing confirmed) that this is dealt with somewhere else. When destroying a building (and its queues) wares are lost, so we have to remove them from the economy. On the other hand, work

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-12-04 Thread GunChleoc
This is WaresQueue does in the cleanup: if (filled_ && owner_.get_economy()) owner_.get_economy()->remove_wares(index_, filled_); filled_ = 0; etc... I guess we should to the same for the workers queue now, since it's now more like the waresqueue and not like th

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-12-04 Thread Notabilis
Seems like the cleanup-bug is fixed. The problem was that the WorkersQueue was free-ing the stored workers on cleanup while the building was also free-ing them. The fix is to simply drop the pointers to the workers when removing the queue and let the building handle them. However, I don't reall

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-12-04 Thread bunnybot
Continuous integration builds have changed state: Travis build 1697. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/180993235. Appveyor build 1537. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_casern_wor

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-28 Thread bunnybot
Continuous integration builds have changed state: Travis build 1660. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/179421689. Appveyor build 1500. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_casern_wor

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-28 Thread bunnybot
Bunnybot encountered an error while working on this merge proposal: ('The read operation timed out',) -- https://code.launchpad.net/~widelands-dev/widelands/casern_workersqueue/+merge/309763 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/c

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-28 Thread Notabilis
Thank you both for looking at this. When looking sooner in the travis log I found these lines: test/maps/plain.wmf/scripting/test_casern.lua ... Running Widelands ... *** Error in `build/src/widelands': double free or corruption (!prev): 0x0670d1e0 *** FAIL So you are right SirVer: A

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-28 Thread SirVer
Gun pinged me to take a look at this. First impression: Looking over the travis output there is clearly a bug in the code somewhere. It is flaky as it sometimes succeeds and sometimes fails. There is no correlation to the compiler (gcc/clang both are flaky) or the type of build (debug/release).

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-28 Thread GunChleoc
As far as I understand the log output of the failed tests and the Python script, widelands itself exits with a return code != 0 - maybe a new assert in the code itself somewhere is causing this, rather than the test code? -- https://code.launchpad.net/~widelands-dev/widelands/casern_workersqueue

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-26 Thread bunnybot
Continuous integration builds have changed state: Travis build 1648. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/178540332. Appveyor build 1487. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_casern_wor

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-26 Thread bunnybot
Bunnybot encountered an error while working on this merge proposal: ('The read operation timed out',) -- https://code.launchpad.net/~widelands-dev/widelands/casern_workersqueue/+merge/309763 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/c

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-23 Thread bunnybot
Continuous integration builds have changed state: Travis build 1645. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/178415914. Appveyor build 1483. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_casern_wor

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-23 Thread Notabilis
Oh, forgot a comment: Priorities for WorkerQueues are and will stay disabled. I am not completely sure what priorities are doing, but they only seem to influence how likely it is that a carrier picks up a ware on a flag. Obviously, this does not make sense for workers. -- https://code.launchpad

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-23 Thread Notabilis
Replacing calls to WaresQueue with calls to the new InputQueue is done in another branch: https://code.launchpad.net/~widelands-dev/widelands/refactoring-input-queue It can be merged either in this branch or into trunk after this branch is merged. Adding a workersQueue for the builder on expedit

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-15 Thread bunnybot
Continuous integration builds have changed state: Travis build 1611. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/175973829. Appveyor build 1449. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_casern_wor

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-14 Thread bunnybot
Continuous integration builds have changed state: Travis build 1608. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/175798398. Appveyor build 1446. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_casern_wor

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-14 Thread GunChleoc
I am always in favour of removing code duplication wherever possible, so please go ahead if you want. If it's older code, you can also do that in a follow-up branch to keep the current diff smaller. Regarding the builder, we actually have an open bug for that, so yes, please :) https://bugs.lau

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-14 Thread bunnybot
Continuous integration builds have changed state: Travis build 1604. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/175631314. Appveyor build 1442. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_casern_work

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-13 Thread bunnybot
Continuous integration builds have changed state: Travis build 1603. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/175565469. Appveyor build 1441. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_casern_wor

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-13 Thread Notabilis
Thanks for the answers earlier. Wares- and WorkerQueues now have a common interface so they can be used interchangeable. Note that the code is not really tested yet. Both are using the same gui class for display, list on the first tab. Support for priorities on the graphical worker queues is dis

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-12 Thread bunnybot
Continuous integration builds have changed state: Travis build 1597. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/175308095. Appveyor build 1435. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_casern_wo

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-12 Thread GunChleoc
Oops - definitely needs increasing to 6. -- https://code.launchpad.net/~widelands-dev/widelands/casern_workersqueue/+merge/309763 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands. ___

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-12 Thread Notabilis
GunChleoc, can you take a look at you "versioning" changes in map_buildingdata_packet.cc, please? The current version does not pass the regression tests. The problem is the line: if (packet_version > 5) { Since kCurrentPacketVersionProductionsite is still set to 5, the code differs on read

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-11 Thread GunChleoc
Yep, I liked the beer too - it's a balancing thing tough. 2) A flag sounds good. The reason for the inconsistency is that until now, this was used to fill working positions, where allowing higher ranking workers is important. 3) It can be resolved like this: bool found = false; for (co

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-09 Thread Notabilis
Nooo, not the beer! I liked the beer! :( But thanks for the review. The small stuff (renaming, etc.) is done, what is left are the bigger changes. Open problems in no particular order: 1) Making WorkersQueue more similar to WaresQueue and replacing the user interface should be no problem. I wil

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-09 Thread GunChleoc
Sorry, overlooked your comment - think now that the UI change should go in before the merge, because the code changes for that will be rather big, and we should get a superclass for wares and workers queues. For the documentation, see https://bazaar.launchpad.net/~widelands-dev/widelands/trunk

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-08 Thread GunChleoc
First round of code review is done - see my last commit. I will have to look into your economy questions - I'm not really familiar with the economy code myself yet. Great contribution overall! I have mostly some minors nits and refactoring ideas - feel free to drop any of those if you don't agr

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-08 Thread Notabilis
I split up the documentation and updated some references. Now I am only wondering: Is there a script which extracts this documentation somewhere? I only found some generic documentation but no list of provided Lua classes or something like that. What else is to do in this branch? Will the UI be

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-08 Thread SirVer
The documentation is not dead. Various classes implement this interface and refer to this as documentation. We can delete this, but then we have to add the same documentation to all classes that implement the semantics. As long as they do not diverge, I think having them only documented once i

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-08 Thread GunChleoc
I think we should delete the documentation then - no need to have dead code documented, it is confusing. -- https://code.launchpad.net/~widelands-dev/widelands/casern_workersqueue/+merge/309763 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/wideland

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-07 Thread SirVer
> Is this something which really exists in the code or documentation-only to > avoid writing the same stuff over and over again? The latter. I think it used to exist, but now it is just an interface without any code associated with it. Splitting up sounds fine to me. I do not think we can give

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-07 Thread Notabilis
Thanks Travis but I already knew that the test were broken. They should be fixed now. However, I have a problem with the scripting documentation. What is the "HasWares" interface which is described in lua_map.cc? Is this something which really exists in the code or documentation-only to avoid w

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-07 Thread bunnybot
Continuous integration builds have changed state: Travis build 1567. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/172859367. Appveyor build 1406. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_casern_wor

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-07 Thread bunnybot
Bunnybot encountered an error while working on this merge proposal: ('The read operation timed out',) -- https://code.launchpad.net/~widelands-dev/widelands/casern_workersqueue/+merge/309763 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/c

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-03 Thread GunChleoc
I gave it a quick test - seems to be working :) I am not sure about the user interface though - I have posted on the forum for user feedback: https://wl.widelands.org/forum/topic/974/?page=3#post-18504 Also, because we renamed a function in the Lua interface, the test suite is broken. Document

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/casern_workersqueue into lp:widelands

2016-11-02 Thread bunnybot
Continuous integration builds have changed state: Travis build 1565. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/172775329. Appveyor build 1404. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_casern_wor