widelands-dev team mailing list archive

[Arty <diego.scheide@xxxxxx>]

I admit the bitmask stuff is still my default way from before C++11, without having to re-define operators or typecast all the time. But you are right, I should do this properly with enum class now. I'll do that.


As for the code duplication between the game saving and map saving, I feel that this doesn't really belong into the GenericSaveHandler. I wanted to keep it on a separate abstraction level, in particular without relying on any UI stuff or other user interaction. (I am not even sure I should have put the generation of localized messages in there.)

The duplication part you pointed out isn't even that big (and half of it just comments), but there is overall a lot of code duplication between the map saving UI and the game saving UI. I think we should rather try to merge those into a single class. There are of course a few key differences between handling games and maps there, but those could possibly be dealt with via overloading or templates or even just via some parameters (including using lambda functions). Same goes for the loading UI and possibly a few others. I wouldn't mind looking into that, but that would be a somewhat bigger change (and require some more consideration how feasible this even is), so that would rather be something for the build21.
-- 
https://code.launchpad.net/~widelands-dev/widelands/robust-file-saving/+merge/358718
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/robust-file-saving into lp:widelands.