Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1732765-economy-refactoring into lp:widelands

2018-06-02 Thread ypopezios
> * As Serial is uint32_t so we have 2^32 different Economies, ok > Should we do some stress testing with a Mutiplayergame placing and removing flags and roads like mad? > We could run this some more to see if we ever hit the limit If by testing the limit you mean the number 2^32, this is probab

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

2018-06-04 Thread ypopezios
ypopezios has proposed merging lp:~widelands-dev/widelands/previous_song_exclusion into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1735938 in widelands: "Random song selection should exclude the previous song" https://bugs.lau

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

2018-06-14 Thread ypopezios
I tested it (both paths) and I found it to be a mature improvement. A few points: - I would like a tiny battle to take place, instead of conquering through a mere touch. - The following comment doesn't reflect what actually happens: "remove all workers from p3 to avoid having them wandering ar

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

2018-06-14 Thread ypopezios
My bad. I thought that the code-comment was for both paths. There is no crowd in the cooperative way. So my own comment was about the conquering. I would expect less people in a monastery, especially if some of them get killed during the attack. Or you could make it so as most of them to be fem

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

2018-06-17 Thread ypopezios
ypopezios has proposed merging lp:~widelands-dev/widelands/congestion into lp:widelands. Commit message: For windows testing. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1535115 in widelands: "ai roads getting jammed a lot" https://bugs.lau

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

2018-06-22 Thread ypopezios
The proposal to merge lp:~widelands-dev/widelands/congestion into lp:widelands has been updated. Status: Needs review => Work in progress For more details, see: https://code.launchpad.net/~widelands-dev/widelands/congestion/+merge/348132 -- Your team Widelands Developers is requested to rev

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

2018-06-22 Thread ypopezios
Review: Resubmit -- https://code.launchpad.net/~widelands-dev/widelands/congestion/+merge/348132 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/congestion. ___ Mailing list: https://launchpad.net/~widelands-dev Pos

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

2018-06-22 Thread ypopezios
ypopezios has proposed merging lp:~widelands-dev/widelands/congestion into lp:widelands. Commit message: For windows testing. Requested reviews: ypopezios (ypopezios) Related bugs: Bug #1535115 in widelands: "ai roads getting jammed a lot" https://bugs.launchpad.net/widelands/+b

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

2018-06-22 Thread ypopezios
The proposal to merge lp:~widelands-dev/widelands/congestion into lp:widelands has been updated. Status: Needs review => Superseded For more details, see: https://code.launchpad.net/~widelands-dev/widelands/congestion/+merge/348132 -- Your team Widelands Developers is subscribed to branch

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

2018-06-22 Thread ypopezios
ypopezios has proposed merging lp:~widelands-dev/widelands/congestion into lp:widelands. Commit message: Repeat request for AppVeyor, after bot's failure. Requested reviews: ypopezios (ypopezios) Related bugs: Bug #1535115 in widelands: "ai roads getting jammed a lo

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

2018-06-25 Thread ypopezios
ypopezios has proposed merging lp:~widelands-dev/widelands/congestion into lp:widelands. Commit message: Another effort to get windows builds. Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1535115 in widelands: "ai roads getting jammed a lot&quo

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

2018-06-26 Thread ypopezios
ypopezios has proposed merging lp:~widelands-dev/widelands/congestion2 into lp:widelands. Commit message: Another attempt to get windows builds. Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net/~widelands-dev/widelands/congestion2

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

2018-07-06 Thread ypopezios
That has been waiting for testers (and especially those who doubted every step of its development). Since no other testers appeared, I'd be happy to have this merged, thus forcing everyone to indirectly test it. -- https://code.launchpad.net/~widelands-dev/widelands/road_promotions/+merge/344182

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

2018-07-07 Thread ypopezios
The proposal to merge lp:~widelands-dev/widelands/road_promotions into lp:widelands has been updated. Description changed to: This is supposed to improve how roads get promoted and demoted under all possible scenarios, without surprises or other negative side-effects (basically without oscilla

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

2018-07-07 Thread ypopezios
Description fixed. -- https://code.launchpad.net/~widelands-dev/widelands/road_promotions/+merge/344182 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/road_promotions. ___ Mailing list: https://launchpad.net/~widelan

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

2018-07-10 Thread ypopezios
@klaus Road promotion was just the "appetizer". Congestion here is the "first plate". There will hopefully come a "main plate" as well, which will seriously improve routing performance. There will also come some other improvements related to economy. Concerning the description of this merge, r

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

2018-07-10 Thread ypopezios
I'd rather write a paper on theoretical aspects of Widelands in graph theory... And I'm not interested in publicity, thanks. I'm interested in testing though. You can test current code the way you said (specific congestion scenarios and strange maps) or you can wait to test after I update the co

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

2018-07-10 Thread ypopezios
@klaus I suggest you don't spend your energy with this before I have a review of the code myself, cause I haven't looked at it after the two merges took place. -- https://code.launchpad.net/~widelands-dev/widelands/congestion2/+merge/348525 Your team Widelands Developers is subscribed to branch

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

2018-07-11 Thread ypopezios
Replied to inline comments. Will update the code soon. Diff comments: > === modified file 'src/economy/flag.cc' > --- src/economy/flag.cc 2018-04-07 16:59:00 + > +++ src/economy/flag.cc 2018-06-26 21:40:51 + > @@ -394,101 +406,112 @@ > #define MAX_TRANSFER_PRIORITY 16 > >

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

2018-08-01 Thread ypopezios
Replied to inline comments. Will update the code soon. Diff comments: > === modified file 'src/economy/flag.cc' > --- src/economy/flag.cc 2018-07-11 08:32:54 + > +++ src/economy/flag.cc 2018-07-30 09:36:58 + > @@ -505,14 +576,43 @@ > sizeof(wares_[0]) * (ware_fil

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe into lp:widelands

2018-08-03 Thread ypopezios
I would not create a special extra tribe. I would instead use a nullptr as a default value and have it represent the random tribe by convention and only where it makes sense. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1783878_editor_random_map_tribe/+merge/352038 Your team Widela

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

2018-08-03 Thread ypopezios
ypopezios has proposed merging lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands. Commit message: Get windows builds and ask for testing. Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net/~widelands-dev/widelands

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe into lp:widelands

2018-08-10 Thread ypopezios
My suggestion has been based solely on general programming principles, not on reading the code. I'm not familiar with the relevant files. I had a look and they are a mess (as most of the codebase is). If someone points me to specific places in the code, maybe I could do it. -- https://code.laun

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786597-test-suite into lp:widelands

2018-08-11 Thread ypopezios
Inline comment. Also, if you haven't done it, check this comment: https://bugs.launchpad.net/widelands/+bug/1535115/comments/33 Diff comments: > === modified file 'src/logic/map_objects/tribes/carrier.cc' > --- src/logic/map_objects/tribes/carrier.cc 2018-08-09 11:11:15 + > +++ src/logic/ma

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786597-test-suite into lp:widelands

2018-08-11 Thread ypopezios
In current problematic design, a carrier in an invalid state can easily get stuck, so we will need to test some actual savegames to answer that. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786597-test-suite/+merge/352928 Your team Widelands Developers is subscribed to branch lp:~

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786597-test-suite into lp:widelands

2018-08-12 Thread ypopezios
Review: Needs Information Does this end this branch' reason of existence? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786597-test-suite/+merge/352928 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1786597-test-suite. __

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786597-test-suite into lp:widelands

2018-08-13 Thread ypopezios
Review: Approve As far as it concerns me, this looks ok ;-P -- https://code.launchpad.net/~widelands-dev/widelands/bug-1786597-test-suite/+merge/352928 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1786597-test-suite.

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1786613-500ms-return-skipped into lp:widelands

2018-08-14 Thread ypopezios
In principle, if I was given a range to compromise with, I would go the conservative route and pick the edge of the range which is closer to the previous value. In this case, the given range is 500 to 1000ms, the previous value is 1ms, so I would go with 1000ms. Having said that, in my eyes

Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1783878_editor_random_map_tribe into lp:widelands

2018-08-26 Thread ypopezios
Added inline comment. Diff comments: > > === modified file 'src/ui_fsmenu/launch_spg.cc' > --- src/ui_fsmenu/launch_spg.cc 2018-07-07 20:22:12 + > +++ src/ui_fsmenu/launch_spg.cc 2018-08-26 11:35:09 + > @@ -283,7 +283,14 @@ > Widelands::PlayerNumber const nrplayers = ma

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

2018-08-29 Thread ypopezios
The proposal to merge lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands has been updated. Status: Needs review => Superseded For more details, see: https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/352335 -- Your team Widelands Developers is requeste

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

2018-08-29 Thread ypopezios
ypopezios has proposed merging lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands. Requested reviews: Widelands Developers (widelands-dev) For more details, see: https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019 See description of the branch

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

2018-08-29 Thread ypopezios
AppVeyor seems broken. Otherwise, this branch is now mature and big enough. Not going to add anything more into it, so as to keep it easier for reviewers. If people test it, it can make it into build 20. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019 Your

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

2018-09-10 Thread ypopezios
Added inline comments concerning the NOCOM comments. Diff comments: > > === modified file 'src/economy/portdock.cc' > --- src/economy/portdock.cc 2018-04-16 07:03:12 + > +++ src/economy/portdock.cc 2018-09-04 18:46:42 + > @@ -281,44 +286,70 @@ > } > } > > -void PortDock::upd

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

2018-09-21 Thread ypopezios
The proposal to merge lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands has been updated. Status: Needs review => Superseded For more details, see: https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/354019 -- Your team Widelands Developers is subscrib

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

2018-09-21 Thread ypopezios
ypopezios has proposed merging lp:~widelands-dev/widelands/ship_scheduling_2 into lp:widelands. Commit message: Improved ware/worker routing algorithm for ships and portdocks. Requested reviews: GunChleoc (gunchleoc) For more details, see: https://code.launchpad.net/~widelands-dev/widelands

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

2018-09-26 Thread ypopezios
>From what I understand, the test suite forces the removal of ports and ships >without running clean-up code. That invalidates the state of transports and >breaks the advanced logic. I'm not sure if it is worthy to change the logic, >instead of changing the tests. -- https://code.launchpad.net/

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

2018-09-27 Thread ypopezios
Are you sure that there is proper simulation of user-actions here? Cause I suspect that there is just a magical removal, without calling e.g. sinking function. Of course the code should cover cases of destruction, but how to do that if it is not properly called? The old code was checking every t

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

2018-09-30 Thread ypopezios
This is not perfect, but should be enough for this branch. -- https://code.launchpad.net/~widelands-dev/widelands/ship_scheduling_2/+merge/355510 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ship_scheduling_2. ___

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

2018-10-05 Thread ypopezios
Concerning the specific TODO, the code for invalid destinations pre-existed my involvement and is suspicious enough. I just moved it earlier in the function, in order to catch more cases. I don't know which are those cases (they don't happen in any part of the code that I touched), only that som

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

2018-10-06 Thread ypopezios
Thanks for clarifying the TODO tags. I would expect the creator to be written on the left and the assignee on the right of it. The only way for this branch to slow down the game is if the ports and the ships become hundreds. Even in that case, it should be lighter than the previous code, so I d

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

2018-10-10 Thread ypopezios
You keep calling that a feature, while it is mostly a fix for the long-standing poor scheduling, so technically it still applies despite the feature freeze. Furthermore, the freeze was announced on 2018-09-17, while this branch has been approved on 2018-09-14. The late problems with the test sui

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

2018-10-11 Thread ypopezios
"Widelands is too mature for the player base to find bugs like that acceptable in a release." Unless you have asked the player-base about the matter, the above statement is arbitrary. If you dare, you can conduct a poll on the issue. Projects much bigger and much more serious than Widelands hav

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

2019-04-24 Thread ypopezios
You can end a discussion, but you cannot help your case. Quite expectedly, the "few weeks" of the "release coming shortly" turned out to be more than half a year (and still counting). Congratulations for the upcoming release, and I wish it to really happen and not to be "too little too late". I

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

2019-04-24 Thread ypopezios
It is funny how you still mention "this instance" when the scale of things here is whole months. Of course introducing new bugs doesn't speed-up things, but I don't remember you ever bothering with project's speed to begin with. Still introducing developers can definitely speed-up things. Especi

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

2019-04-25 Thread ypopezios
What has been common practice when Widelands started development, is not the favorite approach anymore and doesn't have to remain the same practice forever. Big fails in prediction is not in the nature of programming, but in the nature of old-fashioned development. Collaborative development is n