Fix map editor not removing an actor properly.

OpenRA.Mods.Common/Traits/World/EditorActorPreview.cs

@@ -32,7 +32,7 @@ public class EditorActorPreview : IEquatable<EditorActorPreview>
 
 		public string Type => reference.Type;
 
-		public string ID { get; set; }
+		public string ID { get; }
 		public PlayerReference Owner { get; set; }
 		public WPos CenterPosition { get; set; }
 		public IReadOnlyDictionary<CPos, SubCell> Footprint { get; private set; }
@@ -83,6 +83,11 @@ public EditorActorPreview(WorldRenderer worldRenderer, string id, ActorReference
 			onCellEntryChanged = _ => UpdateFromCellChange();
 		}
 
+		public EditorActorPreview WithId(string id)
+		{
+			return new EditorActorPreview(worldRenderer, id, reference.Clone(), Owner);
+		}
+
 		void UpdateFromCellChange()
 		{
 			CenterPosition = PreviewPosition(worldRenderer.World, reference);

OpenRA.Mods.Common/Widgets/Logic/Editor/ActorEditLogic.cs

@@ -59,6 +59,8 @@ enum ActorIDStatus { Normal = 0, Duplicate = 1, Empty = 3 }
 
 		EditorActorPreview SelectedActor => editor.DefaultBrush.Selection.Actor;
 
+		internal bool IsChangingSelection { get; set; }
+
 		[ObjectCreator.UseCtor]
 		public ActorEditLogic(Widget widget, World world, WorldRenderer worldRenderer, Dictionary<string, MiniYaml> logicArgs)
 		{
@@ -155,9 +157,12 @@ void HandleSelectionChanged()
 		{
 			if (SelectedActor != null)
 			{
-				Reset();
+				// Our edit control is updating the selection to account for an actor ID change.
+				// Don't try and reset, we're the ones who instigated the selection change!
+				if (!IsChangingSelection)
+					Reset();
 
-				editActorPreview = new EditActorPreview(SelectedActor);
+				editActorPreview = new EditActorPreview(this, editor, editorActorLayer, SelectedActor);
 
 				initialActorID = actorIDField.Text = SelectedActor.ID;
 
@@ -382,7 +387,7 @@ void Close()
 
 		void Save()
 		{
-			editorActionManager.Add(new EditActorEditorAction(editorActorLayer, SelectedActor, editActorPreview.GetDirtyHandles()));
+			editorActionManager.Add(new EditActorEditorAction(SelectedActor, editActorPreview.GetDirtyHandles()));
 			editActorPreview = null;
 			Close();
 		}
@@ -408,12 +413,12 @@ public void OnChange(T value)
 			this.value = value;
 		}
 
-		public void Do(EditorActorPreview actor)
+		public void Do(ref EditorActorPreview actor)
 		{
 			change(actor, value);
 		}
 
-		public void Undo(EditorActorPreview actor)
+		public void Undo(ref EditorActorPreview actor)
 		{
 			change(actor, initialValue);
 		}
@@ -424,8 +429,8 @@ public void Undo(EditorActorPreview actor)
 
 	public interface IEditActorHandle
 	{
-		void Do(EditorActorPreview actor);
-		void Undo(EditorActorPreview actor);
+		void Do(ref EditorActorPreview actor);
+		void Undo(ref EditorActorPreview actor);
 		bool IsDirty { get; }
 		bool ShouldDoOnSave { get; }
 	}
@@ -435,52 +440,56 @@ sealed class EditActorEditorAction : IEditorAction
 		[TranslationReference("name", "id")]
 		const string EditedActor = "notification-edited-actor";
 
-		public string Text { get; }
+		[TranslationReference("name", "old-id", "new-id")]
+		const string EditedActorId = "notification-edited-actor-id";
+
+		public string Text { get; private set; }
+		public EditorActorPreview Actor;
 
 		readonly IEnumerable<IEditActorHandle> handles;
-		readonly EditorActorLayer editorActorLayer;
-		EditorActorPreview actor;
-		readonly string actorId;
 
-		public EditActorEditorAction(EditorActorLayer editorActorLayer, EditorActorPreview actor, IEnumerable<IEditActorHandle> handles)
+		public EditActorEditorAction(EditorActorPreview actor, IEnumerable<IEditActorHandle> handles)
 		{
-			this.editorActorLayer = editorActorLayer;
-			actorId = actor.ID;
-			this.actor = actor;
+			Actor = actor;
 			this.handles = handles;
 			Text = TranslationProvider.GetString(EditedActor, Translation.Arguments("name", actor.Info.Name, "id", actor.ID));
 		}
 
 		public void Execute()
 		{
+			var before = Actor;
+
 			foreach (var editorActionHandle in handles.Where(h => h.ShouldDoOnSave))
-				editorActionHandle.Do(actor);
+				editorActionHandle.Do(ref Actor);
+
+			var after = Actor;
+			if (before != after)
+				Text = TranslationProvider.GetString(EditedActorId, Translation.Arguments("name", after.Info.Name, "old-id", before.ID, "new-id", after.ID));
 		}
 
 		public void Do()
 		{
-			actor = editorActorLayer[actorId];
 			foreach (var editorActionHandle in handles)
-				editorActionHandle.Do(actor);
+				editorActionHandle.Do(ref Actor);
 		}
 
 		public void Undo()
 		{
 			foreach (var editorActionHandle in handles)
-				editorActionHandle.Undo(actor);
+				editorActionHandle.Undo(ref Actor);
 		}
 	}
 
 	sealed class EditActorPreview
 	{
-		readonly EditorActorPreview actor;
 		readonly SetActorIdAction setActorIdAction;
 		readonly List<IEditActorHandle> handles = new();
+		EditorActorPreview actor;
 
-		public EditActorPreview(EditorActorPreview actor)
+		public EditActorPreview(ActorEditLogic logic, EditorViewportControllerWidget editor, EditorActorLayer editorActorLayer, EditorActorPreview actor)
 		{
 			this.actor = actor;
-			setActorIdAction = new SetActorIdAction(actor.ID);
+			setActorIdAction = new SetActorIdAction(logic, editor, editorActorLayer, actor.ID);
 			handles.Add(setActorIdAction);
 		}
 
@@ -507,12 +516,15 @@ public IEnumerable<IEditActorHandle> GetDirtyHandles()
 		public void Reset()
 		{
 			foreach (var handle in handles.Where(h => h.IsDirty))
-				handle.Undo(actor);
+				handle.Undo(ref actor);
 		}
 	}
 
 	public class SetActorIdAction : IEditActorHandle
 	{
+		readonly ActorEditLogic logic;
+		readonly EditorViewportControllerWidget editor;
+		readonly EditorActorLayer editorActorLayer;
 		readonly string initial;
 		string newID;
 
@@ -522,19 +534,35 @@ public void Set(string actorId)
 			newID = actorId;
 		}
 
-		public SetActorIdAction(string initial)
+		public SetActorIdAction(ActorEditLogic logic, EditorViewportControllerWidget editor, EditorActorLayer editorActorLayer, string initial)
 		{
+			this.logic = logic;
+			this.editor = editor;
+			this.editorActorLayer = editorActorLayer;
 			this.initial = initial;
 		}
 
-		public void Do(EditorActorPreview actor)
+		public void Do(ref EditorActorPreview actor)
 		{
-			actor.ID = newID;
+			// We can't update the ID of an EditorActorPreview in place - it's the hash and equality key of a preview.
+			// So instead we need to swap in an entirely new preview with the updated ID.
+			// This affects the actor layer, and the current selection.
+			editorActorLayer.Remove(actor);
+			actor = actor.WithId(newID);
+			editorActorLayer.Add(actor);
+			logic.IsChangingSelection = true;
+			editor.DefaultBrush.SetSelection(new EditorSelection { Actor = actor });
+			logic.IsChangingSelection = false;
 		}
 
-		public void Undo(EditorActorPreview actor)
+		public void Undo(ref EditorActorPreview actor)
 		{
-			actor.ID = initial;
+			editorActorLayer.Remove(actor);
+			actor = actor.WithId(initial);
+			editorActorLayer.Add(actor);
+			logic.IsChangingSelection = true;
+			editor.DefaultBrush.SetSelection(new EditorSelection { Actor = actor });
+			logic.IsChangingSelection = false;
 		}
 
 		public bool IsDirty { get; private set; }

mods/common/languages/en.ftl

@@ -848,6 +848,7 @@ mirror-mode =
 
 ## ActorEditLogic
 notification-edited-actor = Edited { $name } ({ $id })
+notification-edited-actor-id = Edited { $name } ({ $old-id }->{ $new-id })
 
 ## ConquestVictoryConditions, StrategicVictoryConditions
 notification-player-is-victorious = { $player } is victorious.