A region keeps its own name; only the name lookup is first-come (#944)

Region names lived in one first-wins name → id map that also served `GetName`, so a second region given a taken name answered `GetName()` with nil and was `<unnamed>` in its error strings.

In 1.12.1 the name lives on the region: `SetName` (`0x76c650`) keeps the region's own copy at `[+0x98]`, freed by its destructor (`0x76c560`) and read back by `GetName`. Only `_G` is first-come (`0x701bd0`, `0x701c8a`).

Each region now carries its name; `GetName` and the error strings read it directly, and the name map is a first-come lookup only, whose entry a free drops only when it holds that region.

Tests: same name twice, `$parentText` under colliding parents, the second region's error string, a free leaving a same-named sibling's name.

Fixes #939

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/benilla-ui/src/loader/widgets.rs

@@ -118,9 +118,9 @@ impl Loader<'_> {
                     continue; // build_texture_child already reported why
                 };
                 this.install_texture(wrapper, tag, &region, None, dbg);
-                // Freeing the displaced region took its name out of the registry `GetName()`
-                // reads, which a replacement of the same name takes back: the reference keeps the
-                // name on the region (`0x76c650`). `_G` keeps its first holder (`0x701c8a`).
+                // Freeing the displaced region took its entry out of the name lookup, which a
+                // replacement of the same name takes back. The replacement's `GetName()` is its
+                // own (`0x76c650`); `_G` keeps its first holder (`0x701c8a`).
                 if let Some(rname) = raw.name().map(|n| framexml::resolve_name(n, self_name)) {
                     crate::script::region::publish_region_name(this.lua(), &rname, &region);
                 }

crates/benilla-ui/src/script/loot.rs

@@ -342,7 +342,7 @@ mod tests {
     /// The region data behind a named texture: its portrait key and file.
     fn portrait_of(s: &UiScript, name: &str) -> (Option<String>, Option<String>) {
         let model = s.model_ref();
-        let id = model.region_names[name];
+        let id = model.region_by_name[name];
         let rh = *model.id_to_region.get(&id).expect("a region");
         let data = &model.region_data[&rh];
         (data.portrait_unit.clone(), data.texture.clone())

crates/benilla-ui/src/script/mod.rs

@@ -1198,7 +1198,7 @@ impl UiScript {
             });
         }
         // Region leaves publish into their own name table, not the arena's.
-        let id = *model.region_names.get(name)?;
+        let id = *model.region_by_name.get(name)?;
         let h = *model.id_to_region.get(&id)?;
         model.arena.region(h).map(|r| match r.kind {
             crate::widget::RegionKind::Texture => "Texture",

crates/benilla-ui/src/script/model.rs

@@ -183,8 +183,10 @@ pub(crate) struct Model {
     pub(crate) frame_to_id: HashMap<FrameHandle, u32>,
     pub(crate) id_to_region: IdMap<RegionHandle>,
     pub(crate) region_to_id: HashMap<RegionHandle, u32>,
-    /// Region name to id, first wins: `SetPoint` anchors to a sibling region by name, after frames.
-    pub(crate) region_names: HashMap<String, u32>,
+    /// Region name to the first region that took it, for name lookups only (`SetPoint` anchoring
+    /// to a sibling by name, after frames): first come, as the `_G` entry is (`0x701bd0`). A
+    /// region's own name, for `GetName` and error strings, is [`crate::widget::Region::name`].
+    pub(crate) region_by_name: HashMap<String, u32>,
 
     /// Which handlers each frame has; the closures themselves live in Lua's `REG_SCRIPTS` table.
     pub(crate) scripts: HashMap<FrameHandle, HashSet<&'static str>>,
@@ -1126,7 +1128,7 @@ impl Model {
             frame_to_id: HashMap::new(),
             id_to_region: IdMap::default(),
             region_to_id: HashMap::new(),
-            region_names: HashMap::new(),
+            region_by_name: HashMap::new(),
             scripts: HashMap::new(),
             on_update_frames: Vec::new(),
             on_update_joins: 0,

crates/benilla-ui/src/script/object/events_regions.rs

@@ -359,9 +359,12 @@ fn create_region(
             crate::framexml::resolve_name(&name, &super::parent_token_base(&model, Some(owner)))
         };
         publish_global(lua, &name, &wrapper)?;
-        // First name wins, as for frames; a sibling's `SetPoint` can then name this region.
+        // The region keeps its own name; the lookup a sibling's `SetPoint` names it by stays
+        // first-come, as `_G` is.
         let mut model = lua.app_data_mut::<Model>().expect("model");
-        model.region_names.entry(name).or_insert(id);
+        if let Some(rh) = model.id_to_region.get(&id).copied() {
+            crate::script::region::set_region_name(&mut model, rh, id, &name);
+        }
     }
     Ok(wrapper)
 }

crates/benilla-ui/src/script/region.rs

@@ -378,7 +378,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
         "SetPortraitToTexture",
         lua.create_function(|lua, (name, path): (String, String)| {
             let mut model = lua.app_data_mut::<Model>().expect("model");
-            let Some(id) = model.region_names.get(&name).copied() else {
+            let Some(id) = model.region_by_name.get(&name).copied() else {
                 return Ok(());
             };
             let Some(rh) = model.id_to_region.get(&id).copied() else {
@@ -632,25 +632,33 @@ fn install_region_methods(lua: &Lua) -> mlua::Result<()> {
     Ok(())
 }
 
-/// This region's global name, or `None` when anonymous: a scan of the region-name registry, the
-/// one authority, linear in named regions.
+/// This region's own name, or `None` when anonymous: the string on the region (`[+0x98]`, read by
+/// `GetName` through vtable `+4`, `0x46ff70`), whether or not it holds the lookup entry.
 pub(super) fn region_name_of(model: &Model, id: u32) -> Option<String> {
-    model
-        .region_names
-        .iter()
-        .find(|&(_, &v)| v == id)
-        .map(|(k, _)| k.clone())
+    let rh = *model.id_to_region.get(&id)?;
+    model.arena.region(rh)?.name.clone()
 }
 
-/// Publish a region's name into the registry a named `relativeTo` resolves through, first name
-/// winning as for frames. For sub-textures a setter creates and the XML loader names later
-/// (`SetThumbTexture`, `SetColorWheelTexture`); `CreateTexture(name)` publishes its own.
+/// `CScriptRegion::SetName` (`0x76c650`): the region keeps its own copy of the (already expanded)
+/// name, replacing the old one, and offers it to the name lookup, where the first holder stays
+/// (`_G`'s `0x701bd0` writes only when the entry is empty, `0x701c8a`).
+pub(crate) fn set_region_name(model: &mut Model, rh: RegionHandle, id: u32, name: &str) {
+    if let Some(region) = model.arena.region_mut(rh) {
+        region.name = Some(name.to_string());
+    }
+    model.region_by_name.entry(name.to_string()).or_insert(id);
+}
+
+/// Name a region the loader or a setter built nameless or under another name, as
+/// [`set_region_name`]. For sub-textures a setter creates and the XML loader names later
+/// (`SetThumbTexture`, `SetColorWheelTexture`); `CreateTexture(name)` names its own.
 pub(crate) fn publish_region_name(lua: &Lua, name: &str, region: &Table) {
-    let Ok(id) = super::object::decode_id(region) else {
+    let Ok(rh) = region_handle_of(lua, region) else {
         return;
     };
     let mut model = lua.app_data_mut::<Model>().expect("model app_data");
-    model.region_names.entry(name.to_string()).or_insert(id);
+    let id = model.region_id(rh);
+    set_region_name(&mut model, rh, id, name);
 }
 
 /// The type `GetObjectType()` answers and `IsObjectType` matches; `"Region"` for a dead handle.
@@ -670,9 +678,16 @@ pub(super) fn region_type_name(model: &Model, rh: RegionHandle) -> &'static str
 pub(crate) fn free_region(model: &mut Model, rh: RegionHandle) {
     model.region_data.remove(&rh);
     model.region_resolved.remove(&rh);
+    let name = model.arena.region(rh).and_then(|r| r.name.clone());
     if let Some(id) = model.region_to_id.remove(&rh) {
         model.id_to_region.remove(&id);
-        model.region_names.retain(|_, v| *v != id);
+        // The lookup entry goes only if this region is the one it holds; a later holder of the
+        // same name was never in it.
+        if let Some(name) = name {
+            if model.region_by_name.get(&name) == Some(&id) {
+                model.region_by_name.remove(&name);
+            }
+        }
         // Its node leaves the cached graph with its edges at the next resolve, and whatever
         // anchored to it re-resolves without it.
         model.name_layout_node(id);
@@ -759,13 +774,11 @@ pub(crate) fn implicit_creation_anchor_lua(lua: &Lua, wrapper: &Table) -> mlua::
 /// The receiver's name for error strings and what a leading `$parent` expands to, the owner frame
 /// (`+0x9c`), read under a borrow dropped before the ladder reads `_G`.
 pub(super) fn region_ladder_context(lua: &Lua, rh: RegionHandle) -> (String, String) {
-    let mut model = lua.app_data_mut::<Model>().expect("model");
-    let id = model.region_id(rh);
+    let model = lua.app_data_mut::<Model>().expect("model");
     let who = model
-        .region_names
-        .iter()
-        .find(|(_, &v)| v == id)
-        .map(|(n, _)| n.clone())
+        .arena
+        .region(rh)
+        .and_then(|r| r.name.clone())
         .unwrap_or_else(|| UNNAMED.to_string());
     let owner = model.arena.region(rh).map(|r| r.owner);
     let base = super::object::parent_token_base(&model, owner);

crates/benilla-ui/src/script/tests/name_targets.rs

@@ -258,3 +258,122 @@ fn a_constructor_name_expands_parent() {
     s.run(r#"Icon:SetPoint("TOPLEFT", "RootKid", "TOPLEFT", 0, 0)"#)
         .unwrap();
 }
+
+/// A region's name lives on the region (`CScriptRegion::SetName 0x76c650` stores its own copy at
+/// `[+0x98]`); only the `_G` entry is first-come (`0x701bd0` writes it only when empty).
+#[test]
+fn two_regions_with_one_name_both_answer_it_and_g_keeps_the_first() {
+    let s = script();
+    s.run(
+        r#"
+        Host = CreateFrame("Frame", "NameHost")
+        First = Host:CreateTexture("SharedRegionName")
+        Second = Host:CreateTexture("SharedRegionName")
+        Third = Host:CreateFontString("SharedRegionName")
+        "#,
+    )
+    .unwrap();
+    for r in ["First", "Second", "Third"] {
+        assert_eq!(
+            s.eval::<Option<String>>(&format!("return {r}:GetName()"))
+                .unwrap()
+                .as_deref(),
+            Some("SharedRegionName"),
+            "{r} keeps the name it was given"
+        );
+    }
+    assert!(
+        s.eval::<bool>(r#"return SharedRegionName == First and SharedRegionName ~= Second"#)
+            .unwrap(),
+        "_G holds the first region only"
+    );
+}
+
+/// The `$parent` expansion reads the owner frame, so two owners that share a name give their
+/// regions one expanded name; both answer it.
+#[test]
+fn parent_named_regions_under_colliding_parents_both_answer_their_name() {
+    let s = script();
+    s.run(
+        r#"
+        PA = CreateFrame("Frame", "CollidingParent")
+        PB = CreateFrame("Frame", "CollidingParent")
+        TextA = PA:CreateFontString("$parentText")
+        TextB = PB:CreateFontString("$parentText")
+        "#,
+    )
+    .unwrap();
+    for r in ["TextA", "TextB"] {
+        assert_eq!(
+            s.eval::<Option<String>>(&format!("return {r}:GetName()"))
+                .unwrap()
+                .as_deref(),
+            Some("CollidingParentText"),
+            "{r} answers its expanded name"
+        );
+    }
+}
+
+/// The receiver in an error string is the region's own name, taken or not.
+#[test]
+fn the_error_string_of_a_region_with_a_taken_name_uses_its_name() {
+    let s = script();
+    s.run(
+        r#"
+        Host = CreateFrame("Frame", "ErrHost")
+        First = Host:CreateTexture("ErrRegionName")
+        Second = Host:CreateTexture("ErrRegionName")
+        "#,
+    )
+    .unwrap();
+    for r in ["First", "Second"] {
+        let err = s
+            .run(&format!(r#"{r}:SetPoint()"#))
+            .expect_err("SetPoint with no arguments raises")
+            .to_string();
+        assert!(
+            err.contains("Usage: ErrRegionName:SetPoint("),
+            "{r}'s error names the region: {err}"
+        );
+        let err = s
+            .run(&format!("{r}:IsObjectType(nil)"))
+            .expect_err("IsObjectType(nil) raises")
+            .to_string();
+        assert!(
+            err.contains(r#"Usage: ErrRegionName:IsObjectType("TYPE")"#),
+            "{r}'s error names the region: {err}"
+        );
+    }
+}
+
+/// A freed region takes its lookup entry with it only when the entry is its own: the first
+/// holder going leaves a later same-named region's `GetName` alone.
+#[test]
+fn freeing_a_region_leaves_a_same_named_siblings_name() {
+    let s = script();
+    s.run(
+        r#"
+        Host = CreateFrame("Frame", "FreeHost")
+        First = Host:CreateTexture("FreedRegionName")
+        Second = Host:CreateTexture("FreedRegionName")
+        "#,
+    )
+    .unwrap();
+    {
+        let lua = s.lua();
+        let mut model = lua.app_data_mut::<crate::script::Model>().expect("model");
+        let id = *model.region_by_name.get("FreedRegionName").expect("held");
+        let rh = *model.id_to_region.get(&id).expect("first region");
+        crate::script::region::free_region(&mut model, rh);
+        assert!(
+            !model.region_by_name.contains_key("FreedRegionName"),
+            "the freed holder's entry is gone"
+        );
+    }
+    assert_eq!(
+        s.eval::<Option<String>>("return Second:GetName()")
+            .unwrap()
+            .as_deref(),
+        Some("FreedRegionName")
+    );
+}

crates/benilla-ui/src/script/tests/regions.rs

@@ -549,7 +549,7 @@ fn desaturation_rides_the_extract_for_art_and_never_for_a_solid() {
 fn desaturated(s: &UiScript, name: &str) -> bool {
     let lua = s.lua();
     let model = lua.app_data_ref::<crate::script::Model>().expect("model");
-    let id = *model.region_names.get(name).expect("region name");
+    let id = *model.region_by_name.get(name).expect("region name");
     let h = *model.id_to_region.get(&id).expect("region handle");
     model.region_data.get(&h).is_some_and(|d| d.desaturated)
 }
@@ -996,7 +996,7 @@ fn the_constructors_string_arguments_are_four_shapes_not_one() {
     let layer_of = |s: &UiScript, name: &str| {
         let lua = s.lua();
         let model = lua.app_data_ref::<crate::script::Model>().expect("model");
-        let id = *model.region_names.get(name).expect("published region");
+        let id = *model.region_by_name.get(name).expect("published region");
         let rh = *model.id_to_region.get(&id).expect("live region");
         model.arena.region(rh).expect("region").draw_layer
     };

crates/benilla-ui/src/script/tooltip/mod.rs

@@ -110,7 +110,7 @@ fn ensure_lines(lua: &Lua, this: &Table, n: usize) -> mlua::Result<()> {
                     return None;
                 }
                 let id = model
-                    .region_names
+                    .region_by_name
                     .get(&format!("{frame_name}Text{cell}{i}"))
                     .copied()?;
                 let rh = model.id_to_region.get(&id).copied()?;
@@ -228,12 +228,12 @@ fn ensure_lines(lua: &Lua, this: &Table, n: usize) -> mlua::Result<()> {
                 // Adopted pairs are already named and published by the loader.
                 if adopted_left.is_none() {
                     let ln = format!("{frame_name}TextLeft{i}");
-                    model.region_names.entry(ln.clone()).or_insert(left_id);
+                    crate::script::region::set_region_name(&mut model, left, left_id, &ln);
                     publish.push((left_id, ln));
                 }
                 if adopted_right.is_none() {
                     let rn = format!("{frame_name}TextRight{i}");
-                    model.region_names.entry(rn.clone()).or_insert(right_id);
+                    crate::script::region::set_region_name(&mut model, right, right_id, &rn);
                     publish.push((right_id, rn));
                 }
             }

crates/benilla-ui/src/widget/mod.rs

@@ -234,6 +234,10 @@ pub struct Region {
     /// `Region:SetParent(nil)`: unlinked and not drawn, but not destroyed (`0x77fd10` with a null
     /// parent). The owner's `regions` keeps it so `destroy` frees it; `SetParent(frame)` relinks.
     pub detached: bool,
+    /// The region's own name, the expanded string `SetName` keeps at `[+0x98]` (`0x76c650`); `None`
+    /// while anonymous. Only the `_G` entry is first-come (`0x701bd0`), so two regions can hold
+    /// one name, and `GetName` answers each its own.
+    pub name: Option<String>,
 }
 
 /// A cursor over a frame's child list for [`WidgetArena::next_child`]: the child last returned and
@@ -697,6 +701,7 @@ impl WidgetArena {
             sub_level,
             decl_seq,
             detached: false,
+            name: None,
         });
         let handle = RegionHandle { index, generation };
         self.frame_mut(owner)