diff --git a/crates/benilla-app/src/ui_unit.rs b/crates/benilla-app/src/ui_unit.rs index caede090..47b45a79 100644 --- a/crates/benilla-app/src/ui_unit.rs +++ b/crates/benilla-app/src/ui_unit.rs @@ -163,8 +163,15 @@ impl Plugin for UiUnitPlugin { // First, so a worldport's leaving edge precedes the entering edge `feed_units` // raises for the same port. fire_leaving_world_on_worldport, - feed_units.in_set(UnitFeedDispatch), + // Before `feed_units`, because `fire_event` dispatches into Lua *inside* it: a + // handler of its `PLAYER_TARGET_CHANGED` that asks `CanInspect("target")` must be + // answered about the new target, and the reference answers from live state + // (`0x48a1b0` reads the object manager at call time). Behind it, the map still + // held the previous selection, so `InspectFrame_Show` (`Blizzard_InspectUI.lua:8`) + // re-showed the inspect window for a creature and `InspectPaperDollFrame_SetLevel` + // raised on `UnitRace`'s nil. feed_unit_reach, + feed_units.in_set(UnitFeedDispatch), feed_self_movement, feed_player_control, feed_farsight_focus, @@ -1685,6 +1692,24 @@ fn feed_units( memo.action_bar_toggles = None; } + // `PLAYER_TARGET_CHANGED`, argless, when the selection changes. A `/reload`'s fresh memo fires + // it for a kept selection, where the reference's shutdown clears the target (`0x490bdf`). + // + // Ahead of the token transitions below, not after them, because the reference fires this one + // from the selection setter itself and has no token-diff at all: re-pointing `"target"` at + // another unit writes no descriptor field, so its `UNIT_*` watches stay silent and + // `PLAYER_TARGET_CHANGED` is the only event a new target raises. Ours fans out `UNIT_LEVEL`, + // `UNIT_HEALTH` and the rest off the snapshot diff, and behind this fire they reached handlers + // that still believed the old target: `InspectPaperDollFrame_OnEvent`'s `UNIT_LEVEL` arm ran + // against an `InspectFrame.unit` that `InspectFrame_Show` was about to clear, and + // `InspectPaperDollFrame_SetLevel` raised on a creature's nil `UnitRace`. Every snapshot is + // already pushed above, so a handler here reads the new target, not the old one. + if selection.guid != memo.target_guid { + gate.audit("feed_units", "the PLAYER_TARGET_CHANGED edge"); + memo.target_guid = selection.guid; + script.fire_event("PLAYER_TARGET_CHANGED", vec![]); + } + for (token, snap) in [ ("player", &player), ("target", &target), @@ -1713,14 +1738,6 @@ fn feed_units( } } - // `PLAYER_TARGET_CHANGED`, argless, when the selection changes. A `/reload`'s fresh memo fires - // it for a kept selection, where the reference's shutdown clears the target (`0x490bdf`). - if selection.guid != memo.target_guid { - gate.audit("feed_units", "the PLAYER_TARGET_CHANGED edge"); - memo.target_guid = selection.guid; - script.fire_event("PLAYER_TARGET_CHANGED", vec![]); - } - // `PLAYER_REGEN_DISABLED`/`ENABLED` on our in-combat flag's edge (`UNIT_FLAG_IN_COMBAT` // `0x00080000`, vmangos `UnitDefines.h:564`); the reference's own trigger is untraced. if let Some((_, store, _)) = self_pair { @@ -2479,6 +2496,83 @@ mod tests { ); } + /// `PLAYER_TARGET_CHANGED` leads the token fan-out. The reference has no token diff at all — + /// re-pointing `"target"` writes no descriptor field, so its `UNIT_*` watches stay silent and + /// this is the only event a new target raises. Ours fans out off the snapshot diff, and behind + /// the fire those events reached handlers that still believed the old target. + #[test] + fn a_new_target_raises_player_target_changed_before_the_token_unit_events() { + use benilla_protocol::messages::ObjectFields; + + const FIELD_UNIT_LEVEL: u16 = 34; + const FIRST: u64 = 0x0000_0000_0000_0011; + const SECOND: u64 = 0x0000_0000_0000_0012; + + let mut app = App::new(); + app.init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::(); + app.add_message::(); + let (tx, _rx) = crossbeam_channel::unbounded(); + app.insert_resource(NetCommands(tx)); + app.insert_non_send_resource(UiScript::new().unwrap()); + app.add_systems(Update, feed_units); + + // Two players told apart by level alone, so the repoint is a `UNIT_LEVEL` transition. + let mut player = |level: u32| { + app.world_mut() + .spawn(ObjectStore(ObjectFields::from_pairs(&[( + FIELD_UNIT_LEVEL, + level, + )]))) + .id() + }; + let first = player(10); + let second = player(20); + + let select = |app: &mut App, entity, guid| { + let mut selection = app.world_mut().resource_mut::(); + selection.target = Some(entity); + selection.guid = Some(guid); + app.update(); + }; + let run = |app: &mut App, src: &str| { + app.world_mut() + .non_send_resource_mut::() + .run(src) + .unwrap(); + }; + + select(&mut app, first, FIRST); + run( + &mut app, + r#" + ORDER = {} + local f = CreateFrame("Frame") + f:RegisterEvent("PLAYER_TARGET_CHANGED") + f:RegisterEvent("UNIT_LEVEL") + f:SetScript("OnEvent", function() table.insert(ORDER, event) end) + "#, + ); + select(&mut app, second, SECOND); + + let order = app + .world_mut() + .non_send_resource_mut::() + .eval::(r#"return table.concat(ORDER, ",")"#) + .unwrap(); + assert!( + order.starts_with("PLAYER_TARGET_CHANGED"), + "a new target fired {order:?}, so a handler read the old target first" + ); + } + /// Either half moving pushes; only the count, the one byte the client watches, fires. #[test] fn only_the_count_fires_the_combo_event() {