epic/OpenRA
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix map editor not removing an actor properly.

Browse Source 
If you edit an actor name, then delete the actor - it fails to be removed from the map in the editor. This is because the actor previews are keyed by ID. Editing their name edits their ID and breaks the stability of their hash code. This unstable hash code means the preview will now fail to be removed from collections, even though it's the "same" object.

Fix this by making the ID immutable to ensure hash stability - this means that a preview can be added and removed from collections successfully. Now when we edit the ID in the UI, we can't update the ID in place on the preview. Instead we must generate a new preview with the correct ID and swap it with the preview currently in use.
This commit is contained in:
RoosterDragon
2024-03-14 18:58:21 +00:00
committed by Gustas
parent 25a6b4b6b9
commit 799c4c9e3c
3 changed files with 63 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
OpenRA.Mods.Common/Traits/World/EditorActorPreview.cs
View File

@@ -32,7 +32,7 @@ namespace OpenRA.Mods.Common.Traits
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 @@ namespace OpenRA.Mods.Common.Traits
onCellEntryChanged = _ => UpdateFromCellChange();
}
public EditorActorPreview WithId(string id)
{
return new EditorActorPreview(worldRenderer, id, reference.Clone(), Owner);
}
void UpdateFromCellChange()
{
CenterPosition = PreviewPosition(worldRenderer.World, reference);

84
OpenRA.Mods.Common/Widgets/Logic/Editor/ActorEditLogic.cs
View File

@@ -59,6 +59,8 @@ namespace OpenRA.Mods.Common.Widgets.Logic
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 @@ namespace OpenRA.Mods.Common.Widgets.Logic
{
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 @@ namespace OpenRA.Mods.Common.Widgets.Logic
void Save()
{
editorActionManager.Add(new EditActorEditorAction(editorActorLayer, SelectedActor, editActorPreview.GetDirtyHandles()));
editorActionManager.Add(new EditActorEditorAction(SelectedActor, editActorPreview.GetDirtyHandles()));
editActorPreview = null;
Close();
}
@@ -408,12 +413,12 @@ namespace OpenRA.Mods.Common.Widgets.Logic
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 @@ namespace OpenRA.Mods.Common.Widgets.Logic
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 @@ namespace OpenRA.Mods.Common.Widgets.Logic
[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 @@ namespace OpenRA.Mods.Common.Widgets.Logic
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 @@ namespace OpenRA.Mods.Common.Widgets.Logic
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; }

1
mods/common/languages/en.ftl
View File

@@ -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.