Code review comment for lp:~stdh/widelands/rewrite_wincondition

> *What does wc_version mean & should I provide compatibility code for old savegames?
I have no idea why it is there actually, it is nowhere used in the code so you should be able to just kill it.

> *The code to check for defeated players was broken in that it only checks once. I didn't touch it, some other win conditions use the same faulty code and it should be adressed more globally, I feel.
I agree - are you doing this? :)

> Bonus question: should other players be noticed when one of their friends/enemies is defeated?
This is certainly a design question - I do not think it is necessary. The players can chat and communicate this and the messages are more about you.

> *I noticed that "not rawequal(wl.Game().players[2],wl.Game().players[2])", while the two are considered equal by the "==" operator. This means a Player cannot be used as a key in a table, which is slightly annoying and should be mentioned in the docs if not outright fixed.
How can this be fixed though? There is a new Lua table with the same metatable generated for each time you access the player. I have no idea what property they must implement to compare equal.

> *After I found this out, I used wl.game.Player.name as a key. I hope that's a unique property...
We do not explicitly enforce this imho. Player.number is absolutely unique though, you should use this. Note that they are not necessarily monotonic (i.e. player 3 could not be in the game while player 4 is).

> *Code style and formatting OK? A quick search didn't turn up information about any preference in the project.
We have none for Lua right now. This should be fixed though - but I would much rather piggy back on some style guide that a third party owns. Do you have any suggestions?

> I pasted the GPL2+ boilerplate at
> the top, but I haven't completed the year range(s).
Please do so.

> Also, do I have to officialy claim that the code I contribute is licensed to
> Widelands, and hence the world, under GPL2+? Or is this the implicit
> understanding?
I am not a lawyer. We do it rather more often than not often enough, so I would appreciate if you could keep it around.

Would you mind open a bug report for this Lua restart bug you see? It seems rather serious and should be tracked down?

Now for the review:
- array_ind_max -- please do not use abbreviations.
- +local wc_version = 3 --stdh: increased by one, right? -- I much rather see it removed. Could you try that?
- can walkable fields be swimmable? -- I do not think so.
- can swimmable fields have immovables? -- I do not think so either, but I am unsure with the portdock.
- --stdh: is this f sometimes nil? -- No, the functions throws an error if you give an invalid coordinate.
- --stdh: points are awarded for player land, disregarding teams. -- I think so.
- --stdh: TODO: fix 'cause only runs once -- What do you mean by this? there should be an ifinite loop in the function. and this coroutine should be saved and run again after load.
- --stdh: why is that important/pertinent? I do not think it is important at all, just so it is in the same ballpark from sampling time.

Otherwise this looks pretty good to me. Thanks for working on this.