Merge pull request #10324 from RoosterDragon/change-frozen-actor-removals

GPS updates frozen actors that change owner

OpenRA.Game/TraitDictionary.cs

@@ -255,7 +255,7 @@ namespace OpenRA
 
 				public void Reset() { index = -1; }
 				public bool MoveNext() { return ++index < actors.Count; }
-				public TraitPair<T> Current { get { return new TraitPair<T> { Actor = actors[index], Trait = traits[index] }; } }
+				public TraitPair<T> Current { get { return new TraitPair<T>(actors[index], traits[index]); } }
 				object System.Collections.IEnumerator.Current { get { return Current; } }
 				public void Dispose() { }
 			}

OpenRA.Game/Traits/Player/FrozenActorLayer.cs

@@ -32,17 +32,18 @@ namespace OpenRA.Traits
 		public readonly WPos CenterPosition;
 		public readonly Rectangle Bounds;
 		public readonly HashSet<string> TargetTypes;
-		readonly IRemoveFrozenActor[] removeFrozenActors;
 		readonly Actor actor;
 		readonly Shroud shroud;
 
-		public Player Owner;
+		public Player Owner { get; private set; }
 
-		public ITooltipInfo TooltipInfo;
+		public ITooltipInfo TooltipInfo { get; private set; }
-		public Player TooltipOwner;
+		public Player TooltipOwner { get; private set; }
+		readonly ITooltip tooltip;
 
-		public int HP;
+		public int HP { get; private set; }
-		public DamageState DamageState;
+		public DamageState DamageState { get; private set; }
+		readonly IHealth health;
 
 		public bool Visible = true;
 		public bool Shrouded { get; private set; }
@@ -57,7 +58,6 @@ namespace OpenRA.Traits
 			actor = self;
 			this.shroud = shroud;
 			NeedRenderables = startsRevealed;
-			removeFrozenActors = self.TraitsImplementing<IRemoveFrozenActor>().ToArray();
 
 			// Consider all cells inside the map area (ignoring the current map bounds)
 			Footprint = footprint
@@ -68,6 +68,9 @@ namespace OpenRA.Traits
 			Bounds = self.Bounds;
 			TargetTypes = self.GetEnabledTargetTypes().ToHashSet();
 
+			tooltip = self.TraitsImplementing<ITooltip>().FirstOrDefault();
+			health = self.TraitOrDefault<IHealth>();
+
 			UpdateVisibility();
 		}
 
@@ -76,6 +79,23 @@ namespace OpenRA.Traits
 		public ActorInfo Info { get { return actor.Info; } }
 		public Actor Actor { get { return !actor.IsDead ? actor : null; } }
 
+		public void RefreshState()
+		{
+			Owner = actor.Owner;
+
+			if (health != null)
+			{
+				HP = health.HP;
+				DamageState = health.DamageState;
+			}
+
+			if (tooltip != null)
+			{
+				TooltipInfo = tooltip.TooltipInfo;
+				TooltipOwner = tooltip.Owner;
+			}
+		}
+
 		public void Tick()
 		{
 			if (flashTicks > 0)
@@ -127,16 +147,6 @@ namespace OpenRA.Traits
 
 		public bool HasRenderables { get { return !Shrouded && Renderables.Any(); } }
 
-		public bool ShouldBeRemoved(Player owner)
-		{
-			// PERF: Avoid LINQ.
-			foreach (var rfa in removeFrozenActors)
-				if (rfa.RemoveActor(actor, owner))
-					return true;
-
-			return false;
-		}
-
 		public override string ToString()
 		{
 			return "{0} {1}{2}".F(Info.Name, ID, IsValid ? "" : " (invalid)");
@@ -189,6 +199,13 @@ namespace OpenRA.Traits
 			partitionedFrozenActorIds.Add(fa.ID, FootprintBounds(fa));
 		}
 
+		public void Remove(FrozenActor fa)
+		{
+			partitionedFrozenActorIds.Remove(fa.ID);
+			world.ScreenMap.Remove(owner, fa);
+			frozenActorsById.Remove(fa.ID);
+		}
+
 		Rectangle FootprintBounds(FrozenActor fa)
 		{
 			var p1 = fa.Footprint[0];
@@ -216,7 +233,7 @@ namespace OpenRA.Traits
 		{
 			UpdateDirtyFrozenActorsFromDirtyBins();
 
-			var idsToRemove = new List<uint>();
+			var frozenActorsToRemove = new List<FrozenActor>();
 			VisibilityHash = 0;
 			FrozenHash = 0;
 
@@ -231,22 +248,16 @@ namespace OpenRA.Traits
 				if (dirtyFrozenActorIds.Contains(id))
 					frozenActor.UpdateVisibility();
 
-				if (frozenActor.ShouldBeRemoved(owner))
+				if (frozenActor.Visible)
-					idsToRemove.Add(id);
-				else if (frozenActor.Visible)
 					VisibilityHash += hash;
 				else if (frozenActor.Actor == null)
-					idsToRemove.Add(id);
+					frozenActorsToRemove.Add(frozenActor);
 			}
 
 			dirtyFrozenActorIds.Clear();
 
-			foreach (var id in idsToRemove)
+			foreach (var fa in frozenActorsToRemove)
-			{
+				Remove(fa);
-				partitionedFrozenActorIds.Remove(id);
-				world.ScreenMap.Remove(owner, frozenActorsById[id]);
-				frozenActorsById.Remove(id);
-			}
 		}
 
 		void UpdateDirtyFrozenActorsFromDirtyBins()

OpenRA.Game/Traits/TraitsInterfaces.cs

@@ -388,11 +388,6 @@ namespace OpenRA.Traits
 		void DoImpact(Target target, Actor firedBy, IEnumerable<int> damageModifiers);
 	}
 
-	public interface IRemoveFrozenActor
-	{
-		bool RemoveActor(Actor self, Player owner);
-	}
-
 	public interface IRulesetLoaded<TInfo> { void RulesetLoaded(Ruleset rules, TInfo info); }
 	public interface IRulesetLoaded : IRulesetLoaded<ActorInfo>, ITraitInfoInterface { }
 }

OpenRA.Game/World.cs

@@ -408,14 +408,21 @@ namespace OpenRA
 		}
 	}
 
-	public struct TraitPair<T>
+	public struct TraitPair<T> : IEquatable<TraitPair<T>>
 	{
-		public Actor Actor;
+		public readonly Actor Actor;
-		public T Trait;
+		public readonly T Trait;
 
-		public override string ToString()
+		public TraitPair(Actor actor, T trait) { Actor = actor; Trait = trait; }
-		{
+
-			return "{0}->{1}".F(Actor.Info.Name, Trait.GetType().Name);
+		public static bool operator ==(TraitPair<T> me, TraitPair<T> other) { return me.Actor == other.Actor && Equals(me.Trait, other.Trait); }
-		}
+		public static bool operator !=(TraitPair<T> me, TraitPair<T> other) { return !(me == other); }
+
+		public override int GetHashCode() { return Actor.GetHashCode() ^ Trait.GetHashCode(); }
+
+		public bool Equals(TraitPair<T> other) { return this == other; }
+		public override bool Equals(object obj) { return obj is TraitPair<T> && Equals((TraitPair<T>)obj); }
+
+		public override string ToString() { return "{0}->{1}".F(Actor.Info.Name, Trait.GetType().Name); }
 	}
 }

OpenRA.Mods.Common/Orders/PlaceBuildingOrderGenerator.cs

@@ -236,7 +236,7 @@ namespace OpenRA.Mods.Common.Orders
 
 			var blockers = allTiles.SelectMany(world.ActorMap.GetActorsAt)
 				.Where(a => a.Owner == queue.Actor.Owner && a.IsIdle)
-				.Select(a => new TraitPair<Mobile> { Actor = a, Trait = a.TraitOrDefault<Mobile>() });
+				.Select(a => new TraitPair<Mobile>(a, a.TraitOrDefault<Mobile>()));
 
 			foreach (var blocker in blockers.Where(x => x.Trait != null))
 			{

OpenRA.Mods.Common/Traits/Modifiers/FrozenUnderFog.cs

@@ -37,8 +37,6 @@ namespace OpenRA.Mods.Common.Traits
 		readonly PPos[] footprint;
 
 		PlayerDictionary<FrozenState> frozenStates;
-		ITooltip tooltip;
-		Health health;
 		bool isRendering;
 
 		class FrozenState
@@ -65,9 +63,6 @@ namespace OpenRA.Mods.Common.Traits
 
 		public void Created(Actor self)
 		{
-			tooltip = self.TraitsImplementing<ITooltip>().FirstOrDefault();
-			health = self.TraitOrDefault<Health>();
-
 			frozenStates = new PlayerDictionary<FrozenState>(self.World, (player, playerIndex) =>
 			{
 				var frozenActor = new FrozenActor(self, footprint, player.Shroud, startsRevealed);
@@ -81,20 +76,7 @@ namespace OpenRA.Mods.Common.Traits
 		void UpdateFrozenActor(Actor self, FrozenActor frozenActor, int playerIndex)
 		{
 			VisibilityHash |= 1 << (playerIndex % 32);
-
+			frozenActor.RefreshState();
-			frozenActor.Owner = self.Owner;
-
-			if (health != null)
-			{
-				frozenActor.HP = health.HP;
-				frozenActor.DamageState = health.DamageState;
-			}
-
-			if (tooltip != null)
-			{
-				frozenActor.TooltipInfo = tooltip.TooltipInfo;
-				frozenActor.TooltipOwner = tooltip.Owner;
-			}
 		}
 
 		bool IsVisibleInner(Actor self, Player byPlayer)

OpenRA.Mods.Common/Traits/Player/ProductionQueue.cs

@@ -361,7 +361,7 @@ namespace OpenRA.Mods.Common.Traits
 		public virtual TraitPair<Production> MostLikelyProducer()
 		{
 			var trait = self.TraitsImplementing<Production>().FirstOrDefault(p => p.Info.Produces.Contains(Info.Type));
-			return new TraitPair<Production> { Actor = self, Trait = trait };
+			return new TraitPair<Production>(self, trait);
 		}
 
 		// Builds a unit from the actor that holds this queue (1 queue per building)

OpenRA.Mods.Common/UtilityCommands/UpgradeRules.cs

@@ -2889,6 +2889,16 @@ namespace OpenRA.Mods.Common.UtilityCommands
 						node.Key = "AttackSuicides";
 				}
 
+				// Replaced GpsRemoveFrozenActor with FrozenUnderFogUpdatedByGps
+				if (engineVersion < 20160117)
+				{
+					if (node.Key == "GpsRemoveFrozenActor")
+					{
+						node.Key = "FrozenUnderFogUpdatedByGps";
+						node.Value.Nodes.Clear();
+					}
+				}
+
 				UpgradeActorRules(engineVersion, ref node.Value.Nodes, node, depth + 1);
 			}
 		}

OpenRA.Mods.RA/OpenRA.Mods.RA.csproj

@@ -90,7 +90,7 @@
     <Compile Include="Traits\Chronoshiftable.cs" />
     <Compile Include="Traits\Cloneable.cs" />
     <Compile Include="Traits\Disguise.cs" />
-    <Compile Include="Traits\GpsRemoveFrozenActor.cs" />
+    <Compile Include="Traits\FrozenUnderFogUpdatedByGps.cs" />
     <Compile Include="Traits\HarvesterHuskModifier.cs" />
     <Compile Include="Traits\Infiltration\InfiltrateForCash.cs" />
     <Compile Include="Traits\Infiltration\InfiltrateForExploration.cs" />

OpenRA.Mods.RA/Traits/FrozenUnderFogUpdatedByGps.cs

@@ -0,0 +1,105 @@
+#region Copyright & License Information
+/*
+ * Copyright 2007-2015 The OpenRA Developers (see AUTHORS)
+ * This file is part of OpenRA, which is free software. It is made
+ * available to you under the terms of the GNU General Public License
+ * as published by the Free Software Foundation. For more information,
+ * see COPYING.
+ */
+#endregion
+
+using System;
+using OpenRA.Mods.Common.Traits;
+using OpenRA.Primitives;
+using OpenRA.Traits;
+
+namespace OpenRA.Mods.RA.Traits
+{
+	using FrozenActorAction = Action<FrozenUnderFogUpdatedByGps, FrozenActorLayer, GpsWatcher, FrozenActor>;
+
+	[Desc("Updates frozen actors of actors that change owners, are sold or die whilst having an active GPS power.")]
+	public class FrozenUnderFogUpdatedByGpsInfo : ITraitInfo, Requires<FrozenUnderFogInfo>
+	{
+		public object Create(ActorInitializer init) { return new FrozenUnderFogUpdatedByGps(init); }
+	}
+
+	public class FrozenUnderFogUpdatedByGps : INotifyOwnerChanged, INotifyActorDisposing, IOnGpsRefreshed
+	{
+		static readonly FrozenActorAction Refresh = (fufubg, fal, gps, fa) =>
+		{
+			// Refreshes the visual state of the frozen actor, so ownership changes can be seen.
+			fa.RefreshState();
+			if (fa.HasRenderables)
+				fa.NeedRenderables = true;
+		};
+		static readonly FrozenActorAction Remove = (fufubg, fal, gps, fa) =>
+		{
+			// Removes the frozen actor. Once done, we no longer need to track GPS updates.
+			fal.Remove(fa);
+			gps.UnregisterForOnGpsRefreshed(fufubg.self, fufubg);
+		};
+
+		class Traits
+		{
+			public readonly FrozenActorLayer FrozenActorLayer;
+			public readonly GpsWatcher GpsWatcher;
+			public Traits(Player player, FrozenUnderFogUpdatedByGps frozenUnderFogUpdatedByGps)
+			{
+				FrozenActorLayer = player.PlayerActor.TraitOrDefault<FrozenActorLayer>();
+				GpsWatcher = player.PlayerActor.TraitOrDefault<GpsWatcher>();
+				GpsWatcher.RegisterForOnGpsRefreshed(frozenUnderFogUpdatedByGps.self, frozenUnderFogUpdatedByGps);
+			}
+		}
+
+		readonly PlayerDictionary<Traits> traits;
+		readonly Actor self;
+
+		public FrozenUnderFogUpdatedByGps(ActorInitializer init)
+		{
+			self = init.Self;
+			traits = new PlayerDictionary<Traits>(init.World, player => new Traits(player, this));
+		}
+
+		public void OnOwnerChanged(Actor self, Player oldOwner, Player newOwner)
+		{
+			ActOnFrozenActorsForAllPlayers(Refresh);
+		}
+
+		public void Disposing(Actor self)
+		{
+			ActOnFrozenActorsForAllPlayers(Remove);
+		}
+
+		public void OnGpsRefresh(Actor self, Player player)
+		{
+			if (self.IsDead)
+				ActOnFrozenActorForPlayer(player, Remove);
+			else
+				ActOnFrozenActorForPlayer(player, Refresh);
+		}
+
+		void ActOnFrozenActorsForAllPlayers(FrozenActorAction action)
+		{
+			for (var playerIndex = 0; playerIndex < traits.Count; playerIndex++)
+				ActOnFrozenActorForTraits(traits[playerIndex], action);
+		}
+
+		void ActOnFrozenActorForPlayer(Player player, FrozenActorAction action)
+		{
+			ActOnFrozenActorForTraits(traits[player], action);
+		}
+
+		void ActOnFrozenActorForTraits(Traits t, FrozenActorAction action)
+		{
+			if (t.FrozenActorLayer == null || t.GpsWatcher == null ||
+				!t.GpsWatcher.Granted || !t.GpsWatcher.GrantedAllies)
+				return;
+
+			var fa = t.FrozenActorLayer.FromID(self.ActorID);
+			if (fa == null)
+				return;
+
+			action(this, t.FrozenActorLayer, t.GpsWatcher, fa);
+		}
+	}
+}

OpenRA.Mods.RA/Traits/GpsRemoveFrozenActor.cs

@@ -1,55 +0,0 @@
-#region Copyright & License Information
-/*
- * Copyright 2007-2015 The OpenRA Developers (see AUTHORS)
- * This file is part of OpenRA, which is free software. It is made
- * available to you under the terms of the GNU General Public License
- * as published by the Free Software Foundation. For more information,
- * see COPYING.
- */
-#endregion
-
-using System.Collections.Generic;
-using System.Linq;
-using OpenRA.Traits;
-
-namespace OpenRA.Mods.RA.Traits
-{
-	[Desc("Removes frozen actors of actors that are dead or sold," +
-		" when having an active GPS power.")]
-	public class GpsRemoveFrozenActorInfo : ITraitInfo
-	{
-		[Desc("Should this trait also affect allied players?")]
-		public bool GrantAllies = true;
-
-		public object Create(ActorInitializer init) { return new GpsRemoveFrozenActor(init.Self, this); }
-	}
-
-	public class GpsRemoveFrozenActor : IRemoveFrozenActor
-	{
-		readonly GpsWatcher[] watchers;
-		readonly GpsRemoveFrozenActorInfo info;
-
-		public GpsRemoveFrozenActor(Actor self, GpsRemoveFrozenActorInfo info)
-		{
-			this.info = info;
-			watchers = self.World.ActorsWithTrait<GpsWatcher>().Select(w => w.Trait).ToArray();
-		}
-
-		public bool RemoveActor(Actor self, Player owner)
-		{
-			if (!self.IsDead)
-				return false;
-
-			foreach (var w in watchers)
-			{
-				if (w.Owner != owner && !(info.GrantAllies && w.Owner.IsAlliedWith(owner)))
-					continue;
-
-				if (w.Launched)
-					return true;
-			}
-
-			return false;
-		}
-	}
-}

OpenRA.Mods.RA/Traits/SupportPowers/GpsPower.cs

@@ -25,12 +25,13 @@ namespace OpenRA.Mods.RA.Traits
 
 	class GpsWatcher : ISync, IFogVisibilityModifier
 	{
-		[Sync] public bool Launched = false;
+		[Sync] public bool Launched { get; private set; }
-		[Sync] public bool GrantedAllies = false;
+		[Sync] public bool GrantedAllies { get; private set; }
-		[Sync] public bool Granted = false;
+		[Sync] public bool Granted { get; private set; }
-		public Player Owner;
+		public readonly Player Owner;
 
-		List<Actor> actors = new List<Actor> { };
+		readonly List<Actor> actors = new List<Actor>();
+		readonly HashSet<TraitPair<IOnGpsRefreshed>> notifyOnRefresh = new HashSet<TraitPair<IOnGpsRefreshed>>();
 
 		public GpsWatcher(Player owner) { Owner = owner; }
 
@@ -49,11 +50,11 @@ namespace OpenRA.Mods.RA.Traits
 		public void Launch(Actor atek, SupportPowerInfo info)
 		{
 			atek.World.Add(new DelayedAction(((GpsPowerInfo)info).RevealDelay * 25,
-					() =>
+				() =>
-					{
+				{
-						Launched = true;
+					Launched = true;
-						RefreshGps(atek);
+					RefreshGps(atek);
-					}));
+				}));
 		}
 
 		public void RefreshGps(Actor atek)
@@ -69,11 +70,18 @@ namespace OpenRA.Mods.RA.Traits
 
 		void RefreshGranted()
 		{
+			var wasGranted = Granted;
+			var wasGrantedAllies = GrantedAllies;
+
 			Granted = actors.Count > 0 && Launched;
 			GrantedAllies = Owner.World.ActorsHavingTrait<GpsWatcher>(g => g.Granted).Any(p => p.Owner.IsAlliedWith(Owner));
 
 			if (Granted || GrantedAllies)
 				Owner.Shroud.ExploreAll(Owner.World);
+
+			if (wasGranted != Granted || wasGrantedAllies != GrantedAllies)
+				foreach (var tp in notifyOnRefresh.ToList())
+					tp.Trait.OnGpsRefresh(tp.Actor, Owner);
 		}
 
 		public bool HasFogVisibility()
@@ -89,8 +97,20 @@ namespace OpenRA.Mods.RA.Traits
 
 			return gpsDot.IsDotVisible(Owner);
 		}
+
+		public void RegisterForOnGpsRefreshed(Actor actor, IOnGpsRefreshed toBeNotified)
+		{
+			notifyOnRefresh.Add(new TraitPair<IOnGpsRefreshed>(actor, toBeNotified));
+		}
+
+		public void UnregisterForOnGpsRefreshed(Actor actor, IOnGpsRefreshed toBeNotified)
+		{
+			notifyOnRefresh.Remove(new TraitPair<IOnGpsRefreshed>(actor, toBeNotified));
+		}
 	}
 
+	interface IOnGpsRefreshed { void OnGpsRefresh(Actor self, Player player); }
+
 	class GpsPowerInfo : SupportPowerInfo
 	{
 		public readonly int RevealDelay = 0;

mods/ra/rules/defaults.yaml

@@ -430,7 +430,7 @@
 	Guardable:
 		Range: 3c0
 	FrozenUnderFog:
-	GpsRemoveFrozenActor:
+	FrozenUnderFogUpdatedByGps:
 	Tooltip:
 		GenericName: Structure
 	Demolishable:
@@ -500,7 +500,7 @@
 		SellSounds: cashturn.aud
 	Guardable:
 	FrozenUnderFog:
-	GpsRemoveFrozenActor:
+	FrozenUnderFogUpdatedByGps:
 	Health:
 		HP: 100
 		Shape: Rectangle