From e527931885ff40eee4a159c297f6f964f8eba8d9 Mon Sep 17 00:00:00 2001 From: grom Date: Sun, 18 Oct 2026 09:02:03 -0600 Subject: [PATCH] worldmap: WORLD_MAP_UPDATE is signalled inline, so the map opens on the player's zone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The world-map flash, and it is one frame of the wrong paint rather than a wrong selection. `WorldMapFrame.xml`'s OnShow runs `SetMapToCurrentZone()`, and `WorldMapFrame_OnEvent` answers `WORLD_MAP_UPDATE` with `WorldMapFrame_Update()` **only** `if ( this:IsVisible() )`. Our `store_selection` pushed that event on the deferred lane, so it was heard a tick later — by which time the panel was already on screen, still carrying whatever tiles the last `WorldMapFrame_Update` left. Those are the World sheet's: `WorldMapFrame_OnLoad` runs its own `WorldMapFrame_Update` before the player's zone is known, and a nil `GetMapInfo()` takes FrameXML's own `-- Temporary Hack` branch, `mapFileName = "World"` (`WorldMapFrame.lua:47-55`), painting `Interface\WorldMap\World\World1..12` into the twelve detail tiles. Signalled inline instead, as the setter `0x4a67a0`'s own `SignalEvent` is, so the repaint happens inside the OnShow and the first frame drawn is the player's zone. Signalling moves out of `store_selection` — the writer cannot do it, only a caller holding the Lua state can — and each of the four call sites does it: `SetMapZoom`, `SetMapToCurrentZone`, `ProcessMapClick` (only when the click actually moved the selection, since the setter is the only signal site) and the engine-side world-enter sync. `worldmap_update_event_fires_on_next_tick` asserted the deferred behaviour deliberately, on the reason "a binding cannot re-enter dispatch". That was not so: `UpdateSpells` had already been doing exactly this with the same `fire_event_into`, with its own test pinning the synchronous dispatch. Replaced by one that asserts the inline fire and one that covers the two other legs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- .../benilla-ui/src/script/tests/worldmap.rs | 52 +++++++++-- crates/benilla-ui/src/script/worldmap.rs | 93 +++++++++++++------ 2 files changed, 109 insertions(+), 36 deletions(-) diff --git a/crates/benilla-ui/src/script/tests/worldmap.rs b/crates/benilla-ui/src/script/tests/worldmap.rs index ce4dcc90..9d1d6d22 100644 --- a/crates/benilla-ui/src/script/tests/worldmap.rs +++ b/crates/benilla-ui/src/script/tests/worldmap.rs @@ -565,13 +565,20 @@ fn worldmap_zone_level_hover_names_without_highlight() { assert!(tx > 0.0 && ty > 0.0); } -/// `SetMapZoom` queues `WORLD_MAP_UPDATE` rather than firing it, since a binding cannot re-enter -/// dispatch; the registered frame hears it on the next tick. +/// `WORLD_MAP_UPDATE` fires **inside** the setter, as the reference's `SignalEvent` does — this +/// test previously asserted the opposite, on the belief that "a binding cannot re-enter dispatch", +/// which `UpdateSpells` had already disproved (`spellbook.rs`, same `fire_event_into`). +/// +/// It is the world-map flash. `WorldMapFrame.xml`'s OnShow calls `SetMapToCurrentZone()`, and +/// `WorldMapFrame_OnEvent` repaints only `if ( this:IsVisible() )` — so a queued event is always +/// heard a tick *after* the panel is already on screen, carrying whatever tiles the last +/// `WorldMapFrame_Update` left. That is the World sheet, because `WorldMapFrame_OnLoad` runs its +/// own `WorldMapFrame_Update` before the player's zone is known and `GetMapInfo()`'s nil takes +/// FrameXML's own `-- Temporary Hack` branch, `mapFileName = "World"`. #[test] -fn worldmap_update_event_fires_on_next_tick() { +fn worldmap_update_fires_inside_the_setter_as_signalevent_does() { let mut s = script(); push_catalog(&mut s); - // Drain the catalog push's own queued event first. s.tick(0.01); s.run( r#" @@ -585,16 +592,43 @@ fn worldmap_update_event_fires_on_next_tick() { .unwrap(); assert_eq!( s.eval::("return heard").unwrap(), - 0, - "nothing fires inside the SetMapZoom call itself" + 1, + "the repaint must land inside SetMapZoom, before the frame is drawn again" ); - s.tick(0.01); - assert_eq!(s.eval::("return heard").unwrap(), 1); + // And nothing rides the deferred lane behind it, so the tick does not fire a second one. s.tick(0.01); assert_eq!( s.eval::("return heard").unwrap(), 1, - "the queue drains — no re-fire" + "signalled once, not once inline and once more on the tick" + ); +} + +/// `SetMapToCurrentZone` signals too, which is the one the OnShow depends on, and a click that +/// lands on nothing does not — the setter is the only signal site. +#[test] +fn the_current_zone_setter_signals_and_a_click_on_nothing_does_not() { + let mut s = script(); + push_catalog(&mut s); + s.tick(0.01); + s.run( + r#" + heard = 0 + f = CreateFrame("Frame", "MapListener2") + f:RegisterEvent("WORLD_MAP_UPDATE") + f:SetScript("OnEvent", function() heard = heard + 1 end) + SetMapToCurrentZone() + "#, + ) + .unwrap(); + assert_eq!(s.eval::("return heard").unwrap(), 1); + // World level, and a point outside every continent rect: nothing moves, nothing signals. + s.run("SetMapZoom(0) heard = 0 ProcessMapClick(-1, -1)") + .unwrap(); + assert_eq!( + s.eval::("return heard").unwrap(), + 0, + "a click that selects nothing is silent" ); } diff --git a/crates/benilla-ui/src/script/worldmap.rs b/crates/benilla-ui/src/script/worldmap.rs index e15228fe..d20610c9 100644 --- a/crates/benilla-ui/src/script/worldmap.rs +++ b/crates/benilla-ui/src/script/worldmap.rs @@ -472,8 +472,13 @@ impl super::UiScript { zone: u32, direct_area: Option, ) { - let mut model = self.model_mut(); - select_resolved(&mut model, continent, zone, direct_area); + { + let mut model = self.model_mut(); + select_resolved(&mut model, continent, zone, direct_area); + } + // Inline, as the setter's own `SignalEvent` is. At world enter FrameXML has no listener + // yet, so this is a no-op then; it is the later engine-side moves that need it. + self.fire_event("WORLD_MAP_UPDATE", Vec::new()); } /// The map UV at UI point `(x, y)`, or `None` off the art, normalized over `WorldMapButton` @@ -497,12 +502,26 @@ impl super::UiScript { /// The one writer of the three selection cells, as the setter `0x4a67a0` is: every leg but /// `ecx == -2` clears the direct cell (`0x4a67ea`), and that one stores the id (`0x4a67c1`). +/// +/// Signalling is the caller's, not this writer's, because the signal must be **inline** and only a +/// caller holding the Lua state can do that: see [`signal_world_map_update`]. fn store_selection(model: &mut Model, selection: (u32, u32), direct_area: Option) { model.worldmap.selection = selection; model.worldmap.direct_area = direct_area; - model - .pending_events - .push(("WORLD_MAP_UPDATE".to_string(), Vec::new())); +} + +/// `WORLD_MAP_UPDATE`, dispatched **now**, as the setter `0x4a67a0`'s `SignalEvent` does. +/// +/// This is the world-map flash. `WorldMapFrame.xml`'s OnShow runs `SetMapToCurrentZone()`, and +/// `WorldMapFrame_Update` repaints the twelve detail tiles from `GetMapInfo()` — whose nil at +/// world level FrameXML answers with Blizzard's own `-- Temporary Hack`, `mapFileName = "World"` +/// (`WorldMapFrame.lua:47-55`). Queued on the deferred lane instead, the repaint landed a tick +/// late, so the frame was shown for a frame or more still carrying whatever tiles the last +/// `WorldMapFrame_Update` left — the World sheet, painted by the `WorldMapFrame_OnLoad` that ran +/// before the player's zone was known. Fired inline, the repaint happens inside the OnShow, and +/// the map is the player's zone the first time it is drawn. +fn signal_world_map_update(lua: &Lua) { + super::tick::fire_event_into(lua, "WORLD_MAP_UPDATE", Vec::new()); } /// Clamps and stores a continent and zone, clearing the direct area: the tail of `SetMapZoom`, @@ -596,8 +615,11 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "SetMapZoom", lua.create_function(|lua, (c, z): (i64, Option)| { - let mut model = lua.app_data_mut::().expect("model app_data"); - select(&mut model, c, z.unwrap_or(0)); + { + let mut model = lua.app_data_mut::().expect("model app_data"); + select(&mut model, c, z.unwrap_or(0)); + } + signal_world_map_update(lua); Ok(()) })?, )?; @@ -607,10 +629,13 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "SetMapToCurrentZone", lua.create_function(|lua, ()| { - let mut model = lua.app_data_mut::().expect("model app_data"); - let (c, z) = model.worldmap.player_zone.unwrap_or((0, 0)); - let direct = model.worldmap.player_direct_area; - select_resolved(&mut model, c, z, direct); + { + let mut model = lua.app_data_mut::().expect("model app_data"); + let (c, z) = model.worldmap.player_zone.unwrap_or((0, 0)); + let direct = model.worldmap.player_direct_area; + select_resolved(&mut model, c, z, direct); + } + signal_world_map_update(lua); Ok(()) })?, )?; @@ -694,23 +719,37 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "ProcessMapClick", lua.create_function(|lua, (x, y): (f32, f32)| { - let mut model = lua.app_data_mut::().expect("model app_data"); - // An instance map swallows the click (`0x4a7540` opens `cmp esi,-2; je 0x4a760d`), - // tested first as it shares the world sheet's `continent == 0`. - if model.worldmap.direct_area.is_some() { - return Ok(()); - } - if model.worldmap.selection.0 == 0 { - let hit = model.worldmap.continents.iter().position(|cont| { - let (u0, v0, u1, v1) = cont.world_rect; - (u0..=u1).contains(&x) && (v0..=v1).contains(&y) - }); - if let Some(i) = hit { - select(&mut model, i as i64 + 1, 0); + // Only a click that actually moved the selection signals, as the setter is the only + // signal site: a click on nothing is silent in the reference too. + let moved = { + let mut model = lua.app_data_mut::().expect("model app_data"); + // An instance map swallows the click (`0x4a7540` opens `cmp esi,-2; + // je 0x4a760d`), tested first as it shares the world sheet's `continent == 0`. + if model.worldmap.direct_area.is_some() { + return Ok(()); } - } else if let Some(zi) = grid_area(&model.worldmap, x, y) { - let c = i64::from(model.worldmap.selection.0); - select(&mut model, c, i64::from(zi)); + if model.worldmap.selection.0 == 0 { + let hit = model.worldmap.continents.iter().position(|cont| { + let (u0, v0, u1, v1) = cont.world_rect; + (u0..=u1).contains(&x) && (v0..=v1).contains(&y) + }); + match hit { + Some(i) => { + select(&mut model, i as i64 + 1, 0); + true + } + None => false, + } + } else if let Some(zi) = grid_area(&model.worldmap, x, y) { + let c = i64::from(model.worldmap.selection.0); + select(&mut model, c, i64::from(zi)); + true + } else { + false + } + }; + if moved { + signal_world_map_update(lua); } Ok(()) })?,