widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

I have recently done the same in a branch that isn't quite ready yet: http://bazaar.launchpad.net/~widelands-dev/widelands/bug-1390793/revision/7415

Your function is a lot better. I have delete_tag instead of remove_tag, which has an additional check in it:

void Map::delete_tag(const std::string& tag) {
       if (has_tag(tag)) {
               m_tags.erase(m_tags.find(tag));
        }
}

I have also added some diff comments.

Diff comments:

> 
> === modified file 'src/logic/map.cc'
> --- src/logic/map.cc	2015-02-24 13:51:38 +0000
> +++ src/logic/map.cc	2015-06-30 00:48:12 +0000
> @@ -1992,6 +1996,61 @@
>  }
>  
>  
> +/*
> +===========
> +Map::check_check_seafaring()
> +
> +This function checks if there are two ports that are reachable
> +for each other - then the map is seafaring.
> +=============
> +*/
> +bool Map::check_seafaring() {
> +	Map::PortSpacesSet port_spaces = get_port_spaces();
> +	std::vector<Coords> portdocks;
> +	std::set<Coords, Coords::OrderingFunctor> swim_coords;
> +
> +	for (const Coords& c : port_spaces) {
> +		std::queue<Coords> q_positions;
> +		std::set<Coords, Coords::OrderingFunctor> visited_positions;
> +		FCoords fc = get_fcoords(c);
> +		portdocks = find_portdock(fc);
> +
> +		/* remove the port space if it is not longer valid port space */

I don't think that it is a good idea to remove them here. A map editor might save a work in progress map and then wonder why the port spaces have disappeared.

> +		if ((fc.field->get_caps() & BUILDCAPS_SIZEMASK) != BUILDCAPS_BIG || portdocks.empty()){
> +			set_port_space(c, false);
> +			continue;
> +		}
> +
> +		for (const Coords& portdock: portdocks){

Please add a blank space between ) and { - codecheck doesn't pick these up.

> +			visited_positions.insert(portdock);
> +			q_positions.push(portdock);
> +		}
> +
> +		while (!q_positions.empty()){

Please add a blank space between ) and { - codecheck doesn't pick these up.

> +			const Coords& swim_coord = q_positions.front();
> +			q_positions.pop();
> +			for (uint8_t i = 1; i <= 6; ++i) {
> +				FCoords neighbour;
> +				get_neighbour(get_fcoords(swim_coord), i, &neighbour);
> +				if ((neighbour.field->get_caps() & (MOVECAPS_SWIM|MOVECAPS_WALK)) == MOVECAPS_SWIM){

Please add a blank space between ) and { - codecheck doesn't pick these up. Also, please add blank spaces around |.

> +					if (visited_positions.count(neighbour) == 0){

Please add a blank space between ) and { - codecheck doesn't pick these up.

> +						visited_positions.insert(neighbour);
> +						q_positions.push(neighbour);
> +					}
> +				}
> +			}
> +		}
> +
> +		for (const Coords& swim_coord: visited_positions)
> +			if (swim_coords.count(swim_coord) == 0)
> +				swim_coords.insert(swim_coord);
> +			else
> +				return true;
> +	}
> +	return false;
> +}
> +
> +
>  #define MAX_RADIUS 32
>  MilitaryInfluence Map::calc_influence
>  	(Coords const a, Area<> const area) const


-- 
https://code.launchpad.net/~majcherlk/widelands/seafaring-check/+merge/263192
Your team Widelands Developers is requested to review the proposed merge of lp:~majcherlk/widelands/seafaring-check into lp:widelands.