From 3df3303d0e9f38d2577df49213e5e2cd54ce079c Mon Sep 17 00:00:00 2001 From: Sam Sung <51686767+samwhosung@users.noreply.github.com> Date: Sat, 26 Sep 2026 06:15:14 +0700 Subject: [PATCH] Every loot close deselects a dead unit, and a selection change closes the loot on the old target (#379) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closing a loot window left the looted corpse targeted, and changing target left the old corpse's loot window open. Fixes #238. Fixes #71. - **One close routine** (`0x48f200`): every close (CloseLoot, the last row or the coins taken, the server's release response, stepping away) runs the same routine, then deselects the source when it is a unit or player with health 0 or below (`0x48f34f`-`0x48f369`). Before, only the movement close asked, and it keyed on the loot type rather than health. A skinned corpse now loses the selection; a live pick-pocket target keeps it. - **A selection change closes the old target's loot** (`0x4936cc` → `0x493910` → `0x493974`): Tab, Esc, a click-off or the target's death sends the release before the new selection. A chest window survives a Tab. - Declaring the loot drain's order against the target update also clears about ninety older undeclared schedule pairs, so the ceiling follows down. Tests: four new or extended tests fail with the behaviours stubbed out and pass now; `scripts/check.sh` is green. To see it: kill a mob, loot it and close the window: the target frame clears. Loot a corpse and press Tab with the window open: the window closes and the next mob is targeted. One wire difference stays and is documented at the seam: when Tab closes a window on a dead target, the reference also sends a `CMSG_SET_SELECTION` of 0 before the new selection (`0x493a2a`). The server ends in the same state. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/benilla-app/src/creature_anim.rs | 22 +- crates/benilla-app/src/game_plugins.rs | 2 +- crates/benilla-app/src/go_anim.rs | 3 +- crates/benilla-app/src/target/click.rs | 28 +-- crates/benilla-app/src/target/scan.rs | 64 +++++ crates/benilla-app/src/ui_loot/mod.rs | 295 ++++++++++++++++++++---- crates/benilla-app/src/ui_loot/net.rs | 71 +++++- crates/benilla-app/src/ui_pet_stats.rs | 2 + 8 files changed, 417 insertions(+), 70 deletions(-) diff --git a/crates/benilla-app/src/creature_anim.rs b/crates/benilla-app/src/creature_anim.rs index b30e9b00..04617da9 100644 --- a/crates/benilla-app/src/creature_anim.rs +++ b/crates/benilla-app/src/creature_anim.rs @@ -266,7 +266,7 @@ pub(crate) fn toggle_attack_local( } /// The attack seams' write set as one param, for the selection writers that run the reference's -/// stop, select, re-swing (`SetSelection` `0x493540`). +/// stop, select, re-swing (`SetSelection` `0x493540`), and the loot its teardown closes. #[derive(bevy::ecs::system::SystemParam)] pub(crate) struct AttackSeam<'w, 's> { pub(crate) net: Res<'w, crate::net::NetCommands>, @@ -276,9 +276,29 @@ pub(crate) struct AttackSeam<'w, 's> { pub(crate) ecs: Commands<'w, 's>, /// Our own entity. pub(crate) me: Query<'w, 's, Entity, With>, + /// The open loot, which the selection teardown closes when it loots the outgoing target. + pub(crate) loot: ResMut<'w, crate::ui_loot::LootState>, + /// The loot latch: the teardown's close drops it, and the right-click loot legs arm it. + pub(crate) loot_latch: ResMut<'w, crate::ui_loot::LootLatch>, } impl AttackSeam<'_, '_> { + /// The selection teardown's loot close (`0x493910`, `0x493959`–`0x493974`): a window looting + /// the outgoing selection closes as `CloseLoot` does, its release ahead of anything the new + /// selection sends. The close's dead-unit deselect (`0x48f369`) targets the selection already + /// being torn down, so none is asked; the reference's nested teardown there also sends a + /// `CMSG_SET_SELECTION` 0 (`0x493a2a`), which this does not. + pub(crate) fn close_loot_on(&mut self, outgoing: u64) { + if self.loot.source() == Some(outgoing) { + debug!("target: the selection teardown closes the loot on {outgoing:#x}"); + crate::ui_loot::close_interaction( + &mut self.loot, + &mut self.loot_latch, + Some(&self.net), + ); + } + } + /// StopAttack (`0x5ecac0`). pub(crate) fn stop(&mut self, engaged: bool) { stop_attack_local(engaged, &mut self.queued_melee, &self.net); diff --git a/crates/benilla-app/src/game_plugins.rs b/crates/benilla-app/src/game_plugins.rs index d71380c4..5aab507b 100644 --- a/crates/benilla-app/src/game_plugins.rs +++ b/crates/benilla-app/src/game_plugins.rs @@ -662,7 +662,7 @@ pub(crate) mod schedule_tests { /// Raising the ceiling is a claim that a new undeclared order is acceptable: make it with the /// reason read off the dump, or declare the order (`.after`, a set, a `chain`). A resource /// that commutes by construction belongs in [`Classes`]. - const UPDATE_ACTIONABLE_CEILING: usize = 5_551; + const UPDATE_ACTIONABLE_CEILING: usize = 5_462; const UPDATE_ACTIONABLE_SLACK: usize = 40; fn ratchet(what: &str, n: usize, ceiling: usize, slack: usize) { diff --git a/crates/benilla-app/src/go_anim.rs b/crates/benilla-app/src/go_anim.rs index 239022a7..1edd12f1 100644 --- a/crates/benilla-app/src/go_anim.rs +++ b/crates/benilla-app/src/go_anim.rs @@ -685,7 +685,8 @@ pub(crate) fn plugin(app: &mut App) { ( sync_wire_go_state, open_go_lid, - close_go_lid, + // After the selection writers, whose teardown can close the loot. + close_go_lid.after(crate::target::TargetUpdate), queue_custom_anim, arm_despawn_anim, // Clears `shown` from last frame's finished flags before the drive, so the diff --git a/crates/benilla-app/src/target/click.rs b/crates/benilla-app/src/target/click.rs index c637f7fc..16a14165 100644 --- a/crates/benilla-app/src/target/click.rs +++ b/crates/benilla-app/src/target/click.rs @@ -189,7 +189,6 @@ pub(super) fn act_on_right_click( ui_feedback: ( ResMut, ResMut, - ResMut, ResMut, ResMut, // The opener queue: this system cannot also hold `CastLadder` (a second `Items` and @@ -199,15 +198,8 @@ pub(super) fn act_on_right_click( MessageWriter, ), ) { - let ( - mut ui_error_keys, - mut cast_errors, - mut loot_latch, - mut mail, - mut item_text, - mut openers, - mut stone_uses, - ) = ui_feedback; + let (mut ui_error_keys, mut cast_errors, mut mail, mut item_text, mut openers, mut stone_uses) = + ui_feedback; if clicks.read().last().is_none() { return; } @@ -386,7 +378,7 @@ pub(super) fn act_on_right_click( debug!("right-click corpse loot: {guid:#x}"); let _ = seam.net.0.send(ClientCommand::Loot { guid }); // Predicted, as for a unit corpse (`[player+0x1d28]` armed at the send). - loot_latch.0 = Some(guid); + seam.loot_latch.0 = Some(guid); } } else if store.is_some_and(|s| s.0.corpse_pvp_insignia()) { // Leg 2. `skin_player_corpse` mirrors `[0xb700e8]`, `None` for every 1.12.1 player, @@ -494,7 +486,7 @@ pub(super) fn act_on_right_click( let _ = seam.net.0.send(ClientCommand::Loot { guid }); // Predicted at the send, as the reference's sender (`0x5df253`) arms // `[player+0x1d28]` and kneels before any reply; the anim driver reads the latch. - loot_latch.0 = Some(guid); + seam.loot_latch.0 = Some(guid); } } UnitBranch::Dead(DeadUnitLeg::Skin) => { @@ -1021,7 +1013,7 @@ pub(super) fn clear_target_requests( } /// A deselect that applies only if the selection is this guid (`0x493910(guid, 1)`), raised by -/// the loot window's move-start close (`0x48f369`) and drained by [`clear_target_requests`]. +/// every loot close for a dead unit (`0x48f369`) and drained by [`clear_target_requests`]. #[derive(bevy::ecs::message::Message, Clone, Copy, Debug)] pub(crate) struct DeselectGuid(pub(crate) u64); @@ -1092,7 +1084,10 @@ pub(super) fn clear( engaged: bool, ) { if selection.target.take().is_some() { - selection.guid = None; + if let Some(old) = selection.guid.take() { + // The teardown `0x493910` closes the old target's loot before its own send. + seam.close_loot_on(old); + } let _ = seam.net.0.send(ClientCommand::SetSelection { guid: 0 }); // `SetSelection 0x493540`'s own `0x493a08 call 0x5ecac0`, the real StopAttack, which // also un-queues a pending on-next-swing strike. @@ -1573,6 +1568,8 @@ mod tests { world.init_resource::(); world.init_resource::(); world.init_resource::(); + world.init_resource::(); + world.init_resource::(); world.init_resource::(); world.init_resource::(); world.init_resource::(); @@ -1646,6 +1643,8 @@ mod tests { world.insert_resource(NetCommands(tx)); world.init_resource::(); world.init_resource::(); + world.init_resource::(); + world.init_resource::(); world.init_resource::>(); world.init_resource::(); world.init_resource::(); @@ -1749,6 +1748,7 @@ mod tests { world.init_resource::(); world.init_resource::(); world.init_resource::(); + world.init_resource::(); world.init_resource::(); world.init_resource::(); world.init_resource::(); diff --git a/crates/benilla-app/src/target/scan.rs b/crates/benilla-app/src/target/scan.rs index 559c6ff1..3dff5700 100644 --- a/crates/benilla-app/src/target/scan.rs +++ b/crates/benilla-app/src/target/scan.rs @@ -457,6 +457,10 @@ pub(super) fn commit( return CommitOutcome::default(); // the setter's dedup } let had_old = selection.guid.is_some(); + if let Some(old) = selection.guid { + // The old target's teardown (`0x4936cc` → `0x493910`) closes its loot first. + seam.close_loot_on(old); + } selection.target = Some(entity); selection.guid = Some(guid); let stop_and_repoint = engaged && had_old; @@ -898,10 +902,70 @@ mod tests { world.init_resource::>(); world.init_resource::>(); world.init_resource::(); + world.init_resource::(); + world.init_resource::(); world.spawn(SelfPlayer); (world, rx) } + /// The teardown of the outgoing target (`0x493910`) closes a loot window on it + /// (`0x493959`–`0x493974`), its release ahead of the new selection; a window on anything else + /// stays open. The clear (`0x4938f8`) runs the same teardown. + #[test] + fn a_selection_change_closes_the_loot_on_the_outgoing_target() { + use bevy::ecs::system::RunSystemOnce; + const CORPSE: u64 = 0xF130_0000_0000_0042; + const NEXT: u64 = 0xF130_0000_0000_0043; + const CHEST: u64 = 0xF110_0000_0000_1234; + let (mut world, rx) = commit_world(); + let wire = |rx: &crossbeam_channel::Receiver| { + rx.try_iter() + .map(|c| match c { + ClientCommand::LootRelease { guid } => format!("release {guid:#x}"), + ClientCommand::SetSelection { guid } => format!("select {guid:#x}"), + _ => "other".to_owned(), + }) + .collect::>() + }; + let open = |world: &mut World, guid: u64| { + world + .resource_mut::() + .open(guid, 1, 0, Vec::new()); + world.resource_mut::().0 = Some(guid); + }; + let source = |world: &World| world.resource::().source(); + + // Tab from the looted corpse to the next mob: release, then select. + assert!(go(&mut world, CORPSE, false, Some(1), false).changed); + open(&mut world, CORPSE); + let _ = wire(&rx); + assert!(go(&mut world, NEXT, false, Some(1), true).changed); + assert_eq!( + wire(&rx), + ["release 0xf130000000000042", "select 0xf130000000000043"] + ); + assert_eq!(source(&world), None); + assert_eq!(world.resource::().0, None); + + // A chest's window is not the selection's: a change leaves it open. + open(&mut world, CHEST); + assert!(go(&mut world, CORPSE, false, Some(1), false).changed); + assert_eq!(wire(&rx), ["select 0xf130000000000042"]); + assert_eq!(source(&world), Some(CHEST)); + + // Clearing the target tears it down the same way. + open(&mut world, CORPSE); + world + .run_system_once( + |mut selection: ResMut, mut seam: crate::creature_anim::AttackSeam| { + super::super::click::clear(&mut selection, &mut seam, false); + }, + ) + .expect("clear runs as a one-shot system"); + assert_eq!(wire(&rx), ["release 0xf130000000000042", "select 0x0"]); + assert_eq!(source(&world), None); + } + /// `0x493540`: an engaged switch is stop, select, re-swing; the stop un-queues an on-next-swing /// strike (`0x6e6f30`) and the re-swing cancels auto-repeat (`0x5ecd8c`). #[test] diff --git a/crates/benilla-app/src/ui_loot/mod.rs b/crates/benilla-app/src/ui_loot/mod.rs index 2bce52e6..443e66e0 100644 --- a/crates/benilla-app/src/ui_loot/mod.rs +++ b/crates/benilla-app/src/ui_loot/mod.rs @@ -391,11 +391,17 @@ impl Plugin for UiLootPlugin { Update, ( // Feed before the input pass and drain after it: an open shows, and a click - // sends, the same frame. + // sends, the same frame. The drain's deselect lands before the selection + // writers read it, and their teardown's close finds this frame's window. feed_loot.in_set(UiFeed), - drain_loot.after(UiInput), - // After the net drain that arms the latch; the anim driver orders after this. - resolve_loot_kneel.after(benilla_world::schedule::WorldStage::Net), + drain_loot + .after(UiInput) + .before(crate::target::TargetUpdate), + // After the net drain and the selection writers, which arm and drop the + // latch; the anim driver orders after this. + resolve_loot_kneel + .after(benilla_world::schedule::WorldStage::Net) + .after(crate::target::TargetUpdate), ), ); } @@ -754,35 +760,82 @@ fn bind_confirm_required(items: &Items, commands: &NetCommands, item_id: u32) -> .is_some_and(|t| t.bonding == BIND_WHEN_PICKED_UP && t.quality >= BIND_CONFIRM_MIN_QUALITY) } +/// `CloseInteraction 0x48f200` past its early returns, the one close every loot close shares: the +/// latch drops (`0x48f2c9`), the release goes out when `release` is given (the `cl` argument, +/// `0x48f2da`) and the frame closes (`0x48f33d`). Returns the closed source for +/// [`DeadUnitDeselect`], the close's last step. +pub(crate) fn close_interaction( + loot: &mut LootState, + latch: &mut LootLatch, + release: Option<&NetCommands>, +) -> Option { + let source = loot.source?; + latch.clear_for(source); + if let Some(commands) = release { + let _ = commands.0.send(ClientCommand::LootRelease { guid: source }); + } + loot.clear(); + Some(source) +} + +/// `CloseInteraction`'s last step (`0x48f34f`–`0x48f369`): a closed source that is a unit, a +/// player included, with `UNIT_FIELD_HEALTH` at or below 0 is deselected if it is still the +/// selection (`0x493910(guid, 1)`). It reads the health, never the loot type, so a skinned corpse +/// loses the selection and a live pickpocket target keeps it; an unstreamed source is left alone. +#[derive(bevy::ecs::system::SystemParam)] +pub(crate) struct DeadUnitDeselect<'w, 's> { + index: Res<'w, crate::net::GuidIndex>, + objects: Query< + 'w, + 's, + ( + &'static crate::net::NetEntity, + &'static crate::net::ObjectStore, + ), + >, + asks: MessageWriter<'w, crate::target::DeselectGuid>, +} + +impl DeadUnitDeselect<'_, '_> { + pub(crate) fn after_close(&mut self, closed: Option) { + let Some(source) = closed else { + return; + }; + let dead_unit = self + .index + .0 + .get(&source) + .and_then(|&e| self.objects.get(e).ok()) + .is_some_and(|(net_entity, store)| { + matches!( + net_entity.kind, + benilla_protocol::EntityKind::Unit | benilla_protocol::EntityKind::Player + ) && (store.0.unit_health().unwrap_or(0) as i32) <= 0 // signed, `jg` at `0x48f360` + }); + if dead_unit { + self.asks.write(crate::target::DeselectGuid(source)); + } + } +} + /// `CloseInteraction 0x48f200(cl=1, dl=1, 0)` on a movement start. A disenchant window with rows -/// left (`0x48f24a`) and an item's loot (`0x48f2ab`) survive; anything else clears the latch -/// (`0x48f2c9`), sends the release (`0x48f2da`) and closes (`0x48f33d`). The reference then -/// deselects a unit at health 0 that is the selection (`0x48f35d`, `0x48f369`); this tests the -/// loot type instead, so a skinned corpse, wire type 2 like a pickpocket, keeps its selection. +/// left (`0x48f24a`) and an item's loot (`0x48f2ab`) survive; anything else takes the shared close. fn close_on_move_start( loot: &mut LootState, latch: &mut LootLatch, commands: &NetCommands, - deselect: &mut MessageWriter, -) { +) -> Option { use benilla_protocol::guid; use benilla_protocol::messages::loot_type; - let Some(source) = loot.source() else { - return; - }; + let source = loot.source()?; if loot.loot_type == loot_type::DISENCHANTING && !loot.is_empty() { - return; + return None; } if guid::is_item(source) { - return; + return None; } debug!("ui_loot: movement started with {source:#x} open — release"); - latch.clear_for(source); - let _ = commands.0.send(ClientCommand::LootRelease { guid: source }); - if guid::is_creature_or_pet(source) && loot.loot_type != loot_type::PICKPOCKETING { - deselect.write(crate::target::DeselectGuid(source)); - } - loot.clear(); + close_interaction(loot, latch, Some(commands)) } /// Sends what the Lua asked for, as the take dispatcher `0x4c2790(slot, flag)`: a row click @@ -798,10 +851,10 @@ fn drain_loot( items: Res, // The move-start close runs before the VM check: it needs no Lua. mut move_start: ResMut, - mut deselect: MessageWriter, + mut dead: DeadUnitDeselect, ) { if std::mem::take(&mut move_start.0) { - close_on_move_start(&mut loot, &mut latch, &commands, &mut deselect); + dead.after_close(close_on_move_start(&mut loot, &mut latch, &commands)); } let Some(mut script) = script else { return; @@ -903,23 +956,22 @@ fn drain_loot( target, }); } + // `CloseLoot` (`0x4c2ec0` → `0x48f200(cl=1, dl=0)`), which stock `LootFrame_OnHide` calls. if script.take_loot_close() { - if let Some(guid) = loot.source { + let closed = close_interaction(&mut loot, &mut latch, Some(&commands)); + if let Some(guid) = closed { debug!("ui_loot: release loot {guid:#x}"); - let _ = commands.0.send(ClientCommand::LootRelease { guid }); - loot.clear(); // client-authoritative close - latch.clear_for(guid); // the kneel ends at the release send } + dead.after_close(closed); } - // The last-row auto-close (`0x4c2a70` → `0x48f200`). The feed then fires `LOOT_CLOSED`, and - // the `CloseLoot()` from `OnHide` finds no source. + // The last-row auto-close (`0x4c1f8d`, `0x4c2785` → `0x48f200(cl=1, dl=0)`). The feed then + // fires `LOOT_CLOSED`, and the `CloseLoot()` from `OnHide` finds no source. if loot.take_auto_release() { - if let Some(guid) = loot.source { + let closed = close_interaction(&mut loot, &mut latch, Some(&commands)); + if let Some(guid) = closed { debug!("ui_loot: loot emptied — auto-release {guid:#x}"); - let _ = commands.0.send(ClientCommand::LootRelease { guid }); - loot.clear(); - latch.clear_for(guid); } + dead.after_close(closed); } } @@ -1031,6 +1083,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .insert_resource(NetCommands(tx)); let mut items = app.world_mut().resource_mut::(); @@ -1980,14 +2033,12 @@ mod tests { assert_eq!(if 4u32 > 4 { 3 } else { 4 }, 4); } - /// `CloseInteraction` on a move start, with no server help and no VM. - #[test] - fn a_movement_start_closes_and_releases_the_open_loot() { - use bevy::ecs::message::Messages; - const CORPSE: u64 = 0xF130_0000_0000_0042; - const BOBBER: u64 = 0xF110_0000_0000_0011; - const LOCKBOX: u64 = 0x4000_0000_0000_0007; + /// A creature corpse and a live creature, both streamed. + const CORPSE: u64 = 0xF130_0000_0000_0042; + const LIVE: u64 = 0xF130_0000_0000_0043; + /// `drain_loot` scheduled, with [`CORPSE`] streamed at health 0 and [`LIVE`] at 100. + fn close_app() -> (App, crossbeam_channel::Receiver) { let (tx, rx) = crossbeam_channel::unbounded(); let mut app = App::new(); app.add_message::() @@ -1997,21 +2048,141 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .insert_resource(NetCommands(tx)) .add_systems(Update, drain_loot); - // No UiScript mounted: the move-start leg must not depend on the VM. - let deselects = |app: &mut App| -> Vec { + for (guid, health) in [(CORPSE, 0), (LIVE, 100)] { + let e = app + .world_mut() + .spawn(( + crate::net::NetEntity { + kind: EntityKind::Unit, + display_id: None, + scale: 1.0, + }, + crate::net::ObjectStore(ObjectFields::from_pairs(&[(F_UNIT_HEALTH, health)])), + )) + .id(); app.world_mut() - .resource_mut::>() - .drain() - .map(|d| d.0) + .resource_mut::() + .0 + .insert(guid, e); + } + (app, rx) + } + + /// The deselects asked for since the last call. + fn deselects(app: &mut App) -> Vec { + app.world_mut() + .resource_mut::>() + .drain() + .map(|d| d.0) + .collect() + } + + /// Opens a window as an admitted response does: the rows and the latch. + fn open_loot(app: &mut App, guid: u64, kind: u8, gold: u32, rows: Vec) { + app.world_mut() + .resource_mut::() + .open(guid, kind, gold, rows); + app.world_mut().resource_mut::().0 = Some(guid); + } + + /// Every close reaches `CloseInteraction`'s last step (`0x48f34f`–`0x48f369`): `CloseLoot` + /// (`0x4c2ed6`), the empty-window close after the last row (`0x4c1f8d`) or the coin + /// (`0x4c2785`), each deselecting a dead unit and only a dead unit. + #[test] + fn every_loot_close_deselects_a_dead_unit() { + let (mut app, rx) = close_app(); + let released = |rx: &crossbeam_channel::Receiver| -> Vec { + rx.try_iter() + .filter_map(|c| match c { + ClientCommand::LootRelease { guid } => Some(guid), + _ => None, + }) .collect() }; - let open = |app: &mut App, guid: u64, kind: u8, rows: Vec| { + app.insert_non_send_resource(UiScript::new().unwrap()); + let close_loot = |app: &mut App| { app.world_mut() - .resource_mut::() - .open(guid, kind, 0, rows); - app.world_mut().resource_mut::().0 = Some(guid); + .non_send_resource_mut::() + .run("CloseLoot()") + .unwrap(); + app.update(); + }; + + // `CloseLoot`, as the close button, Escape or any other hide reaches it. + open_loot( + &mut app, + CORPSE, + loot_type::CORPSE, + 0, + vec![item(0, 117, 1)], + ); + close_loot(&mut app); + assert_eq!(released(&rx), [CORPSE]); + assert_eq!(app.world().resource::().0, None); + assert_eq!( + deselects(&mut app), + [CORPSE], + "CloseLoot deselects the corpse" + ); + + // The last row taken. + open_loot( + &mut app, + CORPSE, + loot_type::CORPSE, + 0, + vec![item(0, 117, 1)], + ); + app.world_mut().resource_mut::().remove_slot(0); + app.update(); + assert_eq!(released(&rx), [CORPSE]); + assert_eq!( + deselects(&mut app), + [CORPSE], + "the last row's close deselects" + ); + + // The coin taken from a coin-only window. + open_loot(&mut app, CORPSE, loot_type::CORPSE, 250, vec![]); + app.world_mut().resource_mut::().clear_money(); + app.update(); + assert_eq!(released(&rx), [CORPSE]); + assert_eq!(deselects(&mut app), [CORPSE], "the coin's close deselects"); + + // A live pickpocket target keeps the selection. + open_loot( + &mut app, + LIVE, + loot_type::PICKPOCKETING, + 0, + vec![item(0, 117, 1)], + ); + close_loot(&mut app); + assert_eq!(released(&rx), [LIVE]); + assert!( + deselects(&mut app).is_empty(), + "a live unit is not deselected" + ); + + // With nothing open, a second `CloseLoot` sends and asks nothing. + close_loot(&mut app); + assert!(released(&rx).is_empty()); + assert!(deselects(&mut app).is_empty()); + } + + /// `CloseInteraction` on a move start, with no server help and no VM. + #[test] + fn a_movement_start_closes_and_releases_the_open_loot() { + const BOBBER: u64 = 0xF110_0000_0000_0011; + const LOCKBOX: u64 = 0x4000_0000_0000_0007; + + // No UiScript mounted: the move-start leg must not depend on the VM. + let (mut app, rx) = close_app(); + let open = |app: &mut App, guid: u64, kind: u8, rows: Vec| { + open_loot(app, guid, kind, 0, rows); }; let step = |app: &mut App| { app.world_mut().resource_mut::().0 = true; @@ -2036,6 +2207,32 @@ mod tests { assert!(rx.try_recv().is_err(), "and sends it once"); assert_eq!(deselects(&mut app), vec![CORPSE]); + // Skinning comes as wire type 2, as pick-pocketing does (`Player.cpp:8122`); the + // deselect reads the health (`0x48f35d`), so the skinned corpse is deselected ... + open( + &mut app, + CORPSE, + loot_type::PICKPOCKETING, + vec![item(0, 117, 1)], + ); + step(&mut app); + assert!(matches!(rx.try_recv(), Ok(ClientCommand::LootRelease { guid }) if guid == CORPSE)); + assert_eq!( + deselects(&mut app), + vec![CORPSE], + "a skinned corpse loses it" + ); + // ... and a live pickpocket target is not. + open( + &mut app, + LIVE, + loot_type::PICKPOCKETING, + vec![item(0, 117, 1)], + ); + step(&mut app); + assert!(matches!(rx.try_recv(), Ok(ClientCommand::LootRelease { guid }) if guid == LIVE)); + assert!(deselects(&mut app).is_empty(), "a live target keeps it"); + // A fishing bobber is a GameObject: the same path, minus the unit deselect. open(&mut app, BOBBER, loot_type::FISHING, vec![]); step(&mut app); diff --git a/crates/benilla-app/src/ui_loot/net.rs b/crates/benilla-app/src/ui_loot/net.rs index 1ee08954..ce24974f 100644 --- a/crates/benilla-app/src/ui_loot/net.rs +++ b/crates/benilla-app/src/ui_loot/net.rs @@ -84,13 +84,14 @@ fn on_release_response( mut latch: ResMut, mut pending: ResMut, mut lock_cleared: ResMut, + mut dead: super::DeadUnitDeselect, ) { if let SessionEvent::LootReleaseResponse { guid } = ev { let unlock = ItemUnlock { pending: &mut pending, lock_cleared: &mut lock_cleared, }; - loot_release_response(guid, &mut loot, &mut latch, unlock); + dead.after_close(loot_release_response(guid, &mut loot, &mut latch, unlock)); } } @@ -283,17 +284,18 @@ fn loot_clear_money(loot: &mut LootState) { /// as the reference's (`0x5ec0d4`), so an old window's release keeps a newer request's latch. /// It unlocks an opened item (`0x5ec090` → `0x48f200(cl=0, dl=0)` → `UnlockItem 0x495420`): the /// only clear a lockbox closed with loot left gets, as vmangos destroys only a fully looted item -/// (`LootHandler.cpp:558`). +/// (`LootHandler.cpp:558`). The open window takes the shared close without a send (`0x5ec0f3`); +/// the closed source is returned. fn loot_release_response( guid: u64, loot: &mut LootState, latch: &mut LootLatch, unlock: ItemUnlock, -) { +) -> Option { debug!("net: loot released {guid:#x}"); - loot.clear(); latch.clear_for(guid); unlock.unlock(guid); + super::close_interaction(loot, latch, None) } /// `SMSG_LOOT_MASTER_LIST`, staged: it lands just ahead of the response it belongs to. @@ -584,6 +586,8 @@ mod tests { world.insert_resource(LootLatch(Some(LOCKBOX))); world.init_resource::(); world.init_resource::(); + world.init_resource::(); + world.init_resource::>(); let mut pending = PendingItemOps::default(); pending.add([(0, 3, LOCKBOX, 1)]); world.insert_resource(pending); @@ -624,6 +628,65 @@ mod tests { assert!(world.resource::().0.is_empty()); } + /// The server's release (`0x5ec090` → `0x5ec0f3`) closes the window through the shared close, + /// sending nothing back, and its last step deselects the dead unit it closed on. + #[test] + fn a_release_response_closes_the_window_and_deselects_a_dead_unit() { + let (net, rx) = net(); + let mut world = opened_lockbox_world(); + world.insert_resource(net); + let corpse = world + .spawn(( + crate::net::NetEntity { + kind: benilla_protocol::EntityKind::Unit, + display_id: None, + scale: 1.0, + }, + // `UNIT_FIELD_HEALTH` (22) at 0. + crate::net::ObjectStore(benilla_protocol::messages::ObjectFields::from_pairs(&[( + 22, 0, + )])), + )) + .id(); + world + .resource_mut::() + .0 + .insert(CORPSE, corpse); + world + .resource_mut::() + .open(CORPSE, 1, 0, Vec::new()); + world.resource_mut::().0 = Some(CORPSE); + + world + .run_system_once_with( + on_release_response, + SessionEvent::LootReleaseResponse { guid: CORPSE }, + ) + .expect("the handler runs as a one-shot system"); + assert_eq!(world.resource::().source(), None); + assert_eq!(world.resource::().0, None); + assert!(rx.try_recv().is_err(), "the server's release is not echoed"); + let asked: Vec = world + .resource_mut::>() + .drain() + .map(|d| d.0) + .collect(); + assert_eq!(asked, [CORPSE]); + + // A second release finds nothing open and asks nothing. + world + .run_system_once_with( + on_release_response, + SessionEvent::LootReleaseResponse { guid: CORPSE }, + ) + .expect("the handler runs as a one-shot system"); + assert!(world + .resource_mut::>() + .drain() + .next() + .is_none()); + } + /// Only a releasing arm reaches `UnlockItem 0x495420`, through the tail `0x5ebac2`. #[test] fn a_releasing_loot_error_on_the_opened_item_unlocks_it() { diff --git a/crates/benilla-app/src/ui_pet_stats.rs b/crates/benilla-app/src/ui_pet_stats.rs index 9a1ee659..1d1f26a1 100644 --- a/crates/benilla-app/src/ui_pet_stats.rs +++ b/crates/benilla-app/src/ui_pet_stats.rs @@ -582,6 +582,8 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() + .init_resource::() .add_message::() .add_message::() .insert_resource(NetCommands(tx))