A <BarTexture name=…> never reached _G, which broke crafting in ATSW2
`apply_statusbar` read `file=` and `<Color>` off `<BarTexture>` and passed them to `SetStatusBarTexture`, which builds the fill region itself. Every other attribute of the element went unread, `name=` among them. In the reference `<BarTexture>` is a Texture element and StatusBar's LoadXML (`0x782ef0`) builds the fill through the shared texture ctor, which registers the name like any other region's. A dropped `name=` is invisible until Lua reaches for the global, and then it raises inside the addon. ATSW2's `SPELLCAST_START` arm shows the progress bar, tints it per ore through `ATSWProgressBarTexture` — the raise — and only then calls `ATSW_StartProcessing`. So every craft left the bar shown and running with nothing able to stop it: `Processing.Active` stayed false, `SPELLCAST_INTERRUPTED` was never registered, `ATSW_StopProcessing` and `ATSW_CompleteTask` were never reached, and the guard `not Processing.Active` let the next craft re-raise. grom's log has seven in a row and his screenshot has the stuck bar. Named now, both ways, as Button state textures already were: the region registry for `GetName()` and `_G` for `getglobal` and a sibling's `relativeTo`. A named `<BarTexture>` with no paint still builds its region. Four of the five in this install declare a name (`ATSWProgressBarTexture`, `ATSWRankFrameBar`, two QuickHeal bars); no stock FrameXML status bar names its fill, so the base interface neither gains a global nor collides with one. Also in the skin: the reagent names under the icons take the same black-to-white `SetTextColor` wrapper as the tools line. They measured 1/1/1 in a capture and read as correct, but a capture has no item links and `ATSW_ShowRecipe` paints the title black only inside `if RLink then` — in play it runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
@@ -754,7 +754,7 @@ impl Loader<'_> {
|
||||
self.apply_title_region(el, wrapper, self_name, dbg_name);
|
||||
// 5b · per-kind LoadXML extras (EditBox `0x779fb0`), each gated on the element's own tag,
|
||||
// so a Frame wearing a <Button> template skips the Button steps.
|
||||
self.apply_statusbar(el, wrapper, dbg_name);
|
||||
self.apply_statusbar(el, wrapper, self_name, dbg_name);
|
||||
self.apply_slider(el, wrapper, self_name, dbg_name);
|
||||
self.apply_colorselect(el, wrapper, self_name, dbg_name);
|
||||
self.apply_button(el, wrapper, self_name, dbg_name);
|
||||
|
||||
@@ -1207,6 +1207,50 @@ mod loader_tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// A `<BarTexture name=…>` is a named region like any other: the reference builds the fill
|
||||
/// through the shared texture ctor, so the name reaches `_G` and `GetName()`. ATSW2's progress
|
||||
/// bar reads the global (`AdvancedTradeSkillWindow2.lua:2673`) and raised on every craft.
|
||||
#[test]
|
||||
fn a_named_bar_texture_publishes_its_name() {
|
||||
let mut s = UiScript::new().unwrap();
|
||||
s.set_screen_size(800.0, 600.0);
|
||||
let doc = parse(
|
||||
r#"<Ui>
|
||||
<StatusBar name="NamedBar">
|
||||
<Size><AbsDimension x="100" y="10"/></Size>
|
||||
<Anchors><Anchor point="BOTTOMLEFT"/></Anchors>
|
||||
<BarTexture name="$parentTexture" file="Interface\PaperDollInfoFrame\UI-Character-Skills-Bar"/>
|
||||
</StatusBar>
|
||||
<StatusBar name="BareBar">
|
||||
<Size><AbsDimension x="100" y="10"/></Size>
|
||||
<Anchors><Anchor point="TOPLEFT"/></Anchors>
|
||||
<BarTexture file="Interface\TargetingFrame\UI-StatusBar"/>
|
||||
</StatusBar>
|
||||
</Ui>"#,
|
||||
);
|
||||
let report = load(&s, &doc, &no_files);
|
||||
assert!(report.errors.is_empty(), "errors: {:?}", report.errors);
|
||||
|
||||
let ok: bool = s
|
||||
.eval(
|
||||
r#"
|
||||
return NamedBarTexture ~= nil
|
||||
and NamedBarTexture == NamedBar:GetStatusBarTexture()
|
||||
and getglobal("NamedBarTexture"):GetName() == "NamedBarTexture"
|
||||
and BareBar:GetStatusBarTexture() ~= nil
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
assert!(
|
||||
ok,
|
||||
"$parentTexture reaches _G and GetName(); a nameless one still builds"
|
||||
);
|
||||
|
||||
// The call that was raising: the fill takes a vertex colour through the global.
|
||||
s.eval::<()>("NamedBarTexture:SetVertexColor(0.72, 0.45, 0.20)")
|
||||
.expect("SetVertexColor on the named fill");
|
||||
}
|
||||
|
||||
/// An anchorless `<Texture>` gets `SetAllPoints` after its LoadXML (`0x7701c0`), so its
|
||||
/// `<Size>` goes unread: `StackSplitFrame.xml:10` authors 256×32 and draws 172×96.
|
||||
#[test]
|
||||
|
||||
@@ -11,7 +11,13 @@ use super::{
|
||||
impl Loader<'_> {
|
||||
/// `<StatusBar>` (`0x782ef0`): a reversed `minValue`/`maxValue` pair is swapped, as
|
||||
/// `SetMinMaxValues` also does; `drawLayer` is the bar texture's layer, ARTWORK by default.
|
||||
pub(super) fn apply_statusbar(&mut self, el: &Element, wrapper: &Table, dbg: &str) {
|
||||
pub(super) fn apply_statusbar(
|
||||
&mut self,
|
||||
el: &Element,
|
||||
wrapper: &Table,
|
||||
self_name: &str,
|
||||
dbg: &str,
|
||||
) {
|
||||
if !el.tag.eq_ignore_ascii_case("StatusBar") {
|
||||
return;
|
||||
}
|
||||
@@ -47,7 +53,19 @@ impl Loader<'_> {
|
||||
(c[0], c[1], c[2], c[3]),
|
||||
dbg,
|
||||
);
|
||||
} else {
|
||||
// Neither paint: the object still exists, so `""` builds the region and paints
|
||||
// nothing. Only a named one is worth building — nothing else can reach it.
|
||||
if bar.name().is_some() {
|
||||
self.call(
|
||||
wrapper,
|
||||
"SetStatusBarTexture",
|
||||
(String::new(), layer.clone()),
|
||||
dbg,
|
||||
);
|
||||
}
|
||||
}
|
||||
self.name_bar_texture(bar, wrapper, self_name, dbg);
|
||||
}
|
||||
for bc in children_named(el, "BarColor") {
|
||||
let c = color_of(bc);
|
||||
@@ -58,6 +76,41 @@ impl Loader<'_> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Publish a `<BarTexture name=…>` under its name, both ways: the region registry for
|
||||
/// `GetName()` and `_G` for `getglobal` and a sibling's `relativeTo`.
|
||||
///
|
||||
/// `<BarTexture>` is a Texture element in the schema, and the reference's StatusBar `LoadXML`
|
||||
/// (`0x782ef0`) builds the fill through the shared texture ctor, which registers the name like
|
||||
/// any other region's. benilla created the fill from inside `SetStatusBarTexture` and dropped
|
||||
/// the attribute, so the global was nil. ATSW2's progress bar is the case: it declares
|
||||
/// `name="$parentTexture"` and tints the fill per ore through the global
|
||||
/// `ATSWProgressBarTexture` on `SPELLCAST_START`
|
||||
/// (`AdvancedTradeSkillWindow2.lua:2673`). The raise landed between
|
||||
/// `ATSW_InitializeProgressBar` and `ATSW_StartProcessing`, so every craft left the bar
|
||||
/// *shown and running* with nothing left to stop it — `Processing.Active` stayed false,
|
||||
/// `SPELLCAST_INTERRUPTED` was never registered and `ATSW_StopProcessing` never ran.
|
||||
fn name_bar_texture(&mut self, bar: &Element, wrapper: &Table, self_name: &str, dbg: &str) {
|
||||
let Some(rname) = bar.name().map(|raw| framexml::resolve_name(raw, self_name)) else {
|
||||
return;
|
||||
};
|
||||
let region: Table = match wrapper.call_method("GetStatusBarTexture", ()) {
|
||||
Ok(Some(r)) => r,
|
||||
Ok(None) => return,
|
||||
Err(e) => {
|
||||
self.report
|
||||
.errors
|
||||
.push(format!("{dbg}: GetStatusBarTexture: {e}"));
|
||||
return;
|
||||
}
|
||||
};
|
||||
crate::script::region::publish_region_name(self.lua(), &rname, ®ion);
|
||||
if let Err(e) = self.lua().globals().set(rname.clone(), region) {
|
||||
self.report
|
||||
.warnings
|
||||
.push(format!("{dbg}: bar-texture global '{rname}': {e}"));
|
||||
}
|
||||
}
|
||||
|
||||
/// `<Button>`, `<CheckButton>` and `<LootButton>` (`0x7788c0`, which `0x785170` runs first):
|
||||
/// the label, the state textures with their own layout, `checked`, `text=` and the per-state
|
||||
/// fonts. `$parent` in a child's anchors is this button.
|
||||
|
||||
@@ -4651,3 +4651,98 @@ a perfectly good trade-skill window was on screen — version 1's. `BENILLA_HOME
|
||||
`AddOns/` of symlinks to every folder but `AdvancedTradeSkillWindow`, and a `saved` symlink beside
|
||||
it for `WOW_CAPTURE_SAVED`, gives a deterministic version-2 run without touching the player's
|
||||
install.
|
||||
|
||||
## A `<BarTexture name=…>` never reached `_G`, and it broke crafting
|
||||
|
||||
grom, with the log and two screenshots: *"saw a bunch of errors when actually crafting"* and
|
||||
*"crafting in progress bar also didn't go away when completed."* One defect, ours, and the second
|
||||
symptom is a consequence of the first.
|
||||
|
||||
The log carries the same raise once per craft, seven in a row three seconds apart:
|
||||
|
||||
```
|
||||
ui_script: runtime error: ...AdvancedTradeSkillWindow2.lua:2673:
|
||||
attempt to index global `ATSWProgressBarTexture' (a nil value)
|
||||
stack traceback:
|
||||
...AdvancedTradeSkillWindow2.lua:2691: in function 'ATSW_OnEvent'
|
||||
```
|
||||
|
||||
### Where the name was lost
|
||||
|
||||
`<BarTexture>` is a Texture element in the schema, and the reference's StatusBar `LoadXML`
|
||||
(`0x782ef0`) builds the fill through the shared texture constructor — which registers the name like
|
||||
any other region's. benilla instead built the fill from inside `SetStatusBarTexture` and read only
|
||||
`file=` and `<Color>` off the element, so `name=` was dropped on the floor. Four of the five
|
||||
`<BarTexture>` elements in this install declare one:
|
||||
|
||||
```
|
||||
ATSWProgressBar <BarTexture name="$parentTexture" …> ATSWProgressBarTexture
|
||||
ATSWRankFrame <BarTexture name="$parentBar" …> ATSWRankFrameBar
|
||||
QuickHealHealingBar <BarTexture name="QuickHealHealingBarTexture" …>
|
||||
QuickHealHealingBarPost …
|
||||
```
|
||||
|
||||
Named now, both ways, as a Button's state textures already were: the region registry for
|
||||
`GetName()` and `_G` for `getglobal` and a sibling's `relativeTo`. A declared-but-paintless
|
||||
`<BarTexture name=…>` still builds its region, since the object exists in the reference whether or
|
||||
not it has art. No stock FrameXML status bar names its fill — `CastingBarFrame`, `MainMenuExpBar`,
|
||||
`ReputationWatchStatusBar`, the unit-frame health bars and `MirrorTimer1StatusBar` all answer
|
||||
`nil` — so nothing in the base interface gains or collides with a global.
|
||||
|
||||
### Why one nil global stopped the progress bar
|
||||
|
||||
ATSW2's `SPELLCAST_START` arm does four things in order
|
||||
(`AdvancedTradeSkillWindow2.lua:2641-2694`):
|
||||
|
||||
```lua
|
||||
ATSW_InitializeProgressBar(Processing.Delay, Processing.Delay + CastTime) -- shows it, Working = true
|
||||
ColorizeProgressBar() -- ATSWProgressBarTexture:SetVertexColor(R, G, B) <- raised
|
||||
Processing.Delay = GetTime() - Processing.Delay
|
||||
ATSW_StartProcessing() -- never ran
|
||||
```
|
||||
|
||||
So the bar was *initialised and shown*, then the raise cut the frame before anything that drives or
|
||||
ends it. `ATSW_StartProcessing` never set `Processing.Active`, never registered
|
||||
`SPELLCAST_INTERRUPTED`; `ATSW_StopProcessing` — the only thing that clears `Working` and calls
|
||||
`ATSWProgressBar:Hide()` — was therefore never reached. The queued task sat at `00:21` with its
|
||||
Stop button, exactly as in the screenshot. And because the arm is guarded by
|
||||
`if arg1 == Processing.Recipe and not Processing.Active`, a false `Active` let every subsequent
|
||||
craft re-enter and re-raise: that is the seven log lines.
|
||||
|
||||
The colour it was reaching for is ATSW2's smelting tint — the bar takes the ore's own colour, copper
|
||||
`0.72/0.45/0.20` through elementium `0.78/0.73/0.86`, and `1/0.82/0` for everything else.
|
||||
|
||||
### The reagent names under the icons
|
||||
|
||||
*"lets make the reagents text match the white so it is also readable."*
|
||||
|
||||
`ATSWReagentFont` sets no colour, and the table in the previous section measured
|
||||
`ATSWReagent1Title` at `1/1/1` — correct, and misleading. A capture has no server and so no item
|
||||
links, and `ATSW_ShowRecipe` paints the title only inside `if RLink then`:
|
||||
|
||||
```lua
|
||||
-- ButtonTitle:SetTextColor(R, G, B, A) -- Colored title
|
||||
ButtonTitle:SetTextColor(0, 0, 0, 1) -- Black title
|
||||
```
|
||||
|
||||
(line 4575, the addon's own quality-coloured version commented out above it). In play the link is
|
||||
there, the line runs, and the name goes black like the rest of the pane. The same `SetTextColor`
|
||||
wrapper the tools line uses answers it wherever it is called from. The lesson is the one
|
||||
[`benilla-invented-fixture-hid-the-bug`] keeps teaching: a measurement taken with the server's half
|
||||
of the data missing can read as a clean pass on exactly the thing that is broken.
|
||||
|
||||
### Measured after
|
||||
|
||||
```
|
||||
ATSWProgressBarTexture GetName() = ATSWProgressBarTexture == ATSWProgressBar:GetStatusBarTexture()
|
||||
SetVertexColor(0.72, 0.45, 0.20) -> GetVertexColor 0.72/0.45/0.20
|
||||
ATSWRankFrame fill name = ATSWRankFrameBar
|
||||
ATSWReagent1Title black -> 1/1/1 red stays 1/0/0
|
||||
ATSWTool1Text black -> 1/1/1 red stays 1/0/0
|
||||
```
|
||||
|
||||
A capture cannot open the trade-skill window without a server, so the rows, tool buttons and
|
||||
reagent buttons — which `ATSW_Show` builds behind an `InitializedOnShow` flag — have to be asked
|
||||
for directly: `pcall(ATSW_CreateListButtons)` from `WOW_CAPTURE_SETUP`. The skin's own
|
||||
`hooksecurefunc` on that function then applies the rest, which is the path the live client takes
|
||||
too.
|
||||
|
||||
Reference in New Issue
Block a user