← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/map_revision_data into lp:widelands

 

Review: Needs Fixing

I only did a quick review for now - I think this can go into b18, but it needs some cleanups.

I suggest to refactor the map versioning stuff into a class of its own instead of having it all in Map. Map than only contains one member of this new class type. Map already has too many responsabilities. Also, the map could contain N of these to keep all revisions information around if this is desired - the original_author feels a little weird otherwise imho.

- m_map_source_url.clear(); and the line below are not necessary. strings start out empty.

- + * Copyright (C) 2002-2004, 2006-2008, 2010 by the Widelands Development Team
this is outdated. it's 2013.

- +#include <SDL_image.h>
in the new file this is not needed - some other includes might not be needed either. Please check this.

- there is already packet_version, why do you need packet_compatibility?

- widelands_map_version_data_packet.h 
copyright and comments are wrong.


-- 
https://code.launchpad.net/~widelands-dev/widelands/map_revision_data/+merge/184940
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/map_revision_data.


References