Merge lp://staging/~widelands-dev/widelands/game_end_summary into lp://staging/widelands

Also please tell me about the code design. Currently player states are stored in the game class. The win scripts report results to the game controller, which builds a EndStatus struct and passes it to the game. Once states for all players have been gathered, game ask the interactive_base to show the window.

I think i will go for the ingame window, code is cleaner

I am sorry that I did not yet voice an opinion. I will definitively review this week.

This is still marked as wip. Is this correct?

Yes, you can still fetch the branch as I only committed locally for now

I'd rather wait till you push a somewhat ready version - otherwise I comment on stuff that you will have changed already in your local branch. I wait till you set this branch to be ready for review again.

It is now ready for review. I left the test win condition just in case, it should be removed before merging. I also updated the others to not popup the win/lost message.

I added a bunch of comments on this.

Concerning the new class, I was thinking of a playersmanager that could, eventually, handle all player related stuff. Currently, a lot is done in editor_gamebase. Should I go toward that or only handle the end status?

Well, if you can find a nice separation of concerns that would be great. Keep in mind that the design has to work for the editor too though. Game and EditorGameBase are two balls of mud right now: they have too many responsibilities - I think the design would already improve if they could be split in a bunch of balls of mud with some less responsibilities and the original class would only contain those. Maybe a clearer design becomes apparent then.

What methods would you pull into the new class?

> Should I go toward that or only handle the end status?
I didn't address this question, sorry. My trigger finger is a little too quick these days :). If you have time to improve the design around those classes a bit that would be awesome - only go for it if you feel that it clearly improves the design though. If you feel like this is too much for now just make one for your new game ending stuff.

I created a playersmanager class and moved all player handling code in there. As there are many code making use of the functions of editor_gamebase, i simply delegated the work for now.

I left a few comments

Some more comments here. I also changed some style and nits around. Also you forgot to check in logic/playersmanager.[h|cc]. I also think the file should be called players_manager.h and the class PlayersManager (though we are not very consistent about this either - we definitely should do a bunch of style cleanups throughout the code base).

I went for an enum for the result (won/lost/resigned).
I also changed the report_result function to only take 3 arguments : player, result, and the info string.
I added some comment to show allowed values for that string, and parsed some of them in the game_summary screen.

lgtm - I merged this minus the test win condition.

Spoke too soon: You still need to check in logic/playersmanager.[h|cc].

and there are a few style problems. Could you run the style checker on this too, please?

I fixed this along with some warnings. Sorry i forgot to add the files

I saw some strings from this had made their way into the translation templates when the catalogues were updated. Was that unintential or because this is right around the corner? Just curious.

I have no idea how could that happen

Doesn't matter any longer :). I merged this now. Thanks for all the work Charly!