Continuous integration builds have changed state:
Travis build 4546. State: failed. Details:
https://travis-ci.org/widelands/widelands/builds/499966184.
Appveyor build 4333. State: success. Details:
https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_frisian_mu
GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1818073-worker
into lp:widelands.
Commit message:
Reinstate mining behavior from r8823.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1818073 in widelands: "changes to worker.cc erroneously reverted"
Review: Needs Fixing
>From my understanding this code results in a penalty of 4 instead of 0 for
>totally depleted or wrong fields.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818073-worker/+merge/363851
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/wi
Review: Approve
@hessenfarmer how do you come to that conclusion? 8*0 is 0 not 4
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818073-worker/+merge/363851
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1818073-worker.
___
Because of the first else if statement, 0 is also less than 4. So we need to
have 0 in an own if statement as it has been in rev8823 or we need the !0 in
every if statement
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818073-worker/+merge/363851
Your team Widelands Developers is s
stupid me...
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818073-worker/+merge/363851
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1818073-worker.
___
Mailing list: https://launchpad.net/~widelan
Took care of it in r9003.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818073-worker/+merge/363851
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1818073-worker.
___
Mailing list: https://launchpad
GunChleoc has proposed merging
lp:~widelands-dev/widelands/bug-1818227-replay-desync into lp:widelands.
Commit message:
Move running of win condition scripts back to Game::init_newgame to fix
desyncing replays.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #18182
Review: Needs Fixing
The fix in r9003 is wrong (see my in-line comment)
Diff comments:
> === modified file 'src/logic/map_objects/tribes/worker.cc'
> --- src/logic/map_objects/tribes/worker.cc2019-02-27 17:19:00 +
> +++ src/logic/map_objects/tribes/worker.cc2019-03-01 16:32:50 +
P.S. the math is exactly the same, but adding 0 is really awkward code. So, my
version was better while it does the exact same thing.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818073-worker/+merge/363851
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/w
We want it to be 0 right? I just reverted to how it was in r8823.
Yours did set it to 4 as hessenfarmer said, 4 ways to fix it:
(1)
if (amount != 0 && amount <= 2) {
totalchance += 6;
} else if (amount != 0 && amount <= 4) {
totalchance += 4;
} else if (amount != 0 && amount <= 6) {
All four are just bad coding style IMHO. Setting something to itself or doing
+= 0 is ugly, and empty conditional blocks and repetitive conditions should be
avoided.
Suggestion:
if (amount > 0) {
if (amount <= 2) {
totalchance += 6;
} else if (amount <= 4) {
totalchance
Review: Needs Fixing
Now we have the problem with 4 Minutes of writing scripting data for big maps
like 'Magic Mountain', again.
Writing Scripting Data ... took 116673ms
.
.
.
Writing Scripting Data ... took 97252ms
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818227-replay-desy
Or we simply add an else see r9004.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818073-worker/+merge/363851
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1818073-worker.
___
Mailing list: https:/
Setting it to a fixed value won't work, because we iterate through all
reachable fields and sum up the individual values.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818073-worker/+merge/363851
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug
Are you talking about r9004? Where do I set what to a fixed value?
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818073-worker/+merge/363851
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1818073-worker.
Okay, today I'm especially stubborn.
if (amount == 0) {
totalchance += 1;
} else if (amount <= 2) {
totalchance += 6;
} else if (amount <= 4) {
totalchance += 4;
} else if (amount <= 6) {
totalchance += 2;
} else {
totalchance += 8 * amount;
}
--
https://code.launchpad.net/~wid
Diff line 18. totalchance is a global counter and must not be reset.
Also, you now add the (8 * amount) only if (amount > 6), whereas it was added
for all values of amount before, changing the result for (0 < amount <= 6).
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818073-worker
I'm terrible sorry. I should have stayed AFK today :-) Did what you suggested
Benedikt.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818073-worker/+merge/363851
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1818073-worker.
Please revert the changes made in r8993, otherwise i fear we will never get a
new official release.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818227-replay-desync/+merge/363871
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1818227-replay
Review: Approve code
No problem. I find small reviews without any issues rather boring ;)
This should work as desired now.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1818073-worker/+merge/363851
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bu
Continuous integration builds have changed state:
Travis build 4550. State: errored. Details:
https://travis-ci.org/widelands/widelands/builds/500454380.
Appveyor build 4337. State: success. Details:
https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_18182
Continuous integration builds have changed state:
Travis build 4553. State: failed. Details:
https://travis-ci.org/widelands/widelands/builds/500508131.
Appveyor build 4340. State: success. Details:
https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_181807
23 matches
Mail list logo
