Check ITargetable when deciding target validity. Fixes #3659.

OpenRA.Game/Orders/UnitOrderGenerator.cs

@@ -99,7 +99,7 @@ namespace OpenRA.Orders
 			if (self.Owner != self.World.LocalPlayer)
 				return null;
 
-			if (self.Destroyed || target.Type == TargetType.Invalid)
+			if (self.Destroyed || !target.IsValidFor(self))
 				return null;
 
 			if (mi.Button == Game.mouseButtonPreference.Action)

OpenRA.Game/Traits/DrawLineToTarget.cs

@@ -64,7 +64,7 @@ namespace OpenRA.Traits
 
 			foreach (var target in targets)
 			{
-				if (!target.IsValid)
+				if (target.Type == TargetType.Invalid)
 					continue;
 
 				var to = wr.ScreenPxPosition(target.CenterPosition);

OpenRA.Game/Traits/Selectable.cs

@@ -161,7 +161,7 @@ namespace OpenRA.Traits
 				var c = Color.Green;
 
 				var wlr = Game.Renderer.WorldLineRenderer;
-				foreach (var stp in targets.Where(t => t.IsValid).Select(p => wr.ScreenPxPosition(p.CenterPosition)))
+				foreach (var stp in targets.Where(t => t.Type != TargetType.Invalid).Select(p => wr.ScreenPxPosition(p.CenterPosition)))
 				{
 					wlr.DrawLine(stp + new float2(-1, -1), stp + new float2(-1, 1), c, c);
 					wlr.DrawLine(stp + new float2(-1, 1), stp + new float2(1, 1), c, c);

OpenRA.Game/Traits/Target.cs

@@ -22,6 +22,7 @@ namespace OpenRA.Traits
 
 		TargetType type;
 		Actor actor;
+		ITargetable targetable;
 		FrozenActor frozen;
 		WPos pos;
 		int generation;
@@ -40,6 +41,7 @@ namespace OpenRA.Traits
 			return new Target
 			{
 				actor = a,
+				targetable = a != null ? a.TraitOrDefault<ITargetable>() : null,
 				type = a != null ? TargetType.Actor : TargetType.Invalid,
 				generation = a.Generation,
 			};
@@ -47,7 +49,6 @@ namespace OpenRA.Traits
 
 		public static Target FromFrozenActor(FrozenActor a)  { return new Target { frozen = a, type = TargetType.FrozenActor }; }
 
-		public bool IsValid { get { return Type != TargetType.Invalid; } }
 		public Actor Actor { get { return actor; } }
 		public FrozenActor FrozenActor { get { return frozen; } }
 
@@ -70,6 +71,17 @@ namespace OpenRA.Traits
 			}
 		}
 
+		public bool IsValidFor(Actor targeter)
+		{
+			if (Type == TargetType.Invalid)
+				return false;
+
+			if (targetable != null && !targetable.TargetableBy(actor, targeter))
+				return false;
+
+			return true;
+		}
+
 		// Representative position - see Positions for the full set of targetable positions.
 		public WPos CenterPosition
 		{

OpenRA.Mods.RA/Activities/Attack.cs

@@ -18,7 +18,6 @@ namespace OpenRA.Mods.RA.Activities
 	public class Attack : Activity
 	{
 		protected Target Target;
-		ITargetable targetable;
 		WRange Range;
 		bool AllowMovement;
 
@@ -33,9 +32,6 @@ namespace OpenRA.Mods.RA.Activities
 		public Attack(Target target, WRange range, bool allowMovement)
 		{
 			Target = target;
-			if (target.Type == TargetType.Actor)
-				targetable = target.Actor.TraitOrDefault<ITargetable>();
-
 			Range = range;
 			AllowMovement = allowMovement;
 		}
@@ -54,13 +50,11 @@ namespace OpenRA.Mods.RA.Activities
 				return NextActivity;
 
 			var type = Target.Type;
-			if (type != TargetType.Actor && type != TargetType.Terrain)
+			if (!Target.IsValidFor(self) || type == TargetType.FrozenActor)
-				return NextActivity;
-				
-			if (type == TargetType.Actor && !self.Owner.HasFogVisibility() && Target.Actor.HasTrait<Mobile>() && !self.Owner.Shroud.IsTargetable(Target.Actor))
 				return NextActivity;
 
-			if (targetable != null && !targetable.TargetableBy(Target.Actor, self))
+			// TODO: This is horrible, and probably wrong. Work out what it is trying to solve, then redo it properly.
+			if (type == TargetType.Actor && !self.Owner.HasFogVisibility() && Target.Actor.HasTrait<Mobile>() && !self.Owner.Shroud.IsTargetable(Target.Actor))
 				return NextActivity;
 
 			if (!Target.IsInRange(self.CenterPosition, Range))

OpenRA.Mods.RA/Activities/CaptureActor.cs

@@ -29,7 +29,7 @@ namespace OpenRA.Mods.RA.Activities
 
 			var capturable = target.Actor.Trait<Capturable>();
 
-			if (IsCanceled || !self.IsInWorld || self.IsDead())
+			if (IsCanceled || !self.IsInWorld || self.IsDead() || !target.IsValidFor(self))
 			{
 				if (capturable.CaptureInProgress)
 					capturable.EndCapture();

OpenRA.Mods.RA/Activities/Demolish.cs

@@ -27,7 +27,7 @@ namespace OpenRA.Mods.RA.Activities
 
 		public override Activity Tick(Actor self)
 		{
-			if (IsCanceled || target.Type != TargetType.Actor)
+			if (IsCanceled || !target.IsValidFor(self))
 				return NextActivity;
 
 			self.World.AddFrameEndTask(w => w.Add(new DelayedAction(delay, () =>

OpenRA.Mods.RA/Activities/DonateSupplies.cs

@@ -27,7 +27,10 @@ namespace OpenRA.Mods.RA.Activities
 
 		public override Activity Tick(Actor self)
 		{
-			if (IsCanceled || target.Type != TargetType.Actor)
+			if (IsCanceled || !target.IsValidFor(self))
+				return NextActivity;
+
+			if (target.Type != TargetType.Actor)
 				return NextActivity;
 
 			var targetActor = target.Actor;

OpenRA.Mods.RA/Activities/Enter.cs

@@ -27,7 +27,10 @@ namespace OpenRA.Mods.RA.Activities
 
 		public override Activity Tick(Actor self)
 		{
-			if (IsCanceled || target.Type != TargetType.Actor)
+			if (IsCanceled || !target.IsValidFor(self))
+				return NextActivity;
+
+			if (target.Type != TargetType.Actor)
 				return NextActivity;
 
 			if (!Util.AdjacentCells(target).Any(c => c == self.Location))

OpenRA.Mods.RA/Activities/Follow.cs

@@ -32,7 +32,7 @@ namespace OpenRA.Mods.RA.Activities
 
 		public override Activity Tick(Actor self)
 		{
-			if (IsCanceled || !target.IsValid)
+			if (IsCanceled || !target.IsValidFor(self))
 				return NextActivity;
 
 			if (target.IsInRange(self.CenterPosition, range) || --nextPathTime > 0)

OpenRA.Mods.RA/Activities/Heal.cs

@@ -22,6 +22,9 @@ namespace OpenRA.Mods.RA.Activities
 
 		protected override Activity InnerTick(Actor self, AttackBase attack)
 		{
+			if (!Target.IsValidFor(self))
+				return NextActivity;
+
 			if (Target.Type == TargetType.Actor && Target.Actor.GetDamageState() == DamageState.Undamaged)
 				return NextActivity;
 

OpenRA.Mods.RA/Activities/LegacyCaptureActor.cs

@@ -23,7 +23,10 @@ namespace OpenRA.Mods.RA.Activities
 
 		public override Activity Tick(Actor self)
 		{
-			if (IsCanceled || target.Type != TargetType.Actor)
+			if (IsCanceled || !target.IsValidFor(self))
+				return NextActivity;
+
+			if (target.Type != TargetType.Actor)
 				return NextActivity;
 
 			var actor = target.Actor;

OpenRA.Mods.RA/Activities/MoveAdjacentTo.cs

@@ -21,7 +21,7 @@ namespace OpenRA.Mods.RA.Activities
 
 		public override Activity Tick(Actor self)
 		{
-			if (IsCanceled || !target.IsValid)
+			if (IsCanceled || !target.IsValidFor(self))
 				return NextActivity;
 
 			var mobile = self.Trait<Mobile>();

OpenRA.Mods.RA/Air/FlyAttack.cs

@@ -21,7 +21,7 @@ namespace OpenRA.Mods.RA.Air
 
 		public override Activity Tick(Actor self)
 		{
-			if (!target.IsValid)
+			if (!target.IsValidFor(self))
 				Cancel(self);
 
 			var limitedAmmo = self.TraitOrDefault<LimitedAmmo>();

OpenRA.Mods.RA/Air/HeliAttack.cs

@@ -21,7 +21,7 @@ namespace OpenRA.Mods.RA.Air
 
 		public override Activity Tick(Actor self)
 		{
-			if (IsCanceled || !target.IsValid)
+			if (IsCanceled || !target.IsValidFor(self))
 				return NextActivity;
 
 			var limitedAmmo = self.TraitOrDefault<LimitedAmmo>();

OpenRA.Mods.RA/Air/Land.cs

@@ -22,7 +22,7 @@ namespace OpenRA.Mods.RA.Air
 
 		public override Activity Tick(Actor self)
 		{
-			if (!target.IsValid)
+			if (!target.IsValidFor(self))
 				Cancel(self);
 
 			if (IsCanceled)

OpenRA.Mods.RA/Attack/AttackBase.cs

@@ -45,7 +45,7 @@ namespace OpenRA.Mods.RA
 			if (!self.IsInWorld)
 				return false;
 
-			if (!target.IsValid)
+			if (!target.IsValidFor(self))
 				return false;
 
 			if (Armaments.All(a => a.IsReloading))
@@ -54,10 +54,6 @@ namespace OpenRA.Mods.RA
 			if (self.IsDisabled())
 				return false;
 
-			if (target.Type == TargetType.Actor && target.Actor.HasTrait<ITargetable>() &&
-				!target.Actor.Trait<ITargetable>().TargetableBy(target.Actor, self))
-				return false;
-
 			return true;
 		}
 
@@ -133,7 +129,7 @@ namespace OpenRA.Mods.RA
 			if (order.OrderString == "Attack")
 			{
 				var target = self.ResolveFrozenActorOrder(order, Color.Red);
-				if (!target.IsValid)
+				if (!target.IsValidFor(self))
 					return;
 
 				self.SetTargetLine(target, Color.Red);
@@ -155,7 +151,7 @@ namespace OpenRA.Mods.RA
 
 		public void AttackTarget(Target target, bool queued, bool allowMove)
 		{
-			if (!target.IsValid)
+			if (!target.IsValidFor(self))
 				return;
 
 			if (!queued)

OpenRA.Mods.RA/Attack/AttackOmni.cs

@@ -43,7 +43,7 @@ namespace OpenRA.Mods.RA
 
 			public override Activity Tick( Actor self )
 			{
-				if( IsCanceled || !target.IsValid )
+				if (IsCanceled || !target.IsValidFor(self))
 					return NextActivity;
 
 				self.Trait<AttackOmni>().DoAttack(self, target);

OpenRA.Mods.RA/Attack/AttackTesla.cs

@@ -67,7 +67,8 @@ namespace OpenRA.Mods.RA
 
 			public override Activity Tick( Actor self )
 			{
-				if( IsCanceled || !target.IsValid ) return NextActivity;
+				if (IsCanceled || !target.IsValidFor(self))
+					return NextActivity;
 
 				var attack = self.Trait<AttackTesla>();
 				if( attack.charges == 0 || !attack.CanAttack( self, target ) )
@@ -85,7 +86,8 @@ namespace OpenRA.Mods.RA
 
 			public override Activity Tick( Actor self )
 			{
-				if( IsCanceled || !target.IsValid ) return NextActivity;
+				if (IsCanceled || !target.IsValidFor(self))
+					return NextActivity;
 
 				var attack = self.Trait<AttackTesla>();
 				if( attack.charges == 0 ) return NextActivity;

OpenRA.Mods.RA/Attack/AttackTurreted.cs

@@ -38,7 +38,8 @@ namespace OpenRA.Mods.RA
 			if (self.HasTrait<Building>() && !buildComplete)
 				return false;
 
-			if (!target.IsValid) return false;
+			if (!target.IsValidFor(self))
+				return false;
 
 			bool canAttack = false;
 			foreach (var t in turrets)
@@ -53,7 +54,7 @@ namespace OpenRA.Mods.RA
 		{
 			base.Tick(self);
 			DoAttack(self, Target);
-			IsAttacking = Target.IsValid;
+			IsAttacking = Target.IsValidFor(self);
 		}
 
 		public override Activity GetAttackActivity(Actor self, Target newTarget, bool allowMove)
@@ -84,7 +85,8 @@ namespace OpenRA.Mods.RA
 
 			public override Activity Tick(Actor self)
 			{
-				if (IsCanceled || !target.IsValid) return NextActivity;
+				if (IsCanceled || !target.IsValidFor(self))
+					return NextActivity;
 
 				if (self.IsDisabled()) return this;
 

OpenRA.Mods.RA/Effects/LaserZap.cs

@@ -59,7 +59,7 @@ namespace OpenRA.Mods.RA.Effects
 		public void Tick(World world)
 		{
 			// Beam tracks target
-			if (args.guidedTarget.IsValid)
+			if (args.guidedTarget.IsValidFor(args.sourceActor))
 				target = args.guidedTarget.CenterPosition;
 
 			if (!doneDamage)

OpenRA.Mods.RA/Effects/Missile.cs

@@ -120,7 +120,7 @@ namespace OpenRA.Mods.RA.Effects
 			anim.Tick();
 
 			// Missile tracks target
-			if (args.guidedTarget.IsValid)
+			if (args.guidedTarget.IsValidFor(args.sourceActor))
 				target = args.guidedTarget.CenterPosition;
 
 			var dist = target + offset - pos;
@@ -133,7 +133,7 @@ namespace OpenRA.Mods.RA.Effects
 				desiredFacing = facing + world.SharedRandom.Next(-20, 21);
 				desiredAltitude = world.SharedRandom.Next(-43, 86);
 			}
-			else if (!args.guidedTarget.IsValid)
+			else if (!args.guidedTarget.IsValidFor(args.sourceActor))
 				desiredFacing = facing;
 
 			facing = Traits.Util.TickFacing(facing, desiredFacing, info.ROT);

OpenRA.Mods.RA/Effects/TeslaZap.cs

@@ -48,7 +48,7 @@ namespace OpenRA.Mods.RA.Effects
 
 			if (!doneDamage)
 			{
-				var pos = Args.guidedTarget.IsValid ? Args.guidedTarget.CenterPosition : Args.passiveTarget;
+				var pos = Args.guidedTarget.IsValidFor(Args.sourceActor) ? Args.guidedTarget.CenterPosition : Args.passiveTarget;
 				Combat.DoImpacts(pos, Args.sourceActor, Args.weapon, Args.firepowerModifier);
 				doneDamage = true;
 			}
@@ -58,7 +58,7 @@ namespace OpenRA.Mods.RA.Effects
 		{
 			if (!initialized)
 			{
-				var pos = Args.guidedTarget.IsValid ? Args.guidedTarget.CenterPosition : Args.passiveTarget;
+				var pos = Args.guidedTarget.IsValidFor(Args.sourceActor) ? Args.guidedTarget.CenterPosition : Args.passiveTarget;
 				zap = new TeslaZapRenderable(Args.source, 0, pos - Args.source, Info.Image, Info.BrightZaps, Info.DimZaps);
 			}
 			yield return zap;

OpenRA.Mods.RA/Move/Mobile.cs

@@ -496,7 +496,7 @@ namespace OpenRA.Mods.RA.Move
 
 			public bool CanTarget(Actor self, Target target, List<Actor> othersAtTarget, TargetModifiers modifiers, ref string cursor)
 			{
-				if (!target.IsValid)
+				if (!target.IsValidFor(self))
 					return false;
 
 				var location = target.CenterPosition.ToCPos();

OpenRA.Mods.RA/Move/Move.cs

@@ -73,7 +73,7 @@ namespace OpenRA.Mods.RA.Move
 		{
 			this.getPath = (self, mobile) =>
 			{
-				if (!target.IsValid)
+				if (!target.IsValidFor(self))
 					return NoPath;
 
 				return self.World.WorldActor.Trait<PathFinder>().FindUnitPathToRange(

OpenRA.Mods.RA/Reservable.cs

@@ -26,7 +26,7 @@ namespace OpenRA.Mods.RA
 			if (reservedFor == null)
 				return;		/* nothing to do */
 
-			if (!Target.FromActor( reservedFor ).IsValid)
+			if (!Target.FromActor(reservedFor).IsValidFor(self))
 				reservedFor = null;		/* not likely to arrive now. */
 		}