Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
Travis failed for GCC 7 Debug Build with "No output has been received in the last 10m0s" when running the new test. Since all other builds succeeded, I think this is transitional error. At least, I am not aware of any loops related to this branch that could end up in an endless loop. @bunnybot merge force -- https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1795871-lua-set_workers. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways. Travis build 4171. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/447042376. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1795871-lua-set_workers. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
Yep! @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1795871-lua-set_workers. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
Worked fine now, thanks! So I guess this can be merged now? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1795871-lua-set_workers. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands has been updated. Commit message changed to: Ignore workers in WorkerQueues when setting the amount of "worker" workers by script and test the funtion. - Fix bug when setting inputs/workers via Lua - Add new attribute 'is_stopped' and new method 'toggle_start_stop()' to LuaProductionSite - New test for the input queues For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1795871-lua-set_workers. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
I have added a function to stop the barracks. The test should be green now. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1795871-lua-set_workers. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
Thanks for adding the test. The code is looking good, but for me the test fails: Error in Lua Coroutine [../src/scripting/lua_errors.cc:22] [string "test/maps/plain.wmf/scripting/test_inputqueue..."]:9: expected '8' but was '7'! I think the problem is that the barracks starts working immediately, consuming one carrier while doing so. Possible fixes would be either to stop the barracks (is that even possible by script?) or connecting it to the headquarter (should stop the barracks since there already are enough soldiers on store). -- https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1795871-lua-set_workers. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
Continuous integration builds have changed state: Travis build 4168. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/446543206. Appveyor build 3966. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1795871_lua_set_workers-3966. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1795871-lua-set_workers. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
Seems indeed as if something failed within Launchpad. For convenience, here is the change: https://bazaar.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/revision/8897 Interesting is only the first block, the rest are added brackets. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
This seems to be some Launchpad error. If you click browse the code it is shown including the diff. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
I don't see any diff here. Did you push your changes? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
Continuous integration builds have changed state: Travis build 4162. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/445335793. Appveyor build 3960. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1795871_lua_set_workers-3960. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
Notabilis has proposed merging lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands. Commit message: Ignore workers in WorkerQueues when setting the amount of "worker" workers by script. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1795871 in widelands: "Game data error: inputqueue: workersqueue: not found" https://bugs.launchpad.net/widelands/+bug/1795871 For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 The set_workers() Lua function removes all workers that aren't explicitly given as parameter. In case of the barracks this leads to the carriers in the input queue being deleted when the trainer is added per script. For once, this isn't intended (I think), but more importantly this leads to a crash later on since the WOrkersQueue still contains the references to the deleted "input" workers. For testing, see the attached bug report. -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands
On second thought, this fix is kind of broken. I haven't tested it but it will most likely fail if a building has the same worker type as an "input" worker and as a "worker" worker. In that case, the input workers will probably be deleted as well. Unfortunately the PlayerImmovable given as a function parameter does not offer a way to differentiate between the two kinds of workers. To fix this, one would probably create a copy of this function specifically for production sites. If this should be done, feel free to say so. Otherwise we could just merge this code (since it fixes an appeared bug) and wait whether it fails again (and maybe add a TODO comment in the code). -- https://code.launchpad.net/~widelands-dev/widelands/bug-1795871-lua-set_workers/+merge/357716 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1795871-lua-set_workers into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp