widelands-dev team mailing list archive

[TiborB <tiborb95@xxxxxxxxx>]

Maybe you could rename swim_coords and visited_positions to more obvious names. Swim coords are coords reachable from previous ports and visited_positions are the one reachable from current port.
It is not easy to come up with short names, but something like this: previous_ports_reachable and current_port_reachable? I know they are long and dont sound good, but...

+ 1 comment in diff

Diff comments:

> 
> === modified file 'src/logic/map.cc'
> --- src/logic/map.cc	2017-11-05 19:59:33 +0000
> +++ src/logic/map.cc	2017-12-07 07:03:28 +0000
> @@ -1950,46 +1959,29 @@
>  			check_neighbour_heights(n[i], area);
>  }
>  
> -/*
> -===========
> -Map::allows_seafaring()
> -
> -This function checks if there are two ports that are reachable
> -for each other - then the map is seafaring.
> -=============
> -*/
> -bool Map::allows_seafaring() {
> -	Map::PortSpacesSet port_spaces = get_port_spaces();
> -	std::vector<Coords> portdocks;
> +bool Map::allows_seafaring() const {
>  	std::set<Coords> swim_coords;
>  
> -	for (const Coords& c : port_spaces) {
> -		std::queue<Coords> q_positions;
> +	for (const Coords& c : get_port_spaces()) {
> +		std::queue<Coords> positions_to_check;
>  		std::set<Coords> visited_positions;
>  		FCoords fc = get_fcoords(c);
> -		portdocks = find_portdock(fc);
> -
> -		/* remove the port space if it is not longer valid port space */
> -		if ((fc.field->get_caps() & BUILDCAPS_SIZEMASK) != BUILDCAPS_BIG || portdocks.empty()) {
> -			set_port_space(c, false);
> -			continue;
> -		}
> -
> -		for (const Coords& portdock : portdocks) {
> +
> +		for (const Coords& portdock : find_portdock(fc)) {
>  			visited_positions.insert(portdock);
> -			q_positions.push(portdock);
> +			positions_to_check.push(portdock);
>  		}
>  
> -		while (!q_positions.empty()) {
> -			const Coords& swim_coord = q_positions.front();
> -			q_positions.pop();
> +		while (!positions_to_check.empty()) {
> +			const Coords& swim_coord = positions_to_check.front();
> +			positions_to_check.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) {
>  					if (visited_positions.count(neighbour) == 0) {
>  						visited_positions.insert(neighbour);

Maybe something like 
if(swim_coord.count(neighbour)>){return True;}

would shorthen the code and execution a bit...

> -						q_positions.push(neighbour);
> +						positions_to_check.push(neighbour);
>  					}
>  				}
>  			}


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1718745-allows-seafaring/+merge/333233
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1718745-allows-seafaring.