Merge pull request #6598 from pchote/fix-lua-dead-actor-bogosity

Fix Lua API interaction with destroyed actors

OpenRA.Game/Actor.cs

@@ -212,6 +212,9 @@ namespace OpenRA
 
 				World.traitDict.RemoveActor(this);
 				Destroyed = true;
+
+				if (luaInterface != null)
+					luaInterface.Value.OnActorDestroyed();
 			});
 		}
 
@@ -262,7 +265,8 @@ namespace OpenRA
 		Lazy<ScriptActorInterface> luaInterface;
 		public void OnScriptBind(ScriptContext context)
 		{
-			luaInterface = Exts.Lazy(() => new ScriptActorInterface(context, this));
+			if (luaInterface == null)
+				luaInterface = Exts.Lazy(() => new ScriptActorInterface(context, this));
 		}
 
 		public LuaValue this[LuaRuntime runtime, LuaValue keyValue]

OpenRA.Game/Scripting/ScriptActorInterface.cs

@@ -9,6 +9,7 @@
 #endregion
 
 using System;
+using System.Collections.Generic;
 using System.Linq;
 
 namespace OpenRA.Scripting
@@ -18,15 +19,33 @@ namespace OpenRA.Scripting
 		readonly Actor actor;
 
 		protected override string DuplicateKeyError(string memberName) { return "Actor '{0}' defines the command '{1}' on multiple traits".F(actor.Info.Name, memberName); }
-		protected override string MemberNotFoundError(string memberName) { return "Actor '{0}' does not define a property '{1}'".F(actor.Info.Name, memberName); }
+		protected override string MemberNotFoundError(string memberName)
+		{
+			var actorName = actor.Info.Name;
+			if (actor.IsDead())
+				actorName += " (dead)";
+
+			return "Actor '{0}' does not define a property '{1}'".F(actorName, memberName);
+		}
 
 		public ScriptActorInterface(ScriptContext context, Actor actor)
 			: base(context)
 		{
 			this.actor = actor;
 
+			InitializeBindings();
+		}
+
+		void InitializeBindings()
+		{
+			var commandClasses = context.ActorCommands[actor.Info].AsEnumerable();
+
+			// Destroyed actors cannot have their traits queried
+			if (actor.Destroyed)
+				commandClasses = commandClasses.Where(c => c.HasAttribute<ExposedForDestroyedActors>());
+
 			var args = new object[] { context, actor };
-			var objects = context.ActorCommands[actor.Info].Select(cg =>
+			var objects = commandClasses.Select(cg =>
 			{
 				var groupCtor = cg.GetConstructor(new Type[] { typeof(ScriptContext), typeof(Actor) });
 				return groupCtor.Invoke(args);
@@ -34,5 +53,11 @@ namespace OpenRA.Scripting
 
 			Bind(objects);
 		}
+
+		public void OnActorDestroyed()
+		{
+			// Regenerate bindings to remove access to bogus trait state
+			InitializeBindings();
+		}
 	}
 }

OpenRA.Game/Scripting/ScriptContext.cs

@@ -39,6 +39,9 @@ namespace OpenRA.Scripting
 		public ScriptPropertyGroupAttribute(string category) { Category = category; }
 	}
 
+	// For property groups that are safe to initialize invoke on destroyed actors
+	public sealed class ExposedForDestroyedActors : Attribute { }
+
 	public sealed class ScriptActorPropertyActivityAttribute : Attribute { }
 
 	public abstract class ScriptActorProperties

OpenRA.Mods.RA/Scripting/Properties/GeneralProperties.cs

@@ -15,18 +15,14 @@ using OpenRA.Traits;
 
 namespace OpenRA.Mods.RA.Scripting
 {
+	[ExposedForDestroyedActors]
 	[ScriptPropertyGroup("General")]
-	public class GeneralProperties : ScriptActorProperties
+	public class BaseActorProperties : ScriptActorProperties
 	{
-		readonly IFacing facing;
+		// Note: This class must not make any trait queries so that this
-		readonly AutoTarget autotarget;
+		// remains safe to call on dead actors.
-
+		public BaseActorProperties(ScriptContext context, Actor self)
-		public GeneralProperties(ScriptContext context, Actor self)
+			: base(context, self) { }
-			: base(context, self)
-		{
-			facing = self.TraitOrDefault<IFacing>();
-			autotarget = self.TraitOrDefault<AutoTarget>();
-		}
 
 		[Desc("Specifies whether the actor is in the world.")]
 		public bool IsInWorld
@@ -45,21 +41,44 @@ namespace OpenRA.Mods.RA.Scripting
 			}
 		}
 
+		[Desc("Specifies whether the actor is alive or dead.")]
+		public bool IsDead { get { return self.IsDead(); } }
+
 		[Desc("Specifies whether the actor is idle (not performing any activities).")]
 		public bool IsIdle { get { return self.IsIdle; } }
 
-		[Desc("The actor position in cell coordinates.")]
-		public CPos Location { get { return self.Location; } }
-
-		[Desc("The actor position in world coordinates.")]
-		public WPos CenterPosition { get { return self.CenterPosition; } }
-
 		[Desc("The player that owns the actor.")]
 		public Player Owner { get { return self.Owner; } }
 
 		[Desc("The type of the actor (e.g. \"e1\").")]
 		public string Type { get { return self.Info.Name; } }
 
+		[Desc("Test whether an actor has a specific property.")]
+		public bool HasProperty(string name)
+		{
+			return self.HasScriptProperty(name);
+		}
+	}
+
+	[ScriptPropertyGroup("General")]
+	public class GeneralProperties : ScriptActorProperties
+	{
+		readonly IFacing facing;
+		readonly AutoTarget autotarget;
+
+		public GeneralProperties(ScriptContext context, Actor self)
+			: base(context, self)
+		{
+			facing = self.TraitOrDefault<IFacing>();
+			autotarget = self.TraitOrDefault<AutoTarget>();
+		}
+
+		[Desc("The actor position in cell coordinates.")]
+		public CPos Location { get { return self.Location; } }
+
+		[Desc("The actor position in world coordinates.")]
+		public WPos CenterPosition { get { return self.CenterPosition; } }
+
 		[Desc("The direction that the actor is facing.")]
 		public int Facing
 		{
@@ -129,11 +148,5 @@ namespace OpenRA.Mods.RA.Scripting
 				autotarget.Stance = stance;
 			}
 		}
-
-		[Desc("Test whether an actor has a specific property.")]
-		public bool HasProperty(string name)
-		{
-			return self.HasScriptProperty(name);
-		}
 	}
 }

OpenRA.Mods.RA/Scripting/Properties/HealthProperties.cs

@@ -32,9 +32,6 @@ namespace OpenRA.Mods.RA.Scripting
 
 		[Desc("Maximum health of the actor.")]
 		public int MaxHealth { get { return health.MaxHP; } }
-
-		[Desc("Specifies whether the actor is alive or dead.")]
-		public bool IsDead { get { return health.IsDead; } }
 	}
 
 	[ScriptPropertyGroup("General")]