A second profession's opener updated the book instead of reopening it

grom: "nothing happens when I click another tab." Advanced Trade Skill
Window's side tabs cast the other profession's opener and rebuild on
TRADE_SKILL_SHOW alone — its update arm is `UpdateCount = UpdateCount + 1`
and nothing else — so the book swapped underneath it and nothing redrew.

The open is `0x4fc910` and it carries three rules we had one of:

* one event per *open*, not per change: a changed line while a book is up
  is a fresh TRADE_SKILL_SHOW (`0x4fc99f`), never an update. The update is
  a change *within* one open book — a filter, a fold, an item count.
* the opener is the window's toggle (`0x4fc918`): casting the profession
  already open closes it, which is what ATSW2's `this.Name ~= <current>`
  guard exists to avoid tripping.
* an empty rebuilt list signals nothing (`0x4fc997`, the count at
  `0xbde04c`): the line is open and no window appears. Written over the
  list about to be pushed rather than as the literal count read, because
  ours can be a frame late where the reference builds its list inside the
  open call — a book that arrives empty and fills next frame still fires
  exactly one show, on the frame it fills.

`0x4f6000` is the Craft window's same function, keyed on the craft type at
`0xbdcfb8` and with no empty guard; both of its rules land in `feed_craft`
and `open_trade_skill` too, since CRAFT_SHOW is the arm ATSW2 takes for
Enchanting. The filter reset on a line change was already right —
`set_trade_skill` keys it on `trade_skill_last_line`, the `0xbde064` above.

Verified end to end offline with $WOW_CAPTURE_SPELLS seating a real book:
Blacksmithing open with 24 recipes, `ATSWFrameTab3:Click()`, and the window
comes back title=Cooking line=Cooking rows=9 with the check moved to tab 3.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
2026-11-05 16:28:29 -07:00
co-authored by Claude Opus 5
parent a033f13f0f
commit 6bcf937986
3 changed files with 328 additions and 13 deletions
+14
View File
@@ -238,11 +238,25 @@ fn feed_craft(
.flat_map(|r| r.reagents.iter().map(|re| re.item)),
);
}
// As on the trade-skill side (`ui_tradeskill::feed_trade_skill`): the open fires one event
// per *open* (`0x4f6040`), so a different craft type while a window is up is a fresh
// `CRAFT_SHOW`, not an update. `CRAFT_UPDATE` is a change within one open window. The craft
// open has no empty-list guard — `0x4f601c` signals unconditionally, where the skill line's
// tests its count first.
let type_changed = last.as_ref().map(|f| f.craft_type) != fresh.as_ref().map(|f| f.craft_type);
match (&*last, &fresh) {
(None, Some(f)) => {
debug!("ui_craft: window opens — {} recipe(s)", f.recipes.len());
script.fire_event("CRAFT_SHOW", vec![]);
}
(Some(_), Some(f)) if type_changed => {
debug!(
"ui_craft: window opens on craft type {} — {} recipe(s)",
f.craft_type,
f.recipes.len()
);
script.fire_event("CRAFT_SHOW", vec![]);
}
(Some(_), Some(_)) => script.fire_event("CRAFT_UPDATE", vec![]),
(Some(_), None) => {
debug!("ui_craft: window closes");
+175 -13
View File
@@ -145,17 +145,34 @@ fn open_trade_skill(
.and_then(|s| s.catalog.get(spell_id))
.and_then(|d| u32::try_from(d.effect_misc_value[0]).ok())
.unwrap_or(0);
// The opener is the window's **toggle**: the open's own first test (`0x4fc918` for a
// skill line, `0x4f6000` for a craft) compares the request against what is open and
// closes on a match, firing `TRADE_SKILL_CLOSE` / `CRAFT_CLOSE` rather than reopening.
// Casting your own open profession a second time shuts the book. Advanced Trade Skill
// Window's side tab guards exactly this (`this.Name ~= <current>` before it casts), which
// is how the behaviour reads off an addon as well as off the binary. The craft side keys
// on the craft type, not the line, because `0xbdcfb8` is what it stores and compares.
if craft_type != 0 {
if craft_open.craft_type == craft_type && craft_open.line.is_some() {
debug!("ui_tradeskill: opener {spell_id} re-cast on the open craft — closes");
craft_open.line = None;
craft_open.craft_type = 0;
continue;
}
debug!(
"ui_tradeskill: opener {spell_id} opens the CraftFrame (line {line}, craft type {craft_type})"
);
craft_open.line = Some(line);
craft_open.craft_type = craft_type;
} else {
debug!("ui_tradeskill: opener {spell_id} opens skill line {line}");
if open.line != Some(line) {
if open.line == Some(line) {
debug!("ui_tradeskill: opener {spell_id} re-cast on open line {line} — closes");
open.line = None;
repeat.clear();
continue;
}
debug!("ui_tradeskill: opener {spell_id} opens skill line {line}");
repeat.clear();
open.line = Some(line);
}
}
@@ -318,6 +335,39 @@ fn resolve_recipe(
})
}
/// Which event one feed step owes the VM, from the book it had against the book it now has.
///
/// The open (`0x4fc910`) fires one event per **open**, not one per change: a different line while
/// a book is already open is a fresh `TRADE_SKILL_SHOW` (`0x4fc99f`), never an update.
/// `TRADE_SKILL_UPDATE` is for a change *within* one open book — a filter, a fold, an item count.
/// The client also withholds the show when the rebuilt list came back empty (`0x4fc997`, over the
/// count at `0xbde04c`): the line is open and no window appears.
///
/// Advanced Trade Skill Window is the addon that tells the two apart. Its side tabs cast the other
/// profession's opener (`CastSpellByName(this.Name)`) and it rebuilds on `TRADE_SKILL_SHOW` alone
/// — its update arm increments a counter and does nothing else. With the update this used to send,
/// a tab click swapped the book underneath it and redrew nothing, which is what "I can't switch to
/// other professions" looked like.
///
/// The emptiness test is written over the list we are about to push rather than as the reference's
/// literal count read, because ours can be a frame late where the reference builds its list inside
/// the open call: a book that arrives empty and fills next frame still fires exactly one show, on
/// the frame it fills.
fn book_event(
last: &Option<TradeSkillState>,
fresh: &Option<TradeSkillState>,
) -> Option<&'static str> {
let shown = |s: &Option<TradeSkillState>| s.as_ref().is_some_and(|f| !f.recipes.is_empty());
let line_of = |s: &Option<TradeSkillState>| s.as_ref().map(|f| f.line);
if fresh.is_none() {
return last.is_some().then_some("TRADE_SKILL_CLOSE");
}
if shown(fresh) && (!shown(last) || line_of(last) != line_of(fresh)) {
return Some("TRADE_SKILL_SHOW");
}
last.is_some().then_some("TRADE_SKILL_UPDATE")
}
/// Build the book: the known recipes of the open line, banded against the current rank.
fn feed_trade_skill(
script: Option<NonSendMut<UiScript>>,
@@ -406,23 +456,29 @@ fn feed_trade_skill(
std::iter::once(r.product_item).chain(r.reagents.iter().map(|re| re.item))
}));
}
match (&*last, &fresh) {
(None, Some(f)) => {
debug!(
"ui_tradeskill: window opens — {} recipe(s), first group {:?}",
f.recipes.len(),
f.recipes.first().and_then(|r| r.group.clone())
);
match book_event(last, &fresh) {
Some("TRADE_SKILL_SHOW") => {
if let Some(f) = &fresh {
debug!(
"ui_tradeskill: window opens on line {} — {} recipe(s), first group {:?}",
f.line,
f.recipes.len(),
f.recipes.first().and_then(|r| r.group.clone())
);
}
script.fire_event("TRADE_SKILL_SHOW", vec![]);
}
(Some(_), Some(_)) => {
script.fire_event("TRADE_SKILL_UPDATE", vec![]);
Some("TRADE_SKILL_CLOSE") => {
debug!("ui_tradeskill: window closes");
script.fire_event("TRADE_SKILL_CLOSE", vec![]);
}
Some(other) => {
script.fire_event(other, vec![]);
if repeat_changed {
script.fire_event("UPDATE_TRADESKILL_RECAST", vec![]);
}
}
(Some(_), None) => script.fire_event("TRADE_SKILL_CLOSE", vec![]),
(None, None) => {}
None => {}
}
*last = fresh;
}
@@ -579,6 +635,112 @@ mod tests {
assert_eq!(recipe_icon(&d, None, &deps.items, &deps.commands), None);
}
fn book(line: u32, recipes: usize) -> Option<TradeSkillState> {
Some(TradeSkillState {
line,
line_name: format!("Line {line}"),
rank: 1,
max_rank: 75,
recipes: (0..recipes)
.map(|i| TradeSkillRecipe {
group: None,
spell_id: 3000 + i as u32,
name: format!("Recipe {i}"),
difficulty: TradeSkillDifficulty::Optimal,
num_available: 1,
icon: None,
min_made: 1,
max_made: 1,
cooldown_secs: None,
product_item: 0,
product_inv_type: 0,
product_item_level: 0,
reagents: Vec::new(),
tools: Vec::new(),
})
.collect(),
repeat_count: 0,
})
}
/// The defect grom played: with the book open on one profession, clicking Advanced Trade
/// Skill Window's side tab for another casts that profession's opener, and the window used to
/// get `TRADE_SKILL_UPDATE` — which that addon counts and ignores. The reference signals an
/// open per open (`0x4fc99f`), so a changed line is a show.
#[test]
fn a_second_professions_opener_reopens_the_book_rather_than_updating_it() {
assert_eq!(
book_event(&book(164, 24), &book(185, 8)),
Some("TRADE_SKILL_SHOW"),
"Blacksmithing open, Cooking cast: a new book, so a new show",
);
assert_eq!(
book_event(&None, &book(164, 24)),
Some("TRADE_SKILL_SHOW"),
"the first open",
);
assert_eq!(
book_event(&book(164, 24), &book(164, 23)),
Some("TRADE_SKILL_UPDATE"),
"the same line with a changed list is a change within one open book",
);
assert_eq!(book_event(&book(164, 24), &None), Some("TRADE_SKILL_CLOSE"),);
assert_eq!(book_event(&None, &None), None);
}
/// `0x4fc997`: the show is signalled only when the rebuilt list came back non-empty — the
/// line is open either way. Ours can arrive a frame before the recipes do, so the show waits
/// for them rather than being skipped outright.
#[test]
fn an_empty_book_shows_nothing_until_it_fills() {
assert_eq!(book_event(&None, &book(164, 0)), None);
assert_eq!(
book_event(&book(164, 0), &book(164, 24)),
Some("TRADE_SKILL_SHOW"),
"one show, on the frame the list fills",
);
assert_eq!(
book_event(&book(164, 24), &book(164, 24)),
Some("TRADE_SKILL_UPDATE"),
"and not a second one",
);
}
/// `0x4fc918`, the open's own first test: the opener is the window's toggle, so casting the
/// profession that is already open shuts it. Advanced Trade Skill Window's side tab guards
/// exactly this before it casts (`this.Name ~= <current>`).
#[test]
fn the_open_professions_opener_closes_the_book() {
use bevy::ecs::system::RunSystemOnce;
let mut app = App::new();
app.insert_resource(SkillLines {
catalog: benilla_formats::SkillLineCatalog::from_spell_lines([(2018, 164)]),
});
app.init_resource::<TradeSkillOpens>()
.init_resource::<TradeSkillOpen>()
.init_resource::<crate::ui_craft::CraftOpen>()
.init_resource::<TradeSkillRepeat>();
app.world_mut()
.resource_mut::<TradeSkillOpens>()
.0
.push(2018);
app.world_mut().run_system_once(open_trade_skill).unwrap();
assert_eq!(app.world().resource::<TradeSkillOpen>().line, Some(164));
app.world_mut()
.resource_mut::<TradeSkillOpens>()
.0
.push(2018);
app.world_mut().run_system_once(open_trade_skill).unwrap();
assert_eq!(
app.world().resource::<TradeSkillOpen>().line,
None,
"the same opener a second time closes the book",
);
}
/// Driven through the real handler registration.
#[test]
fn the_session_end_closes_the_book_and_drops_the_repeat() {
+139
View File
@@ -4360,3 +4360,142 @@ His own session agrees: 11 `frame hitch:` lines in an hour, every one of them at
largest being the loading screens, and **zero** live pipeline compiles. So the thing to pin down
next is which "laggy" it is — a sustained cost, or motion trailing the input, which would be the
zoom ease's 0.29 s half-life rather than a frame cost.
## 2026-11-05, second session: every `<OnLoad>` in the interface read text widths as 0
grom, with ATSW2 finally loading: *"okay atsw2 is loading now, but barely functioning."* Its sort
row was the visible part — `Sort by: ◉ Catego⟨Difficulty⟩ry ◯ Name` — and chasing that one
overlapping label landed on something that had nothing to do with the addon.
`seat_text_measurer`'s own comment says why it exists: *"so `GetStringWidth` right after `SetText`
measures … seated before the manifest loads, **for every `<OnLoad>`**"*. It was not being seated.
`UiFontAtlas` is inserted by an `Update` system through `Commands`, so it appears at the next
command flush — and `load_ingame_ui_on_world_entry` is an exclusive system on the `InWorld` state
transition, which comes **first**. `seat_raster_seam_for_load` took its `let Some(atlas) = … else
{ return }`, and the VM loaded the whole interface with no measurer behind
`measure::ensure_measured`.
So for all of FrameXML and every addon, `FontString:GetWidth()`, `GetHeight()` and
`GetStringWidth()` answered **0** for a string that plainly had text. The reference measures inline
and always knows (`0x79e510` → `0x772890`). ATSW2's line 1642 is the case that showed:
```lua
ATSWNameSortCheckbox:SetPoint('LEFT', ATSWDifficultySortCheckbox, 'RIGHT',
ATSWDifficultySortCheckboxText2:GetWidth()+5, 0)
```
which stored **5.0** instead of 53.8, and drew "Difficulty" across "Category".
**Why it never looks like this.** Nothing errors, nothing logs, and by the time anything asks, the
widths are all correct — the measure is taken on the next read, after the load. The only reads that
are wrong are the ones taken *during* an `<OnLoad>`, and what they leave behind is a stored
geometry nobody recomputes. The three probes that named it, in order, each cheap:
1. Is it the geometry or the drawing? `GetPoint()` on the misplaced frame — a stored offset of 5
where the label measures 48.8 says the arithmetic ran on a zero.
2. Re-run the addon's own `<OnLoad>` from a capture eval (they are globals). Same code, same frame,
right answer ⇒ the difference is *when*, not *what*.
3. One `eprintln!` in `region::measured_wh` when a FontString with text answers 0, printing
`model.measurer.is_some()`. It printed `measurer=false`.
`ensure_font_atlas(&mut World)` is now `init`'s body for a caller holding the world, and a world
that still cannot build one warns instead of loading the interface in silence. The check that it
stays fixed is a walk of `UIParent` in a live capture counting FontStrings that have text and
measure 0: dozens before, **0** after.
The class to watch: **a resource another system inserts through `Commands` is not there for an
exclusive system running on a state transition.** Anything the UI load needs must be built by the
load, not awaited.
### The map's glide is a setting, because two reports bracketed one guess
The same constant was reported from both sides — "slippery" on the 4th, *"its the maps motion that
is laggy"* on the 5th — which is the signal to stop guessing and hand it over. `mapMotionSmoothing`
(Options → Interface, 0–100 in tens, 0 reads "Off") scales the one half-life both the pan and the
zoom ease on, `7 / (pct/100 × 0.30)`. Measured frames for one wheel notch to settle at 60 fps:
| 0% | 10% | **25% (ships)** | 50% | 100% |
|---|---|---|---|---|
| 1 | 17 | **42 (0.70 s)** | 84 | 167 |
What he had played was 167. The top of the slider is still that, deliberately — it is the slowest
anyone could want — and the default is a quarter of it. One rate drives both axes because a zoom
that settles while the pan is still drifting reads as the pan being broken. A drag still writes the
scroll directly and never eases.
### `$WOW_CAPTURE_SPELLS`: seat a real spellbook in a capture
A capture never logs in, so its spellbook is whatever a fixture hardcodes — and a whole class of
addon is driven by what is *in* that book. ATSW2 finds the player's professions by walking tab 1
and matching each spell's icon against its own thirteen-leaf `Professions` list, so with no
profession spell seated it finds nothing, draws no tab and switches nothing, which is
indistinguishable from the defect being chased. `$WOW_CAPTURE_SPELLS=<id>[,<id>…]` adds spells to
whatever book the fixture seats, rather than replacing it. The realm's own ids are in the comment
beside it: Survival 46050, Jewelcrafting 30220, and 1.12's Alchemy 2259, Blacksmithing 2018,
Cooking 2550, Enchanting 7411, Engineering 4036, First Aid 3273, Leatherworking 2108, Tailoring
3908, Smelting 2656. Seat a handful of **recipe** ids with the opener and the window comes up with
a real list, which is the only way to photograph one of these addons offline.
## A second profession's opener updated the book instead of reopening it
grom: *"nothing happens when I click another tab."* Advanced Trade Skill Window's side tabs are the
whole reason it exists, and their `<OnClick>` is two lines:
```lua
if this.Name ~= ATSW_Profession[ATSW_realm][ATSW_player] then
CastSpellByName(this.Name)
```
Everything up to the cast was right — the tab scan builds five correct tabs with their names and
icons, the saved profession key resolves, the opener resolves through the book, and
`send_spell_cast` intercepts it as a profession opener (`0x6e4bce`) and sets the open skill line.
What was wrong was the **event**. `feed_trade_skill` fired one event per *change*: a first open was
`TRADE_SKILL_SHOW` and everything after it `TRADE_SKILL_UPDATE`. ATSW2's update arm is
`UpdateCount = UpdateCount + 1` and nothing else; it rebuilds on `TRADE_SKILL_SHOW` alone, and its
show arm is explicitly written for the window **already being up**:
```lua
elseif event == 'TRADE_SKILL_SHOW' then
if UIParent:IsVisible() then
if ATSWFrame:IsVisible() or ATSWCSFrame:IsVisible() then … CloseCraft() … end
```
so the book swapped underneath it and nothing redrew.
The open is `0x4fc910`, and it reads cleanly:
```
4fc913 call 0x4fc890 ; drop the old list
4fc918 cmp esi, [0xbde040] ; the line already open
4fc91e jne 0x4fc935
4fc92a mov ecx, 0x13b ; → TRADE_SKILL_CLOSE, and return
4fc94c cmp [0xbde064], esi ; the last line shown
4fc95a or eax,-1 … [0x84dd60/64/68] ; → different: reset the three filters
4fc985 call 0x4fca20 ; rebuild
4fc997 jbe 0x4fc9a4 ; the rebuilt count at [0xbde04c] is 0 → no event
4fc999 mov ecx, 0x139 ; → TRADE_SKILL_SHOW
```
Three rules, all of them ours now, in `book_event`:
- **one event per open, not per change** — a changed line while a book is up is a fresh
`TRADE_SKILL_SHOW`; `TRADE_SKILL_UPDATE` is a change *within* one open book (a filter, a fold, an
item count);
- **the opener is the window's toggle** — casting the profession already open closes it, which is
exactly what ATSW2's `this.Name ~=` guard is there to avoid tripping;
- **an empty rebuilt list signals nothing** — the line is open, no window appears. Written over the
list about to be pushed rather than as the literal count read, because ours can be a frame late
where the reference builds its list inside the open call: a book that arrives empty and fills next
frame still fires exactly one show, on the frame it fills.
`0x4f6000` is the Craft window's same function, keyed on the craft type at `0xbdcfb8` instead of a
skill line and with no empty guard; both of its rules are in `feed_craft` and `open_trade_skill`
too, since `CRAFT_SHOW` is the arm ATSW2 uses for Enchanting.
The filter reset on a line change was already right — `set_trade_skill` keys it on
`trade_skill_last_line`, the `0xbde064` above.
Verified end to end offline, which `$WOW_CAPTURE_SPELLS` is what made possible: Blacksmithing open
with 24 recipes, `ATSWFrameTab3:Click()`, and the window comes back `title=Cooking line=Cooking
rows=9` with the check moved to tab 3. Before the fix the same click left the title on
Blacksmithing.