← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/warehouse_refactor into lp:widelands

 


Diff comments:

> 
> === renamed file 'src/logic/map_objects/attackable.h' => 'src/logic/map_objects/attack_target.h'

done.

> 
> === 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);

I do not particularly like the upcast macro - it makes a dynamic_cast look cheap, though it is pretty expensive. I added an additional assert to make clear that we never expect this to fail ever.

> +			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.


References