Review: Approve
2 tiny nits, code LGTM otherwise. Not tested yet.
And trading - yay! :)
Diff comments:
>
> === renamed file 'src/logic/map_objects/attackable.h' =>
> 'src/logic/map_objects/attack_target.h'
Move this to src/logic/map_objects/tribes?
>
> === modified file 'src/logic/map_objects/tribes/soldier.cc'
> --- src/logic/map_objects/tribes/soldier.cc 2017-05-13 18:48:26 +0000
> +++ src/logic/map_objects/tribes/soldier.cc 2017-05-21 06:14:49 +0000
> @@ -1520,20 +1521,21 @@
> PlayerNumber const land_owner = get_position().field->get_owned_by();
> // First check if the soldier is standing on someone else's territory
> if (land_owner != owner().player_number()) {
> - // Let's collect all reachable attackable sites in vicinity
> (militarysites mainly)
> - std::vector<BaseImmovable*> attackables;
> + // Let's collect all reachable attack_target sites in vicinity
> (militarysites mainly)
> + std::vector<BaseImmovable*> attack_targets;
> game.map().find_reachable_immovables_unique(
> - Area<FCoords>(get_position(), MaxProtectionRadius),
> attackables,
> - CheckStepWalkOn(descr().movecaps(), false),
> FindImmovableAttackable());
> + Area<FCoords>(get_position(), kMaxProtectionRadius),
> attack_targets,
> + CheckStepWalkOn(descr().movecaps(), false),
> FindImmovableAttackTarget());
>
> - for (BaseImmovable* temp_attackable : attackables) {
> - const Player* attackable_player =
> - dynamic_cast<const
> PlayerImmovable&>(*temp_attackable).get_owner();
> + for (BaseImmovable* temp_attack_target : attack_targets) {
> + Building* building =
> dynamic_cast<Building*>(temp_attack_target);
Why not use the upcast macro?
> + assert(building->attack_target() != nullptr);
> + const Player& attack_target_player = building->owner();
> // Let's inform the site that this (=enemy) soldier is
> nearby and within the site's owner's
> // territory
> - if (attackable_player->player_number() == land_owner &&
> - attackable_player->is_hostile(*get_owner())) {
> -
> dynamic_cast<Attackable&>(*temp_attackable).aggressor(*this);
> + if (attack_target_player.player_number() == land_owner
> &&
> + attack_target_player.is_hostile(*get_owner())) {
> +
> building->attack_target()->enemy_soldier_approaches(*this);
> }
> }
> }
--
https://code.launchpad.net/~widelands-dev/widelands/warehouse_refactor/+merge/324367
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/warehouse_refactor.
_______________________________________________
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
