grom/benilla-octo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

worldmap: WORLD_MAP_UPDATE is signalled inline, so the map opens on the player's zone

Browse Source 
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
grom
2026-10-18 09:02:03 -06:00
co-authored by Claude Opus 5
parent 8e0d1bae8d
commit e527931885
2 changed files with 109 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/benilla-ui/src/script/tests/worldmap.rs
+43 -9
View File
@@ -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::<i64>("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::<i64>("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::<i64>("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::<i64>("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::<i64>("return heard").unwrap(),
0,
"a click that selects nothing is silent"
);
}
crates/benilla-ui/src/script/worldmap.rs
+66 -27
View File
@@ -472,8 +472,13 @@ impl super::UiScript {
zone: u32,
direct_area: Option<u32>,
) {
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<u32>) {
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<i64>)| {
let mut model = lua.app_data_mut::<Model>().expect("model app_data");
select(&mut model, c, z.unwrap_or(0));
{
let mut model = lua.app_data_mut::<Model>().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::<Model>().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::<Model>().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::<Model>().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::<Model>().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(())
})?,