Review: Approve code
I still want to do some testing, but code LGTM - just a few minor nits :)
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
Typo: onwned => owned
> +/// 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'.
Typo: errect => erect
> +bool can_build_port_here(const PlayerNumber player_number, const Map& map,
> const FCoords& coord) {
const PlayerNumber& player_number or PlayerNumber player_number?
PlayerNumber is an uint8_t really, so I guess both versions would be correct?.
> + 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"
The "artifacts" tag should probably go?
--
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.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
