From 5b3c2a98d41fb06c3c79ea5416e897dab576d6af Mon Sep 17 00:00:00 2001 From: Sam Sung <51686767+samwhosung@users.noreply.github.com> Date: Wed, 30 Sep 2026 11:49:14 +0700 Subject: [PATCH] pet bar: PickupPetAction needs the pet in view (#709) `PickupPetAction` lifted a pet action onto the cursor while the pet was out of the client's view, because the pickup gate read an unstreamed pet's missing flags as "not possessed". The reference's `PickupPetAction` (`0x4be180`) looks the pet up first (`0x4be1f7`). With no object it returns at once (`0x4be1fe`), and only then does it test `UNIT_FLAG_POSSESSED` (`0x4be20a`). The gate now needs the pet's object before the possessed test, and three comments that cited a mid-instruction address now cite these. Tests: with the pet out of view, a pickup lifts nothing and a held action stays on the cursor. An in-view pet still lifts, and a possessed one does not. Fixes #688 Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/benilla-app/src/ui_pet/bar.rs | 7 +- crates/benilla-app/src/ui_pet/usable_tests.rs | 78 ++++++++++++++++++- crates/benilla-ui/src/script/cursor/pet.rs | 7 +- crates/benilla-ui/src/script/pet.rs | 9 ++- 4 files changed, 88 insertions(+), 13 deletions(-) diff --git a/crates/benilla-app/src/ui_pet/bar.rs b/crates/benilla-app/src/ui_pet/bar.rs index 90085b3d..89a5a9d4 100644 --- a/crates/benilla-app/src/ui_pet/bar.rs +++ b/crates/benilla-app/src/ui_pet/bar.rs @@ -189,9 +189,10 @@ pub(super) fn feed_pet_bar( let pet_store = pet.store(bar.spells.pet_guid); let pet_flags = pet_store.map(|s| s.0.unit_flags()); let usable = pet.actions_usable(&bar); - // `PickupPetAction`'s gate alone (`0x4be1c1`): a possessed unit's bar cannot be rearranged, - // but its buttons work, so possession stays out of `usable`. - let pickup_allowed = pet_flags.unwrap_or(0) & UNIT_FLAG_POSSESSED == 0; + // `PickupPetAction`'s gate alone: the pet's object must resolve (`0x4be1f7`) and not be + // possessed (`0x4be20a`). A possessed unit's bar cannot be rearranged but its buttons work, + // so possession stays out of `usable`; an unstreamed pet has no flags to test and lifts nothing. + let pickup_allowed = pet_flags.is_some_and(|flags| flags & UNIT_FLAG_POSSESSED == 0); let pet_attacking = bar.attacking; // `GetPetTimeRemaining`'s expiry on the VM clock, signed both ways around the anchor sample. script.set_pet_expiry(bar.expires.map(|t| match t.checked_duration_since(anchor) { diff --git a/crates/benilla-app/src/ui_pet/usable_tests.rs b/crates/benilla-app/src/ui_pet/usable_tests.rs index a6d9fdc4..658561ba 100644 --- a/crates/benilla-app/src/ui_pet/usable_tests.rs +++ b/crates/benilla-app/src/ui_pet/usable_tests.rs @@ -90,8 +90,9 @@ struct Case { apply: fn(&mut Rig), /// What `0x4bcf70` answers. usable: bool, - /// Whether `PickupPetAction`'s own gate lets the drag start: `UNIT_FLAG_POSSESSED` blocks it - /// (`0x4be1c1`), which is no exit of the predicate. + /// Whether `PickupPetAction`'s own gate lets the drag start: the pet's object must resolve + /// (`0x4be1f7`) and not be `UNIT_FLAG_POSSESSED` (`0x4be20a`), which is no exit of the + /// predicate, whose possession never blocks and whose unresolved pet is one of its exits. draggable: bool, } @@ -179,7 +180,8 @@ fn all_cases() -> impl Iterator { name, apply, usable: false, - draggable: true, + // Every other exit still lifts the action; only a pet that does not resolve returns first. + draggable: !name.starts_with("the pet does not resolve"), }); usable.chain(exits) } @@ -436,6 +438,76 @@ fn each_exit_alone_stops_the_drags_writes() { } } +/// `0x4be1f7`-`0x4be20a`: `PickupPetAction` looks the pet up and returns at once with no object, +/// then again on `UNIT_FLAG_POSSESSED`, so a drag from an unstreamed or a possessed pet's bar lifts +/// nothing. Every other state lifts the action, an unusable pet included: `0x4bc9d0` refuses only +/// the pickup's blank, never the lift (`0x4be25d`). +#[test] +fn the_pickup_lifts_only_with_the_pets_object_held_and_not_possessed() { + for Case { + name, + apply, + draggable, + .. + } in all_cases() + { + let mut rig = scene(); + apply(&mut rig); + rig.frame(); + + ui(&mut rig, |s| s.run("PickupPetAction(1)").unwrap()); + let held = ui(&mut rig, |s| { + matches!( + s.cursor_payload(), + Some(benilla_ui::script::CursorPayload::PetAction(c)) if c.src_slot == 1 + ) + }); + assert_eq!( + held, draggable, + "{name}: the action is lifted onto the cursor" + ); + } +} + +/// The same lookup sits above the cursor fork, so a held action is neither dropped nor cleared +/// while the pet's object is gone: the drop's `ClearCursor` (`0x4be220`) is past it. +#[test] +fn a_held_action_stays_held_while_the_pet_is_out_of_view() { + let mut rig = scene(); + ui(&mut rig, |s| s.run("PickupPetAction(1)").unwrap()); + rig.app + .world_mut() + .resource_mut::() + .0 + .remove(&PET); + rig.frame(); + rig.app + .world_mut() + .run_system_once(super::drain::drain_pet_actions) + .expect("the drain runs"); + let _ = rig.sent(); + + ui(&mut rig, |s| s.run("PickupPetAction(2)").unwrap()); + rig.app + .world_mut() + .run_system_once(super::drain::drain_pet_actions) + .expect("the drain runs"); + + assert!( + matches!( + ui(&mut rig, |s| s.cursor_payload()), + Some(benilla_ui::script::CursorPayload::PetAction(c)) if c.src_slot == 1 + ), + "the payload is still on the cursor" + ); + assert!( + !rig.sent() + .iter() + .any(|c| matches!(c, ClientCommand::PetSetAction { .. })), + "and nothing was written" + ); +} + /// The whole default UI on the rig's world in place of its listening VM: the stock pet bar, and /// the fed pet under it. fn with_stock_ui(rig: &mut Rig) { diff --git a/crates/benilla-ui/src/script/cursor/pet.rs b/crates/benilla-ui/src/script/cursor/pet.rs index 5ff4da6a..51c42565 100644 --- a/crates/benilla-ui/src/script/cursor/pet.rs +++ b/crates/benilla-ui/src/script/cursor/pet.rs @@ -92,8 +92,9 @@ fn assign(slots: &mut [u32], target: usize, source: u32, passive: bool) -> Optio }) } -/// `PickupPetAction(slot)` (`0x4be180`), 1-based. `UNIT_FLAG_POSSESSED` blocks both ends, its -/// gate sitting above the cursor fork (`0x4be20a`). Another payload stays held here, where the +/// `PickupPetAction(slot)` (`0x4be180`), 1-based. A pet whose object does not resolve +/// (`0x4be1f7`) or is `UNIT_FLAG_POSSESSED` (`0x4be20a`) blocks both ends, the gate sitting above +/// the cursor fork, so the cursor is left as it was. Another payload stays held here, where the /// reference clears it (`0x4be220`) and picks the slot up. A pickup blanks only a spell slot /// (`0x4be268`), so a dragged token is copied, not moved. pub(super) fn pickup_pet_action(model: &mut Model, slot: u32) -> bool { @@ -511,7 +512,7 @@ mod tests { assert!(s.cursor_payload().is_none()); } - /// The third flag, `pickup_allowed`, is false under `UNIT_FLAG_POSSESSED`. + /// The third flag, `pickup_allowed`, is false for a pet that is out of view or possessed. #[test] fn a_possessed_bar_takes_no_drag_at_all() { let mut s = UiScript::new().unwrap(); diff --git a/crates/benilla-ui/src/script/pet.rs b/crates/benilla-ui/src/script/pet.rs index e82b8e79..44279704 100644 --- a/crates/benilla-ui/src/script/pet.rs +++ b/crates/benilla-ui/src/script/pet.rs @@ -112,10 +112,11 @@ pub(crate) struct PetBarState { /// `has_bar`, whose gate is the cached guid alone. pub(crate) has_ui: bool, pub(crate) stats: PetStats, - /// `PickupPetAction`'s gate, `UNIT_FLAG_POSSESSED` clear (`0x4be1c1`), which blocks the drop - /// as well as the pick-up. Not `actions_usable`: possession does not grey the bar, and the - /// pick-up itself does not ask it. The writes at both ends do (`0x4bc9d0`), so a greyed bar - /// still lifts an action onto the cursor and then writes nothing. + /// `PickupPetAction`'s gate: the pet's object resolves (`0x4be1f7`) and `UNIT_FLAG_POSSESSED` + /// is clear (`0x4be20a`), which blocks the drop as well as the pick-up. Not `actions_usable`: + /// possession does not grey the bar, and the pick-up itself does not ask it. The writes at + /// both ends do (`0x4bc9d0`), so a greyed bar with its pet in view still lifts an action onto + /// the cursor and then writes nothing. pub(crate) pickup_allowed: bool, /// `PetCanBeAbandoned()`: a kept pet rather than a summon. It forks the pet menu: paperdoll, /// rename and abandon show when true, dismiss only when false (`UnitPopup.lua:402-417`).