Review: Needs Fixing I only looked at the commit-diffs, but the changes are mostly fine with me, thanks.
However, there are two problems: - Codecheck complains about wrong include order in logic/game.cc - For me, it does not compile. Strangely, Travis seems to be happy with the code. The problem is the undefined variable mso in ai/defaultai_warfare.cc. It has been removed in commit 8154, is there a reason for it? After re-adding the line it compiles fine. A quick test with my locally fixed version did not crash any longer in the menus and the issues from the review are done or have become TODOs. What is a bit strange: The preview-map of savegames and the additional game information are gone. Is this intentional? Oh, and I am not able to start or load a game. It loads fine but the game crashes as soon as the game is displayed. I found the source in AiDnaHandler::fetch_dna() in logic/ai_dna_handler.cc. The path to the file is created by concatenating char pointers since a std::string() cast is missing due to the new char* constants. I haven't checked the other uses of the constants, maybe its happening there, too. Is it possible to make the constants std::strings to avoid this (possible/future) bug? Regarding constants.h: Currently the file is in src/logic/ but also contains constants from, e.g. the ai. Maybe place it directly in src/ ? Related: in src/network/ there is also a constants.h containing the used network port numbers. Shall I move them to our global(?) constants file (I have to do this myself)? -- https://code.launchpad.net/~widelands-dev/widelands/savegame-menu/+merge/322924 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/savegame-menu. _______________________________________________ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
