Merge lp:~widelands-dev/widelands/wood_gnome_infinite_trees into lp:widelands

[SirVer]

When a player is defeated, it is removed from the 'plrs' array by the 'check_player_defeated'. This lead to this players points not being reset on recalculation, ergo on each calculation the player gained the number of trees currently on its terrain as points. A classical CS bug: Cache invalidation errors.

Also: no longer calculate score for a defeated player.

[TiborB (tiborb95)]

The game crashed with:

Fatal exception: [/var/widelands/BZR/bad_cast/src/scripting/lua_errors.cc:22] [string "scripting/win_conditions/wood_gnome.lua"]:79: attempt to perform arithmetic on a nil value (field '?')

review: Needs Fixing

[SirVer (sirver)]

thanks for testing - I made a thinko here. Will look into it tomorrow or on the weekend.

[GunChleoc (gunchleoc)]

Why do you do _plrpoints[k] = nil and not _plrpoints[k] = 0? The crashy line is probably trying to calculate nil + 1.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 1771. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/184368559.
Appveyor build 1609. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_wood_gnome_infinite_trees-1609.

[SirVer (sirver)]

> The crashy line is probably trying to calculate nil + 1.

yes, that is what happens. I forgot that a defeated player could still own land - which opens the possibilities for a real broken situation: if you have no warehouse, the game declares you as defeated - but theoretically you could still own the most trees at 4 hours. In this case, the came would declare a defeated player as the winner. I decided that this makes no sense and now no longer count trees for defeated players.

ptal (please take another look).

review: Needs Resubmitting

[GunChleoc (gunchleoc)]

Code LGTM, not tested.

review: Approve

[bunnybot (widelandsofficial)]

Bunnybot encountered an error while working on this merge proposal:

('The read operation timed out',)

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 1774. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/184461866.
Appveyor build 1612. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_wood_gnome_infinite_trees-1612.

[TiborB (tiborb95)]

Tested. Seems OK now.

review: Approve

[SirVer (sirver)]

@bunnybot merge

> Am 16.12.2016 um 22:37 schrieb TiborB <email address hidden>:
>
> Review: Approve
>
> Tested. Seems OK now.
> --
> https://code.launchpad.net/~widelands-dev/widelands/wood_gnome_infinite_trees/+merge/313397
> You proposed lp:~widelands-dev/widelands/wood_gnome_infinite_trees for merging.

=== modified file 'data/scripting/win_conditions/wood_gnome.lua'
--- data/scripting/win_conditions/wood_gnome.lua	2016-09-17 11:29:34 +0000
+++ data/scripting/win_conditions/wood_gnome.lua	2016-12-16 06:32:50 +0000
@@ -57,14 +57,20 @@
          return _plrpoints
       end
 
-      -- init the playerpoints for each player
+      -- clear out the table. We count afresh.
+      for k,v in pairs(_plrpoints) do 
+         _plrpoints[k] = 0
+      end
+      
+      -- Insert all players who are still in the game.
       for idx,plr in ipairs(plrs) do
          _plrpoints[plr.number] = 0
       end
+      
       for idf,f in ipairs(fields) do
          -- check if field is owned by a player
          local owner = f.owner
-         if owner then
+         if owner and not owner.defeated then
             owner = owner.number
             -- check if field has an immovable
             local imm = f.immovable