target: a new target is answered for before anything reads it

Two orderings let the inspect window re-show itself on a creature and then raise inside
`InspectPaperDollFrame_SetLevel`, where TurtleWoW's three-spec `PLAYER_LEVEL` meets a creature's
nil `UnitRace`.

`feed_unit_reach` now runs ahead of `feed_units`, not behind it. `fire_event` dispatches into Lua
synchronously, so a handler of `PLAYER_TARGET_CHANGED` that asks `CanInspect("target")` was being
answered off the previous frame's reach map — the old player target, in range and inspectable — and
`InspectFrame_Show` showed the window for the creature that had just replaced it.

And `PLAYER_TARGET_CHANGED` now leads the token `UNIT_*` fan-out. The reference has no token diff at
all: re-pointing `"target"` writes no descriptor field, so its watches stay silent and this is the
only event a new target raises. Behind the fan-out, `InspectPaperDollFrame_OnEvent`'s `UNIT_LEVEL`
arm ran against an `InspectFrame.unit` the next event was about to clear. Every snapshot is pushed
before either fires, so a handler reads the new target either way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn

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::<UnitFeedState>()
+            .init_resource::<Selection>()
+            .init_resource::<NameCache>()
+            .init_resource::<Reputations>()
+            .init_resource::<crate::ui_party::GroupState>()
+            .init_resource::<crate::ui_chat::ChatLog>()
+            .init_resource::<crate::sound::MessageSounds>()
+            .init_resource::<crate::ui_guild::GuildState>()
+            .init_resource::<crate::ui_session::InteractNpc>();
+        app.add_message::<FieldChanged>();
+        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>();
+            selection.target = Some(entity);
+            selection.guid = Some(guid);
+            app.update();
+        };
+        let run = |app: &mut App, src: &str| {
+            app.world_mut()
+                .non_send_resource_mut::<UiScript>()
+                .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::<UiScript>()
+            .eval::<String>(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() {