Fix editor area/actor deselection bugs

OpenRA.Mods.Common/EditorBrushes/EditorClipboard.cs

@@ -1,4 +1,15 @@
-using System.Collections.Generic;
+#region Copyright & License Information
+/*
+ * Copyright (c) The OpenRA Developers and Contributors
+ * 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, either version 3 of
+ * the License, or (at your option) any later version. For more
+ * information, see COPYING.
+ */
+#endregion
+
+using System.Collections.Generic;
 using OpenRA.Mods.Common.Traits;
 
 namespace OpenRA.Mods.Common.EditorBrushes
@@ -7,10 +18,10 @@ public readonly struct ClipboardTile
 	{
 		public readonly TerrainTile TerrainTile;
 		public readonly ResourceTile ResourceTile;
-		public readonly ResourceLayerContents ResourceLayerContents;
+		public readonly ResourceLayerContents? ResourceLayerContents;
 		public readonly byte Height;
 
-		public ClipboardTile(TerrainTile terrainTile, ResourceTile resourceTile, ResourceLayerContents resourceLayerContents, byte height)
+		public ClipboardTile(TerrainTile terrainTile, ResourceTile resourceTile, ResourceLayerContents? resourceLayerContents, byte height)
 		{
 			TerrainTile = terrainTile;
 			ResourceTile = resourceTile;

OpenRA.Mods.Common/EditorBrushes/EditorCopyPasteBrush.cs

@@ -161,7 +161,7 @@ EditorClipboard CopySelectionContents()
 				if (!mapTiles.Contains(cell))
 					continue;
 
-				var resourceLayerContents = resourceLayer.GetResource(cell);
+				var resourceLayerContents = resourceLayer?.GetResource(cell);
 				tiles.Add(cell, new ClipboardTile(mapTiles[cell], mapResources[cell], resourceLayerContents, mapHeight[cell]));
 
 				if (copyFilters.HasFlag(MapCopyFilters.Actors))
@@ -189,7 +189,7 @@ public void Do()
 					continue;
 
 				// Clear any existing resources.
-				if (copyFilters.HasFlag(MapCopyFilters.Resources))
+				if (resourceLayer != null && copyFilters.HasFlag(MapCopyFilters.Resources))
 					resourceLayer.ClearResources(position);
 
 				var tile = tileKeyValuePair.Value;
@@ -201,33 +201,38 @@ public void Do()
 					map.Height[position] = tile.Height;
 				}
 
-				if (copyFilters.HasFlag(MapCopyFilters.Resources) && !string.IsNullOrWhiteSpace(resourceLayerContents.Type))
-					resourceLayer.AddResource(resourceLayerContents.Type, position, resourceLayerContents.Density);
+				if (copyFilters.HasFlag(MapCopyFilters.Resources) &&
+					resourceLayerContents.HasValue &&
+					!string.IsNullOrWhiteSpace(resourceLayerContents.Value.Type))
+					resourceLayer.AddResource(resourceLayerContents.Value.Type, position, resourceLayerContents.Value.Density);
 			}
 
-			// Clear any existing actors in the paste cells.
-			var selectionSize = clipboard.CellRegion.BottomRight - clipboard.CellRegion.TopLeft;
-			var pasteRegion = new CellRegion(map.Grid.Type, pastePosition, pastePosition + selectionSize);
-			foreach (var regionActor in pasteRegion.SelectMany(editorActorLayer.PreviewsAt).ToHashSet())
-				editorActorLayer.Remove(regionActor);
-
-			// Now place actors.
-			foreach (var actorKeyValuePair in clipboard.Actors)
+			if (copyFilters.HasFlag(MapCopyFilters.Actors))
 			{
-				var selection = clipboard.CellRegion;
-				var copy = actorKeyValuePair.Value.Export();
-				var locationInit = copy.GetOrDefault<LocationInit>();
-				if (locationInit != null)
+				// Clear any existing actors in the paste cells.
+				var selectionSize = clipboard.CellRegion.BottomRight - clipboard.CellRegion.TopLeft;
+				var pasteRegion = new CellRegion(map.Grid.Type, pastePosition, pastePosition + selectionSize);
+				foreach (var regionActor in pasteRegion.SelectMany(editorActorLayer.PreviewsAt).ToHashSet())
+					editorActorLayer.Remove(regionActor);
+
+				// Now place actors.
+				foreach (var actorKeyValuePair in clipboard.Actors)
 				{
-					var actorPosition = locationInit.Value + new CVec(pastePosition.X - selection.TopLeft.X, pastePosition.Y - selection.TopLeft.Y);
-					if (!map.Contains(actorPosition))
-						continue;
-
-					copy.RemoveAll<LocationInit>();
-					copy.Add(new LocationInit(actorPosition));
+					var selection = clipboard.CellRegion;
+					var copy = actorKeyValuePair.Value.Export();
+					var locationInit = copy.GetOrDefault<LocationInit>();
+					if (locationInit != null)
+					{
+						var actorPosition = locationInit.Value + new CVec(pastePosition.X - selection.TopLeft.X, pastePosition.Y - selection.TopLeft.Y);
+						if (!map.Contains(actorPosition))
+							continue;
+
+						copy.RemoveAll<LocationInit>();
+						copy.Add(new LocationInit(actorPosition));
+					}
+
+					editorActorLayer.Add(copy);
 				}
-
-				editorActorLayer.Add(copy);
 			}
 		}
 
@@ -240,7 +245,7 @@ public void Undo()
 				var resourceLayerContents = tile.ResourceLayerContents;
 
 				// Clear any existing resources.
-				if (copyFilters.HasFlag(MapCopyFilters.Resources))
+				if (resourceLayer != null && copyFilters.HasFlag(MapCopyFilters.Resources))
 					resourceLayer.ClearResources(position);
 
 				if (copyFilters.HasFlag(MapCopyFilters.Terrain))
@@ -249,18 +254,22 @@ public void Undo()
 					map.Height[position] = tile.Height;
 				}
 
-				if (copyFilters.HasFlag(MapCopyFilters.Resources) && !string.IsNullOrWhiteSpace(resourceLayerContents.Type))
-					resourceLayer.AddResource(resourceLayerContents.Type, position, resourceLayerContents.Density);
+				if (copyFilters.HasFlag(MapCopyFilters.Resources) &&
+					resourceLayerContents.HasValue &&
+					!string.IsNullOrWhiteSpace(resourceLayerContents.Value.Type))
+					resourceLayer.AddResource(resourceLayerContents.Value.Type, position, resourceLayerContents.Value.Density);
 			}
 
-			// Clear existing actors.
-			foreach (var regionActor in undoClipboard.CellRegion.SelectMany(editorActorLayer.PreviewsAt).Distinct().ToList())
-				editorActorLayer.Remove(regionActor);
-
-			// Place actors back again.
 			if (copyFilters.HasFlag(MapCopyFilters.Actors))
+			{
+				// Clear existing actors.
+				foreach (var regionActor in undoClipboard.CellRegion.SelectMany(editorActorLayer.PreviewsAt).Distinct().ToList())
+					editorActorLayer.Remove(regionActor);
+
+				// Place actors back again.
 				foreach (var actor in undoClipboard.Actors.Values)
 					editorActorLayer.Add(actor);
+			}
 		}
 	}
 }

OpenRA.Mods.Common/EditorBrushes/EditorDefaultBrush.cs

@@ -26,13 +26,16 @@ public class EditorSelection
 	{
 		public CellRegion Area;
 		public EditorActorPreview Actor;
+
+		public bool HasSelection => Area != null || Actor != null;
 	}
 
 	public sealed class EditorDefaultBrush : IEditorBrush
 	{
 		const int MinMouseMoveBeforeDrag = 32;
 
 		public event Action SelectionChanged;
+		public event Action UpdateSelectedTab;
 
 		readonly WorldRenderer worldRenderer;
 		readonly World world;
@@ -43,7 +46,8 @@ public sealed class EditorDefaultBrush : IEditorBrush
 
 		public CellRegion CurrentDragBounds => selectionBounds ?? Selection.Area;
 
-		public EditorSelection Selection = new();
+		public EditorSelection Selection { get; private set; } = new();
+
 		EditorSelection previousSelection;
 		CellRegion selectionBounds;
 		int2? selectionStartLocation;
@@ -73,17 +77,34 @@ long CalculateActorSelectionPriority(EditorActorPreview actor)
 			return ((long)pixelDistance << 32) + worldZPosition;
 		}
 
-		public void ClearSelection()
+		public void ClearSelection(bool updateSelectedTab = false)
 		{
-			if (Selection.Area != null || Selection.Actor != null)
+			if (Selection.HasSelection)
 			{
 				previousSelection = Selection;
-				Selection = new EditorSelection();
+				SetSelection(new EditorSelection());
 				editorActionManager.Add(new ChangeSelectionAction(this, Selection, previousSelection));
-				SelectionChanged?.Invoke();
+
+				if (updateSelectedTab)
+					UpdateSelectedTab?.Invoke();
 			}
 		}
 
+		public void SetSelection(EditorSelection selection)
+		{
+			if (Selection == selection)
+				return;
+
+			if (Selection.Actor != null)
+				Selection.Actor.Selected = false;
+
+			Selection = selection;
+			if (Selection.Actor != null)
+				Selection.Actor.Selected = true;
+
+			SelectionChanged?.Invoke();
+		}
+
 		public bool HandleMouseInput(MouseInput mi)
 		{
 			// Exclusively uses mouse wheel and both mouse buttons, but nothing else.
@@ -155,70 +176,55 @@ public bool HandleMouseInput(MouseInput mi)
 					{
 						// Set this as the editor selection.
 						previousSelection = Selection;
-						Selection = new EditorSelection
+						SetSelection(new EditorSelection
 						{
 							Area = selectionBounds
-						};
+						});
 
-						editorActionManager.Add(new ChangeSelectionAction(this, Selection, previousSelection));
 						selectionBounds = null;
-						SelectionChanged?.Invoke();
+						editorActionManager.Add(new ChangeSelectionAction(this, Selection, previousSelection));
+						UpdateSelectedTab?.Invoke();
 					}
 					else if (underCursor != null)
 					{
 						// We've clicked on an actor.
 						if (Selection.Actor != underCursor)
 						{
 							previousSelection = Selection;
-							Selection = new EditorSelection
+							SetSelection(new EditorSelection
 							{
 								Actor = underCursor,
-							};
+							});
 
 							editorActionManager.Add(new ChangeSelectionAction(this, Selection, previousSelection));
-							SelectionChanged?.Invoke();
+							UpdateSelectedTab?.Invoke();
 						}
 					}
-					else if (Selection.Area != null || Selection.Actor != null)
+					else if (Selection.HasSelection)
 					{
 						// Released left mouse without dragging or selecting an actor - deselect current.
-						previousSelection = Selection;
-						Selection = new EditorSelection();
-
-						editorActionManager.Add(new ChangeSelectionAction(this, Selection, previousSelection));
-						SelectionChanged?.Invoke();
+						ClearSelection(updateSelectedTab: true);
 					}
 				}
-
-				if (mi.Button == MouseButton.Right)
+				else if (mi.Button == MouseButton.Right)
 				{
 					editorWidget.SetTooltip(null);
 
-					if (Selection.Area != null)
-					{
-						// Release right mouse button with a selection - clear selection.
-						previousSelection = Selection;
-						Selection = new EditorSelection();
-
-						editorActionManager.Add(new ChangeSelectionAction(this, Selection, previousSelection));
-					}
-					else
-					{
-						// Release right mouse button with no selection and over an actor - delete actor.
-						if (underCursor != null && underCursor != Selection.Actor)
-							editorActionManager.Add(new RemoveActorAction(editorLayer, underCursor));
+					// Delete actor.
+					if (underCursor != null && underCursor != Selection.Actor)
+						editorActionManager.Add(new RemoveActorAction(editorLayer, underCursor));
 
-						// Or delete resource if found under cursor.
-						if (resourceUnderCursor != null)
-							editorActionManager.Add(new RemoveResourceAction(resourceLayer, cell, resourceUnderCursor));
-					}
+					// Or delete resource if found under cursor.
+					if (resourceUnderCursor != null)
+						editorActionManager.Add(new RemoveResourceAction(resourceLayer, cell, resourceUnderCursor));
 				}
 			}
 
 			return true;
 		}
 
 		public void Tick() { }
+
 		public void Dispose() { }
 	}
 
@@ -271,12 +277,59 @@ public void Execute()
 
 		public void Do()
 		{
-			defaultBrush.Selection = selection;
+			defaultBrush.SetSelection(selection);
+		}
+
+		public void Undo()
+		{
+			defaultBrush.SetSelection(previousSelection);
+		}
+	}
+
+	sealed class RemoveSelectedActorAction : IEditorAction
+	{
+		[TranslationReference("name", "id")]
+		const string RemovedActor = "notification-removed-actor";
+
+		public string Text { get; }
+
+		readonly EditorSelection selection;
+		readonly EditorDefaultBrush defaultBrush;
+		readonly EditorActorLayer editorActorLayer;
+		readonly EditorActorPreview actor;
+
+		public RemoveSelectedActorAction(
+			EditorDefaultBrush defaultBrush,
+			EditorActorLayer editorActorLayer,
+			EditorActorPreview actor)
+		{
+			this.defaultBrush = defaultBrush;
+			this.editorActorLayer = editorActorLayer;
+			this.actor = actor;
+			selection = new EditorSelection
+			{
+				Actor = defaultBrush.Selection.Actor
+			};
+
+			Text = TranslationProvider.GetString(RemovedActor,
+				Translation.Arguments("name", actor.Info.Name, "id", actor.ID));
+		}
+
+		public void Execute()
+		{
+			Do();
+		}
+
+		public void Do()
+		{
+			defaultBrush.SetSelection(new EditorSelection());
+			editorActorLayer.Remove(actor);
 		}
 
 		public void Undo()
 		{
-			defaultBrush.Selection = previousSelection;
+			editorActorLayer.Add(actor);
+			defaultBrush.SetSelection(selection);
 		}
 	}
 

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

@@ -156,6 +156,7 @@ public OpenMapAction()
 
 		public void Execute()
 		{
+			Do();
 		}
 
 		public void Do()

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

@@ -59,7 +59,7 @@ void IWorldLoaded.WorldLoaded(World w, WorldRenderer wr)
 
 		IEnumerable<IRenderable> IRenderAnnotations.RenderAnnotations(Actor self, WorldRenderer wr)
 		{
-			if (editor.CurrentBrush == editor.DefaultBrush && editor.DefaultBrush.CurrentDragBounds != null)
+			if (editor.DefaultBrush.CurrentDragBounds != null)
 			{
 				yield return new EditorSelectionAnnotationRenderable(editor.DefaultBrush.CurrentDragBounds, info.AltColor, info.AltPixelOffset, null);
 				yield return new EditorSelectionAnnotationRenderable(editor.DefaultBrush.CurrentDragBounds, info.MainColor, int2.Zero, null);

OpenRA.Mods.Common/Widgets/EditorViewportControllerWidget.cs

@@ -42,7 +42,8 @@ public EditorViewportControllerWidget(WorldRenderer worldRenderer)
 		public void ClearBrush() { SetBrush(null); }
 		public void SetBrush(IEditorBrush brush)
 		{
-			CurrentBrush?.Dispose();
+			if (CurrentBrush != DefaultBrush)
+				CurrentBrush?.Dispose();
 
 			CurrentBrush = brush ?? DefaultBrush;
 		}