From 77b845e0986b7baabbfc8d3f714a9ffa3cdc0bb3 Mon Sep 17 00:00:00 2001
From: David Mann <daaavid.mann@gmail.com>
Date: Mon, 19 Oct 2026 20:20:02 +0200
Subject: [PATCH] Actions: loading a set renames each Play target once (#6806)
 (#6807)

Load Actions applied each rename as its own pass over every action in the set, so chained
renames ("A" -> "A 2", "A 2" -> "A 2 2") retargeted a step twice, and with replace:false it
also rewrote steps of actions already in the set. Now one original->new map is applied once
to the newly loaded actions' Play and Conditional (then/else) steps and to function keys.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---
 crates/engine/src/atn_cmds.rs       | 30 ++++++++++++++++----------
 crates/engine/src/atn_cmds/tests.rs | 33 +++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/crates/engine/src/atn_cmds.rs b/crates/engine/src/atn_cmds.rs
index 0cfbc31156..9e8eb6cb7c 100644
--- a/crates/engine/src/atn_cmds.rs
+++ b/crates/engine/src/atn_cmds.rs
@@ -3290,31 +3290,39 @@ fn load(s: &mut Session, p: &Value) -> Result<Value> {
     }
     let mut taken: Vec<String> = s.actions.list.iter().map(|a| a.name.clone()).collect();
     let mut names = Vec::with_capacity(imp.actions.len());
-    let mut renamed = Vec::new();
+    // The file's name of each action that had to be renamed, and its new name.
+    let mut renamed: std::collections::HashMap<String, String> = std::collections::HashMap::new();
     let mut steps = 0usize;
+    let mut added = Vec::with_capacity(imp.actions.len());
     for mut a in imp.actions {
         let name = unique(&a.name, &taken);
         if name != a.name {
-            renamed.push((a.name.clone(), name.clone()));
+            renamed.entry(a.name.clone()).or_insert_with(|| name.clone());
         }
         a.name = name.clone();
         taken.push(name.clone());
         steps = steps.saturating_add(a.steps.len());
         names.push(name);
-        s.actions.list.push(a);
+        added.push(a);
     }
-    // A Play step calls its action by name: follow renames within the set.
-    for (from, to) in &renamed {
-        for a in s.actions.list.iter_mut().filter(|a| a.set.as_deref() == Some(imp.set.as_str())) {
-            for (id, params) in &mut a.steps {
-                if id == "actions.play" && params.get("action").and_then(Value::as_str) == Some(from) {
-                    params["action"] = json!(to);
+    // Play and Conditional steps call actions by name: follow the renames in the actions just
+    // loaded, each target looked up once by the name the file gave it.
+    for a in &mut added {
+        for (id, params) in &mut a.steps {
+            let keys: &[&str] = match id.as_str() {
+                "actions.play" => &["action"],
+                "actions.conditional" => &["then", "else"],
+                _ => &[],
+            };
+            for key in keys {
+                if let Some(to) = params.get(*key).and_then(Value::as_str).and_then(|n| renamed.get(n)) {
+                    params[*key] = json!(to);
                 }
             }
         }
     }
-    let keys: Vec<(String, String)> =
-        imp.function_keys.into_iter().map(|(n, k)| (renamed.iter().find(|(f, _)| f == &n).map_or(n, |(_, t)| t.clone()), k)).collect();
+    s.actions.list.extend(added);
+    let keys: Vec<(String, String)> = imp.function_keys.into_iter().map(|(n, k)| (renamed.get(&n).cloned().unwrap_or(n), k)).collect();
     if !keys.is_empty() {
         s.edit_prefs(|prefs| {
             for (name, key) in &keys {
diff --git a/crates/engine/src/atn_cmds/tests.rs b/crates/engine/src/atn_cmds/tests.rs
index f22a153232..26865d4060 100644
--- a/crates/engine/src/atn_cmds/tests.rs
+++ b/crates/engine/src/atn_cmds/tests.rs
@@ -130,6 +130,39 @@ fn load_maps_events_keeps_flags_and_plays() {
     assert_eq!(s.actions.list[2].set.as_deref(), Some("Other"));
 }
 
+/// Renames that chain ("A" becomes "A 2" while the file's own "A 2" becomes "A 2 2") rewrite
+/// each Play and Conditional target once, to the renamed copy of the action it named.
+#[test]
+fn load_renames_each_play_target_once() {
+    let mut s = Session::new();
+    let mut mine = Action::new("A", vec![]);
+    mine.set = Some("Mine".into());
+    s.actions.list.push(mine);
+    let a = Action::new("A", vec![("actions.conditional".into(), json!({"if": "always", "then": "A", "else": "A 2"}))]);
+    let a2 = Action::new("A 2", vec![("actions.play".into(), json!({"action": "A"}))]);
+    let keys = [("A".to_string(), "F4".to_string())];
+    let (bytes, _) = export("File", &[&a, &a2], &keys, false).unwrap();
+    let r = s.execute("actions.load", json!({"data": b64(&bytes)})).unwrap();
+    assert_eq!(r["actions"], json!(["A 2", "A 2 2"]));
+    let get = |n: &str| s.actions.list.iter().find(|a| a.name == n && a.set.as_deref() == Some("File")).unwrap();
+    assert_eq!(get("A 2 2").steps[0].1["action"], "A 2", "plays the file's A, not its A 2");
+    let cond = &get("A 2").steps[0].1;
+    assert_eq!((&cond["then"], &cond["else"]), (&json!("A 2"), &json!("A 2 2")), "{cond}");
+    assert_eq!(r["functionKeys"], json!([{"action": "A 2", "key": "F4"}]));
+
+    // Adding to a set of the same name leaves the Play steps already in it alone.
+    let mut s = Session::new();
+    for (name, steps) in [("A", vec![]), ("B", vec![("actions.play".to_string(), json!({"action": "A"}))])] {
+        let mut a = Action::new(name, steps);
+        a.set = Some("File".into());
+        s.actions.list.push(a);
+    }
+    let (bytes, _) = export("File", &[&Action::new("A", vec![])], &[], false).unwrap();
+    let r = s.execute("actions.load", json!({"data": b64(&bytes), "replace": false})).unwrap();
+    assert_eq!(r["actions"], json!(["A 2"]));
+    assert_eq!(s.actions.list[1].steps[0].1, json!({"action": "A"}), "B still plays the A it had");
+}
+
 #[test]
 fn save_round_trips_through_photoshop_events() {
     let mut s = Session::new();