A frame or region takes no name or id from its template (#839)
An instance with no `name=` took its template's name: `framexml::merge` laid the instance's attributes over the template's, so `name` and `id` carried through. Stock `UIOptionsFrame.xml`'s unnamed `<Button inherits="UIPanelCloseButton">` became the global `UIPanelCloseButton`, with `GetName()` answering it, and any unnamed addon frame did the same to its template's name. In 1.12.1, `PreLoadXML` (`0x769770`) reads `name` and `id` from the instance node before `LoadXML` walks the templates (`0x76985c`). The texture and font string factories also read `name` once, outside that walk (`0x6f2751`, `0x6f27e1`). So neither is ever inherited. The merge now drops the template's `name` and `id`. Tests cover an unnamed frame, a named frame (no template id), an unnamed texture and `CreateFrame` with a nil name. All four fail without the fix. Fixes #394 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
d0d1b5def9
commit
87706792cb
@@ -315,8 +315,10 @@ fn element_from_node(node: roxmltree::Node) -> Element {
|
||||
/// attributes override or extend the template's (case-insensitively). The reference merges
|
||||
/// nothing: `LoadXML` re-enters itself on the one object against each template, the most distant
|
||||
/// first, then the instance (`0x76985c`; `<Frames>` repeat the walk, `0x76a060`), and reads `name`
|
||||
/// and `id` from the instance alone (`PreLoadXML 0x769770`). Here `name` and `virtual` merge like
|
||||
/// any attribute. A cycle is skipped with a warning.
|
||||
/// and `id` from the instance alone (`PreLoadXML 0x769770`; a region's factory reads its `name`
|
||||
/// once, outside the walk, `0x6f2751`/`0x6f27e1`). So here `name` and `id` come from the element
|
||||
/// alone, and an unnamed instance stays unnamed; `virtual` merges like any attribute, read only at
|
||||
/// registration. A cycle is skipped with a warning.
|
||||
pub fn expand(
|
||||
element: &Element,
|
||||
templates: &HashMap<&str, &Element>,
|
||||
@@ -390,9 +392,15 @@ fn expand_inner(
|
||||
}
|
||||
}
|
||||
|
||||
/// [`expand`]'s merge; `over`'s tag wins, and its body when it has one.
|
||||
/// [`expand`]'s merge; `over`'s tag wins, and its body when it has one. `base`'s `name` and `id`
|
||||
/// are dropped: they belong to the node they are written on.
|
||||
fn merge(base: &Element, over: &Element) -> Element {
|
||||
let mut attrs = base.attrs.clone();
|
||||
let mut attrs: Vec<(String, String)> = base
|
||||
.attrs
|
||||
.iter()
|
||||
.filter(|(k, _)| !k.eq_ignore_ascii_case("name") && !k.eq_ignore_ascii_case("id"))
|
||||
.cloned()
|
||||
.collect();
|
||||
for (k, v) in &over.attrs {
|
||||
if let Some(slot) = attrs.iter_mut().find(|(ek, _)| ek.eq_ignore_ascii_case(k)) {
|
||||
slot.1 = v.clone();
|
||||
|
||||
@@ -898,5 +898,7 @@ pub(super) fn font_object_from_element(el: &Element) -> FontObject {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod template_name_tests;
|
||||
#[cfg(test)]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
//! `name=` and `id=` belong to the node they are written on: `PreLoadXML 0x769770` reads them from
|
||||
//! the instance before `LoadXML` walks its templates (`0x76985c`), and the region factories read
|
||||
//! `name` once, outside that walk (`0x6f2751`, `0x6f27e1`).
|
||||
|
||||
use crate::framexml;
|
||||
use crate::loader::load;
|
||||
use crate::script::UiScript;
|
||||
|
||||
const DOC: &str = r#"<Ui>
|
||||
<Button name="TmpCloseTemplate" id="3" virtual="true">
|
||||
<Size><AbsDimension x="32" y="32"/></Size>
|
||||
</Button>
|
||||
<Texture name="TmpTexTemplate" virtual="true">
|
||||
<Color r="1" g="0" b="0" a="1"/>
|
||||
</Texture>
|
||||
<Frame name="Holder">
|
||||
<Size><AbsDimension x="64" y="64"/></Size>
|
||||
<Anchors><Anchor point="CENTER"/></Anchors>
|
||||
<Layers>
|
||||
<Layer level="ARTWORK">
|
||||
<Texture inherits="TmpTexTemplate"/>
|
||||
</Layer>
|
||||
</Layers>
|
||||
<Frames>
|
||||
<Button inherits="TmpCloseTemplate"/>
|
||||
<Button name="$parentClose" inherits="TmpCloseTemplate"/>
|
||||
</Frames>
|
||||
</Frame>
|
||||
</Ui>"#;
|
||||
|
||||
fn loaded() -> UiScript {
|
||||
let mut s = UiScript::new().unwrap();
|
||||
s.set_screen_size(1024.0, 768.0);
|
||||
let report = load(&s, &framexml::parse(DOC).unwrap(), &|_| None);
|
||||
assert!(report.errors.is_empty(), "{:?}", report.errors);
|
||||
s
|
||||
}
|
||||
|
||||
/// Stock `UIOptionsFrame.xml:1477`'s unnamed `<Button inherits="UIPanelCloseButton">` is the
|
||||
/// shape: in 1.12.1 the template's name is no global and the button answers no name.
|
||||
#[test]
|
||||
fn an_unnamed_instance_takes_neither_its_templates_name_nor_its_id() {
|
||||
let s = loaded();
|
||||
assert!(
|
||||
s.eval::<bool>("return TmpCloseTemplate == nil").unwrap(),
|
||||
"the unnamed instance published the template's name as a global"
|
||||
);
|
||||
assert!(s
|
||||
.eval::<bool>("return ({Holder:GetChildren()})[1]:GetName() == nil")
|
||||
.unwrap());
|
||||
assert_eq!(
|
||||
s.eval::<i64>("return ({Holder:GetChildren()})[1]:GetID()")
|
||||
.unwrap(),
|
||||
0
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_named_instance_keeps_its_own_name_and_takes_no_template_id() {
|
||||
let s = loaded();
|
||||
assert_eq!(
|
||||
s.eval::<String>("return HolderClose:GetName()").unwrap(),
|
||||
"HolderClose"
|
||||
);
|
||||
assert_eq!(s.eval::<i64>("return HolderClose:GetID()").unwrap(), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unnamed_region_takes_no_name_from_its_template() {
|
||||
let s = loaded();
|
||||
assert!(s.eval::<bool>("return TmpTexTemplate == nil").unwrap());
|
||||
assert!(s
|
||||
.eval::<bool>("return ({Holder:GetRegions()})[1]:GetName() == nil")
|
||||
.unwrap());
|
||||
}
|
||||
|
||||
/// `CreateFrame` builds its node with the `name` it was given (`0x70622d`), nil included.
|
||||
#[test]
|
||||
fn create_frame_with_no_name_stays_unnamed_under_a_named_template() {
|
||||
let s = loaded();
|
||||
assert!(s
|
||||
.eval::<bool>(
|
||||
"local b = CreateFrame('Button', nil, Holder, 'TmpCloseTemplate') \
|
||||
return b:GetName() == nil and TmpCloseTemplate == nil"
|
||||
)
|
||||
.unwrap());
|
||||
}
|
||||
Reference in New Issue
Block a user