widelands-dev team mailing list archive

[Notabilis <notabilis27@xxxxxx>]

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.