[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-26 Thread Toni Förster
Toni Förster has proposed merging lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands. Commit message: - fix broadcast() - use broadcast() in _send_state() instead of send_message() Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1721

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-26 Thread Toni Förster
The proposal to merge lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands has been updated. Commit message changed to: - fix broadcast() - fix send_message() for territorial For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-des

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-26 Thread bunnybot
Continuous integration builds have changed state: Travis build 4415. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/484807553. Appveyor build 4205. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_181158

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-27 Thread bunnybot
Continuous integration builds have changed state: Travis build 4418. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/485117314. Appveyor build 4208. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_18115

[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-02-02 Thread noreply
The proposal to merge lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-desync-with-territorial/+merge/362272 -- Your team

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-26 Thread kaputtnik
Review: Needs Fixing Please pay attention for messages where each player get a different message (e.g. different land size). In think this is the case in territorial_time and territorial_lord. Using broadcast() will send the same message to all players. We should also take care of the RST comm

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-26 Thread Toni Förster
Are you sure. I thought that the status messages are the same for all players. A message usually contains the sizes for all players. But I could be wrong. If you would take care of the RST that would be nice. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-desync-with-territo

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-26 Thread kaputtnik
Yes, i am, just look at the code. E.g. for territorial_lord they are: For the currently winning player: "You own more than half of the map’s area. Keep it for %i more minute to win the game." For the currently loosing player: "Playername owns more than half of the map’s area. You’ve still got %

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-26 Thread Toni Förster
You're right... fixed it. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-desync-with-territorial/+merge/362272 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-26 Thread Toni Förster
Review: Resubmit -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-desync-with-territorial/+merge/362272 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial. ___ Mail

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-27 Thread kaputtnik
Review: Approve code review LGTM :-) Approve this, but i want to have GunChleoc to take a look at my glorious English wording ;) Of course some one else with knowledge of English can do this also. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-desync-with-territorial/+merge

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-27 Thread kaputtnik
I have removed the link to bug 1721126, because of https://bugs.launchpad.net/widelands/+bug/1721126/comments/27 -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-desync-with-territorial/+merge/362272 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widel

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-27 Thread Toni Förster
Review: Approve proof reading -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-desync-with-territorial/+merge/362272 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial. ___

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-27 Thread Notabilis
I haven't tested it but the code is looking good, thanks. Regarding the documentation: Please update the documentation of win_condition_functions::broadcast() since it no longer waits for roadbuilding. Also, could you add a comment in coroutines.lua (similar to ui.lua)? In multiplayer, these met

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-27 Thread kaputtnik
Oh, thanks for the hint regarding broadcast(), just forgot it. About coroutines.lua: I do not know which function is NOT used for all players at the same time. I guess those functions are the functions called when a particular user action is triggered, and i think that are all the functions in

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-30 Thread Klaus Halfmann
Review: Approve compile, testplay We played this to the End on a 3vs3 Game on some large Map and had no problems whatsover. This way I finally learned Tonis game-name. Traviss had an error with apt.llvm.org port one build only. @bunnbot merge -- https://code.launchpad.net/~widelands-dev/widel

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-30 Thread Notabilis
Sorry for the late response. Currently, these functions are indeed called for all players in parallel within the win condition scripts. But if this would change for whichever reason, the game would desync. A point where this happened are the functions within ui.lua which call the sleep() functio

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-31 Thread kaputtnik
> But if this would change for whichever reason, the game would desync. Thats the reason why i think this is just a workaround. A proper fix would be to make the problematic functions, like wait_for_roadbuilding(), work for multiplayer games too. Don't know if this is feasible though. Your sugg

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-31 Thread Toni Förster
>> Make sure they are called for none or all players in parallel, >I think most people (including me) do not now how to achieve that :-S Maybe an example would help? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-desync-with-territorial/+merge/362272 Your team Widelands Deve

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-31 Thread Toni Förster
I don't think we can fix wait_for_roadbuilding(). What we could do is either remove it from send_message() so that it looks like this: function send_message(player, title, body, parameters) player:send_message(title, body, parameters) end Or, every send_message() call needs to be called from

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-31 Thread Toni Förster
Fourth option would be to check for multiplayer. Here is some pseudo-code function wait_for_roadbuilding() if not multiplayer then _await_animation() while (wl.ui.MapView().is_building_road) do sleep(2000) end end end Is there a function to check for multiplayer? -- https://code.lau

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-31 Thread Toni Förster
I have an idea but no clue how to implement this: the lua part: function wait_for_roadbuilding() local game_type = wl.Game().game_type if game_type = singleplayer then _await_animation() while (wl.ui.MapView().is_building_road) do sleep(2000) end end end The c++ part I got from src/w

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-31 Thread Notabilis
You are right, my note is most likely to abstract. If I wouldn't be knowing the code I possibly wouldn't understand it either. I like the idea of checking for single-/multiplayer game. Since I wasn't able to find a lua method for this, I guess someone has to add one for it. I can take a look at

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-01-31 Thread Toni Förster
>I can take a look at it in the next days. This would be great! -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-desync-with-territorial/+merge/362272 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial. __

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-02-01 Thread kaputtnik
Klaus, it is called bunnYbot :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-desync-with-territorial/+merge/362272 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial. __

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-02-01 Thread Toni Förster
It won't merge since travis through an error. But if notabilis implements the single- multiplayer?-method we wouldn't need this branch here... -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-desync-with-territorial/+merge/362272 Your team Widelands Developers is subscribed to b

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-02-02 Thread kaputtnik
Looks like bunnybot is in winter sleep ... The travis error is about curl, and i would wonder if the changes would affect building widelands at all, since only some lua code is changed. > But if notabilis implements the single- multiplayer?-method we wouldn't need > this branch here... I think

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-02-02 Thread kaputtnik
This is merged into trunk now. New bug: https://bugs.launchpad.net/widelands/+bug/1814372 -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-desync-with-territorial/+merge/362272 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1811583-desync

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1811583-desync-with-territorial into lp:widelands

2019-02-02 Thread Notabilis
A branch with the requested lua method is up: https://code.launchpad.net/~widelands-dev/widelands/bug-1814372-lua-gametype/+merge/362638 -- https://code.launchpad.net/~widelands-dev/widelands/bug-1811583-desync-with-territorial/+merge/362272 Your team Widelands Developers is subscribed to branch