widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

> I think even portdock consisting one field would be still sufficient for port's operation...

I think that is the best solution, make the portdock one field only.

> I dont agree, especially for AI - it can easily 'miss' a portspace so multiple port spaces even in a tight line is better for it....

That is an unsafe solution - what if a map designer only adds one portspace in an area - I think most do that. AI must be clever enough to detect and uncover it anyways. And limiting the number of ports in an area will make the transport problem simpler for humans and the engine - having ten ports in a row on one shore will lead to really confusing transportation patterns.
I find your screenshot is a good argument for that - having those two ports does just not make sense. But I also think that this is another bug and should be tracked and fixed individually.

Addressed all code review comments. Getting it in!

@bunnybot merge

Diff comments:

> 
> === modified file 'src/logic/map_objects/tribes/ship.cc'
> --- src/logic/map_objects/tribes/ship.cc	2016-02-08 08:01:21 +0000
> +++ src/logic/map_objects/tribes/ship.cc	2016-02-08 21:01:32 +0000
> @@ -49,6 +49,59 @@
>  
>  namespace Widelands {
>  
> +namespace  {
> +
> +/// Returns true if 'coord' is not occupied or onwned by 'player_number' and
> +/// nothing stands there.
> +bool can_support_port(const PlayerNumber player_number, const FCoords& coord) {
> +	const PlayerNumber owner = coord.field->get_owned_by();
> +	if (owner != neutral() && owner != player_number) {
> +		return false;
> +	}
> +	BaseImmovable* baim = coord.field->get_immovable();
> +	if (baim != nullptr && baim->descr().type() >= MapObjectType::FLAG) {
> +		return false;
> +	}
> +	return true;
> +}
> +
> +/// Returns true if a ship owned by 'player_number' can land and errect a port at 'coord'.
> +bool can_build_port_here(const PlayerNumber player_number, const Map& map, const FCoords& coord) {

Why should const PlayerNumber be incorrect? It is a uint8, so it is more efficient to pass it by value and marking it const in the implementation gives me the safety that I do not change it accidentally in the function.

> +	if (!can_support_port(player_number, coord)) {
> +		return false;
> +	}
> +
> +	// All fields of the port + their neighboring fields (for the border) must
> +	// be conquerable without military influence.
> +	Widelands::FCoords c[4];  //  Big buildings occupy 4 locations.
> +	c[0] = coord;
> +	map.get_ln(coord, &c[1]);
> +	map.get_tln(coord, &c[2]);
> +	map.get_trn(coord, &c[3]);
> +	for (int i = 0; i < 4; ++i) {
> +		MapRegion<Area<FCoords>> area(map, Area<FCoords>(c[i], 1));
> +		do {
> +			if (!can_support_port(player_number, area.location())) {
> +				return false;
> +			}
> +		} while (area.advance(map));
> +	}
> +
> +	// Also all areas around the flag must be conquerable and must not contain
> +	// another flag already.
> +	FCoords flag_position;
> +	map.get_brn(coord, &flag_position);
> +	MapRegion<Area<FCoords>> area(map, Area<FCoords>(flag_position, 1));
> +	do {
> +		if (!can_support_port(player_number, area.location())) {
> +			return false;
> +		}
> +	} while (area.advance(map));
> +	return true;
> +}
> +
> +}  // namespace
> +
>  ShipDescr::ShipDescr(const std::string& init_descname, const LuaTable& table)
>  	:
>  	BobDescr(init_descname, MapObjectType::SHIP, MapObjectDescr::OwnerType::kTribe, table) {
> === added file 'test/maps/port_space.wmf/elemental'
> --- test/maps/port_space.wmf/elemental	1970-01-01 00:00:00 +0000
> +++ test/maps/port_space.wmf/elemental	2016-02-08 21:01:32 +0000
> @@ -0,0 +1,12 @@
> +# Automatically created by Widelands bzr7801[trunk] (Debug)
> +
> +[global]
> +packet_version="1"
> +map_w="64"
> +map_h="64"
> +nr_players="2"
> +name="port_space"
> +author="Unknown"
> +descr="No description defined"
> +hint=
> +tags="artifacts,seafaring"

good catch. Why was it added in the first place? I did never place an artifact on this map.



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