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

fix: close a loot window that opens while you move, as 1.12.1 does (#786)

Browse Source 
- A loot window that opens while you move closes, as in 1.12.1 (#782)
- Set auto_loot in the LootConfig initializer (clippy field_reassign_with_default)
- Close a window opened on the move in the feed's pass, so it never paints

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Sam <samuv.eth@gmail.com>
This commit is contained in:
Mathih13
2026-10-03 17:36:21 +07:00
committed by GitHub
co-authored by Claude Opus 5.5 Sam
parent b396bbf69a
commit cf891dc375
6 changed files with 334 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/benilla-app/src/creature_anim/select.rs
+4 -1
View File
@@ -111,10 +111,13 @@ pub(crate) mod move_flags {
/// Any horizontal direction bit: the client's `[9e8] & 0xf` gate.
pub const ANY_MOVE: u32 = FORWARD | BACKWARD | STRAFE_LEFT | STRAFE_RIGHT;
/// The low byte: forward, back, strafe, turn and pitch, the loot response's test (`0x5ebc40`).
pub const STEERING: u32 = 0xff;
/// The bits that get a mover integrated at all, the client's `0x20ff` (`0x616e20`, `0x6166f5`):
/// without one a mover keeps its last packet's pose, and a flag-less unit is not even in the
/// mover list (`0x618940`). Swim and the mode bits are not among them.
pub const INTEGRATED: u32 = 0xff | FALLING;
pub const INTEGRATED: u32 = STEERING | FALLING;
/// The committed-lower-body test routing a one-shot to the masked overlay (`0x5fe6dc`); the
/// client's separate mouse-turn test (`d58 & 0x1800`) reads a field benilla does not model.
crates/benilla-app/src/game_plugins.rs
+2
View File
@@ -1149,6 +1149,8 @@ pub(crate) mod schedule_tests {
"both packets answer the `CMSG_LOGOUT_REQUEST`/cancel the game menu sent"),
("ui_loot/mod.rs", "drain_loot", Because::Deliberate,
"the pre-VM take of `LootMoveStart` is documented at the line and publishes nothing to the VM; the event-firing takes are Lua's queues"),
("ui_loot/mod.rs", "feed_loot", Because::PlayerRoundTrip,
"it takes `LootMoveStart` only on a window it has just opened, which answers a loot click"),
("ui_loot_roll.rs", "drain_loot_rolls", Because::FilledByVm,
"only a Need/Greed/Pass click queues a confirm or vote, held in the VM"),
("ui_loot_roll.rs", "feed_loot_rolls", Because::PlayerRoundTrip,
crates/benilla-app/src/target/click.rs
+1 -1
View File
@@ -1430,7 +1430,7 @@ pub(crate) fn service_action(
/// Drain the guid-scoped deselects ([`DeselectGuid`]). `ClearTarget()`, the last leg of the Esc
/// chain (`UIParent.lua:1492`), is a script call and lands in call order
/// ([`crate::script_calls`]).
pub(super) fn clear_target_requests(
pub(crate) fn clear_target_requests(
mut selection: ResMut<Selection>,
mut seam: crate::creature_anim::AttackSeam,
engaged: Query<(), (With<Engaged>, With<SelfPlayer>)>,
crates/benilla-app/src/ui_loot/mod.rs
+39 -1
View File
@@ -363,7 +363,8 @@ impl LootLatch {
/// A movement start that closes the loot: the reference's guard `0x60e990`, called by every
/// movement-start emitter, turning included, but not by mouse-look facing. No loot target reaches
/// the range gate `0x493230`, so there is no distance leash. vmangos also releases on movement
/// (`MovementHandler.cpp:1104`), but the close is the client's.
/// (`MovementHandler.cpp:1104`), but the close is the client's. A loot response that lands while
/// we move sets it too (`0x5ebc3a`).
#[derive(Resource, Default)]
pub(crate) struct LootMoveStart(pub(crate) bool);
@@ -674,6 +675,7 @@ fn feed_loot(
group: Res<GroupState>,
names: Res<NameCache>,
objects: LootSourceObjects,
mut moving: OpenWhileMoving,
) {
let Some(mut script) = script else {
return;
@@ -755,6 +757,19 @@ fn feed_loot(
}
}
}
// The response's tail (`0x5ebc3a`-`0x5ebc51`) closes a window that opened while we
// move, after the copier's open and sweep and in the same handler: `LOOT_CLOSED` fires
// in this pass, so the window never paints.
if std::mem::take(&mut moving.move_start.0) {
let closed = close_on_move_start(&mut loot, &mut moving.latch, &commands);
if closed.is_some() {
moving.dead.after_close(closed);
script.set_loot(None);
script.fire_event("LOOT_CLOSED", vec![]);
*last = None;
return;
}
}
}
// One `LOOT_SLOT_CLEARED` per row that went, with its 1-based row: the stock handler hides
// that button and pages down when the page empties (`LootFrame.lua:22-50`).
@@ -862,6 +877,14 @@ fn close_on_move_start(
close_interaction(loot, latch, Some(commands))
}
/// What [`feed_loot`] needs to take the move-start close on a window it has just opened.
#[derive(bevy::ecs::system::SystemParam)]
struct OpenWhileMoving<'w, 's> {
move_start: ResMut<'w, LootMoveStart>,
latch: ResMut<'w, LootLatch>,
dead: DeadUnitDeselect<'w, 's>,
}
/// Sends what the Lua asked for, as the take dispatcher `0x4c2790(slot, flag)`: a row click
/// (a `LootButton`'s own, flag 0) and the `LOOT_BIND` confirm (`LootSlot`, flag 1) stay apart,
/// so a second click on a bind-on-pickup row asks again.
@@ -1170,6 +1193,11 @@ mod tests {
let (tx, _rx) = crossbeam_channel::unbounded();
let mut app = App::new();
app.add_message::<crate::sound::LootPickupSound>()
// What the feed's move-start close reads (`OpenWhileMoving`).
.add_message::<crate::target::DeselectGuid>()
.init_resource::<LootMoveStart>()
.init_resource::<LootLatch>()
.init_resource::<crate::net::GuidIndex>()
.init_resource::<LootState>()
.init_resource::<crate::ui_chat::ChatLog>()
.init_resource::<GroupState>()
@@ -1227,6 +1255,11 @@ mod tests {
let (tx, rx) = crossbeam_channel::unbounded();
let mut app = App::new();
app.add_message::<crate::sound::LootPickupSound>()
// What the feed's move-start close reads (`OpenWhileMoving`).
.add_message::<crate::target::DeselectGuid>()
.init_resource::<LootMoveStart>()
.init_resource::<LootLatch>()
.init_resource::<crate::net::GuidIndex>()
.init_resource::<LootState>()
.init_resource::<crate::ui_chat::ChatLog>()
.init_resource::<GroupState>()
@@ -1460,6 +1493,11 @@ mod tests {
let (tx, rx) = crossbeam_channel::unbounded();
let mut app = App::new();
app.add_message::<crate::sound::LootPickupSound>()
// What the feed's move-start close reads (`OpenWhileMoving`).
.add_message::<crate::target::DeselectGuid>()
.init_resource::<LootMoveStart>()
.init_resource::<LootLatch>()
.init_resource::<crate::net::GuidIndex>()
.init_resource::<LootState>()
.init_resource::<crate::ui_chat::ChatLog>()
.init_resource::<GroupState>()
crates/benilla-app/src/ui_loot/net.rs
+287 -36
View File
@@ -5,7 +5,7 @@ use benilla_protocol::messages::ItemPushResult;
use benilla_protocol::{SessionEvent, SessionEventKind};
use bevy::prelude::*;
use super::{LootLatch, LootState};
use super::{LootLatch, LootMoveStart, LootState};
use crate::net::{ClientCommand, NetCommands, NetHandlerApp, SelfGuid};
use crate::pending_item_ops::{LockTransitions, PendingItemOps};
use crate::ui_action::{UiError, UiErrorKeys};
@@ -26,8 +26,15 @@ pub(super) fn register(app: &mut App) {
}
/// An admitted response selects a looted unit (`0x5ebc35` → `0x48f3a0`) before the window opens
/// (`0x4c1cb0`), so the outgoing target's teardown closes a window still open on it.
fn on_response(In(ev): In<SessionEvent>, mut select: crate::target::SelectCommit) {
/// (`0x4c1cb0`), so the outgoing target's teardown closes a window still open on it. One that lands
/// while our own body moves takes the move-start close (`0x5ebc3a`-`0x5ebc51`), which the feed runs
/// after it fires `LOOT_OPENED`, or the drain when the window waits on an item template.
fn on_response(
In(ev): In<SessionEvent>,
mut select: crate::target::SelectCommit,
player: Option<Res<crate::player::Player>>,
mut move_start: ResMut<LootMoveStart>,
) {
let SessionEvent::LootResponse {
guid,
loot_type,
@@ -47,6 +54,12 @@ fn on_response(In(ev): In<SessionEvent>, mut select: crate::target::SelectCommit
);
select.select_unit(guid);
select.seam.loot.open(guid, loot_type, gold, items);
if player.is_some_and(|p| {
p.foreign_mover.is_none()
&& p.move_flags() & crate::creature_anim::move_flags::STEERING != 0
}) {
move_start.0 = true;
}
}
fn on_error(
@@ -763,6 +776,8 @@ mod tests {
world.init_resource::<LockTransitions>();
world.init_resource::<bevy::ecs::message::Messages<crate::target::DeselectGuid>>();
world.init_resource::<bevy::ecs::message::Messages<crate::player::StandStateRequest>>();
world.init_resource::<crate::player::Player>();
world.init_resource::<super::super::LootMoveStart>();
world.spawn((
SelfPlayer,
Guid(1),
@@ -988,34 +1003,33 @@ mod tests {
);
}
/// The issue's symptom on the stock frames: the Tab target is on `TargetFrame` when the body's
/// window opens, and the unit feed then names the body there (`TargetFrame.lua:63`,
/// `UnitFrame.lua:25`).
#[test]
fn the_stock_target_frame_names_the_looted_body() {
/// The stock unit frames over `TargetFrame`, for [`stock_app`].
const TARGET_UI: &[&str] = &[
"Interface\\FrameXML\\Fonts.xml",
"Interface\\FrameXML\\GlobalStrings.lua",
"Interface\\FrameXML\\UIParent.xml",
"Interface\\FrameXML\\BasicControls.xml",
"Interface\\FrameXML\\MoneyFrame.lua",
"Interface\\FrameXML\\MoneyFrame.xml",
"Interface\\FrameXML\\GameTooltip.xml",
"Interface\\FrameXML\\UIDropDownMenu.xml",
"Interface\\FrameXML\\TextStatusBar.lua",
"Interface\\FrameXML\\TextStatusBar.xml",
"Interface\\FrameXML\\BuffFrame.xml",
"Interface\\FrameXML\\CombatFeedback.xml",
"Interface\\FrameXML\\UnitPopup.xml",
"Interface\\FrameXML\\UnitFrame.xml",
"Interface\\FrameXML\\PlayerFrame.xml",
"Interface\\FrameXML\\PartyFrame.xml",
"Interface\\FrameXML\\TargetFrame.xml",
];
/// [`tabbed_world`] under the stock `ui` files, with the unit feed and both creatures named.
fn stock_app(ui: &[&[&str]]) -> App {
use benilla_ui::script::UiScript;
benilla_formats::wow_data_or_skip!();
let mut s = UiScript::new().expect("VM");
s.set_screen_size(1024.0, 768.0);
for f in crate::ui_script::test_ui::production_order(&[&[
"Interface\\FrameXML\\Fonts.xml",
"Interface\\FrameXML\\GlobalStrings.lua",
"Interface\\FrameXML\\UIParent.xml",
"Interface\\FrameXML\\BasicControls.xml",
"Interface\\FrameXML\\MoneyFrame.lua",
"Interface\\FrameXML\\MoneyFrame.xml",
"Interface\\FrameXML\\GameTooltip.xml",
"Interface\\FrameXML\\UIDropDownMenu.xml",
"Interface\\FrameXML\\TextStatusBar.lua",
"Interface\\FrameXML\\TextStatusBar.xml",
"Interface\\FrameXML\\BuffFrame.xml",
"Interface\\FrameXML\\CombatFeedback.xml",
"Interface\\FrameXML\\UnitPopup.xml",
"Interface\\FrameXML\\UnitFrame.xml",
"Interface\\FrameXML\\PlayerFrame.xml",
"Interface\\FrameXML\\PartyFrame.xml",
"Interface\\FrameXML\\TargetFrame.xml",
]]) {
for f in crate::ui_script::test_ui::production_order(ui) {
crate::ui_script::test_ui::load_ui(&s, f);
}
@@ -1048,20 +1062,257 @@ mod tests {
.init_resource::<crate::ui_guild::GuildState>()
.insert_non_send_resource(s);
crate::ui_unit::add_unit_feed(&mut app);
let shown = |app: &mut App| -> String {
let s = app.world().non_send_resource::<UiScript>();
assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
s.eval("return TargetFrame:IsShown() and TargetFrame.name:GetText() or ''")
.expect("the target frame's name")
};
app
}
/// The name on the stock `TargetFrame`, empty while it is hidden.
fn target_frame_name(app: &App) -> String {
let s = app
.world()
.non_send_resource::<benilla_ui::script::UiScript>();
assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
s.eval("return TargetFrame:IsShown() and TargetFrame.name:GetText() or ''")
.expect("the target frame's name")
}
/// The issue's symptom on the stock frames: the Tab target is on `TargetFrame` when the body's
/// window opens, and the unit feed then names the body there (`TargetFrame.lua:63`,
/// `UnitFrame.lua:25`).
#[test]
fn the_stock_target_frame_names_the_looted_body() {
benilla_formats::wow_data_or_skip!();
let mut app = stock_app(&[TARGET_UI]);
app.update();
assert_eq!(shown(&mut app), "Kobold Vermin", "the Tab target");
assert_eq!(target_frame_name(&app), "Kobold Vermin", "the Tab target");
app.world_mut().resource_mut::<LootLatch>().0 = Some(CORPSE);
respond(app.world_mut(), CORPSE, 1);
app.update();
assert_eq!(shown(&mut app), "Kobold Worker", "the looted body");
assert_eq!(target_frame_name(&app), "Kobold Worker", "the looted body");
}
/// What [`super::super::drain_loot`] reads beside [`seat_tabbed`]'s, with our body's movement
/// word at `player`'s.
fn seat_drain(world: &mut World, player: crate::player::Player) {
world.insert_resource(player);
world.init_resource::<crate::ui_party::GroupState>();
world.init_resource::<crate::items::Items>();
world.init_resource::<bevy::ecs::message::Messages<crate::sound::LootPickupSound>>();
}
/// The deselects asked for since the last call.
fn deselects(world: &mut World) -> Vec<u64> {
world
.resource_mut::<bevy::ecs::message::Messages<crate::target::DeselectGuid>>()
.drain()
.map(|d| d.0)
.collect()
}
/// A response on the corpse with `player` seated, then one drain.
fn open_while(
player: crate::player::Player,
) -> (World, crossbeam_channel::Receiver<ClientCommand>) {
let (mut world, rx) = tabbed_world();
seat_drain(&mut world, player);
world.resource_mut::<LootLatch>().0 = Some(CORPSE);
respond(&mut world, CORPSE, 1);
world
.run_system_once(super::super::drain_loot)
.expect("the drain runs as a one-shot system");
(world, rx)
}
/// The response's tail (`0x5ebc3a`-`0x5ebc51`): a bit of our movement word's low byte, forward,
/// back, strafe, turn or pitch, takes the move-start close on the window it just opened, the
/// release after the selection and the dead body's deselect last (`0x48f369`).
#[test]
fn a_window_that_opens_while_moving_closes_as_a_movement_start_does() {
for bit in (0..8).map(|b| 1u32 << b) {
let (mut world, rx) = open_while(crate::player::Player::with_move_flags(bit));
assert_eq!(world.resource::<LootState>().source(), None, "{bit:#x}");
assert_eq!(world.resource::<LootLatch>().0, None, "{bit:#x}");
let sent: Vec<_> = rx.try_iter().collect();
assert!(
matches!(
sent[..],
[ClientCommand::SetSelection { guid: a }, ClientCommand::LootRelease { guid: b }]
if a == CORPSE && b == CORPSE
),
"{bit:#x}: {sent:?}"
);
assert_eq!(deselects(&mut world), [CORPSE], "{bit:#x}");
}
}
/// Only that byte, and only our body's: walking, falling or swimming alone leaves the window
/// open, as does a step of the creature we possess.
#[test]
fn a_window_opened_falling_swimming_or_possessing_stays_open() {
use crate::creature_anim::move_flags as f;
use crate::player::Player;
let mut possessing = Player::with_move_flags(f::FORWARD);
possessing.foreign_mover = Some(KOBOLD);
for (what, player) in [
("still", Player::with_move_flags(0)),
("walk mode", Player::with_move_flags(f::WALK_MODE)),
("falling", Player::with_move_flags(f::FALLING)),
("swimming", Player::with_move_flags(f::SWIMMING)),
("possessing", possessing),
] {
let (mut world, rx) = open_while(player);
assert_eq!(
world.resource::<LootState>().source(),
Some(CORPSE),
"{what}"
);
let sent: Vec<_> = rx.try_iter().collect();
assert!(
matches!(sent[..], [ClientCommand::SetSelection { guid }] if guid == CORPSE),
"{what}: {sent:?}"
);
assert!(deselects(&mut world).is_empty(), "{what}");
}
}
/// The copier `0x4c1cb0` sweeps an auto-loot window (`0x4c1fa0`) before the response's
/// movement close (`0x5ebc51`), so a body auto-looted on the run is taken, then released.
#[test]
fn auto_loot_on_the_run_takes_the_rows_before_the_release() {
const TOUGH_JERKY: u32 = 117;
let mut app = App::new();
let rx = seat_tabbed(app.world_mut());
seat_drain(
app.world_mut(),
crate::player::Player::with_move_flags(crate::creature_anim::move_flags::FORWARD),
);
app.world_mut()
.resource_mut::<crate::items::Items>()
.insert_template(
TOUGH_JERKY,
Some(crate::items::test_template("Tough Jerky")),
);
app.insert_resource(super::super::LootConfig {
auto_loot: true,
..Default::default()
})
.init_resource::<crate::ui_chat::ChatLog>()
.init_resource::<crate::names::NameCache>()
.init_resource::<ButtonInput<KeyCode>>()
.insert_non_send_resource(benilla_ui::script::UiScript::new().expect("VM"))
.add_systems(
Update,
(super::super::feed_loot, super::super::drain_loot).chain(),
);
app.world_mut().resource_mut::<LootLatch>().0 = Some(CORPSE);
app.world_mut()
.run_system_once_with(
on_response,
SessionEvent::LootResponse {
guid: CORPSE,
loot_type: 1,
gold: 0,
items: vec![benilla_protocol::messages::LootItem {
slot: 0,
item_id: TOUGH_JERKY,
count: 1,
display_info_id: 1117,
random_property_id: 0,
slot_type: 0,
}],
},
)
.expect("the handler runs as a one-shot system");
app.update();
let sent: Vec<_> = rx.try_iter().collect();
assert!(
matches!(
sent[..],
[
ClientCommand::SetSelection { .. },
ClientCommand::AutostoreLootItem { slot: 0 },
ClientCommand::LootRelease { guid },
] if guid == CORPSE
),
"{sent:?}"
);
}
/// The issue's spot on the stock frames: a body's window that opens on the run is shown by
/// `LOOT_OPENED` and hidden by `LOOT_CLOSED` in the same frame (`LootFrame.lua:13-16`,
/// `:53-55`), its one release
/// sent, and `TargetFrame` lets the body go; standing, the window and the target stay.
#[test]
fn the_stock_loot_frame_closes_on_a_body_looted_on_the_run() {
use crate::creature_anim::move_flags as f;
use benilla_ui::script::UiScript;
benilla_formats::wow_data_or_skip!();
for (flags, stays) in [(f::FORWARD, false), (0, true)] {
let mut app = stock_app(&[
crate::ui_script::test_ui::LOOT_UI,
TARGET_UI,
&["Interface\\FrameXML\\LootFrame.xml"],
]);
let (tx, rx) = crossbeam_channel::unbounded();
app.insert_resource(crate::net::NetCommands(tx));
seat_drain(
app.world_mut(),
crate::player::Player::with_move_flags(flags),
);
app.init_resource::<super::super::LootConfig>()
.init_resource::<ButtonInput<KeyCode>>()
.add_systems(
Update,
(
super::super::feed_loot,
super::super::drain_loot,
crate::target::click::clear_target_requests,
)
.chain(),
);
app.world()
.non_send_resource::<UiScript>()
.run(
"LOOT_EVENTS = {} local f = CreateFrame('Frame') \
f:RegisterEvent('LOOT_OPENED') f:RegisterEvent('LOOT_CLOSED') \
f:SetScript('OnEvent', function() tinsert(LOOT_EVENTS, event) end)",
)
.unwrap();
app.update();
app.world_mut().resource_mut::<LootLatch>().0 = Some(CORPSE);
respond(app.world_mut(), CORPSE, 1);
// One frame: the reference opens and closes in the response's handler.
app.update();
let (events, shown): (String, bool) = app
.world()
.non_send_resource::<UiScript>()
.eval("return table.concat(LOOT_EVENTS, ','), LootFrame:IsShown() == 1")
.unwrap();
for _ in 0..2 {
app.update();
}
let releases = rx
.try_iter()
.filter(|c| matches!(c, ClientCommand::LootRelease { guid } if *guid == CORPSE))
.count();
if stays {
assert_eq!(events, "LOOT_OPENED");
assert!(shown, "standing, the window stays");
assert_eq!(releases, 0);
assert_eq!(target_frame_name(&app), "Kobold Worker");
} else {
assert_eq!(events, "LOOT_OPENED,LOOT_CLOSED");
assert!(
!shown,
"on the run, the window closes in the frame it opens"
);
assert_eq!(releases, 1, "released once");
assert_eq!(target_frame_name(&app), "", "and the body is let go");
}
}
}
/// Only a releasing arm reaches `UnlockItem 0x495420`, through the tail `0x5ebac2`.
crates/benilla-app/src/ui_script/test_ui.rs
+1 -1
View File
@@ -501,7 +501,7 @@ pub(crate) const GOSSIP_UI: &[&str] = &[
/// lacks what the app loads at setup (the master-loot menu reads `GROUP` and `GIVE_LOOT`),
/// `ItemButtonTemplate.xml` because a missing template only warns, and `PartyFrame.xml` because
/// `LootFrame.lua:217` reads `MAX_PARTY_MEMBERS` at load. Needs client data.
pub(super) const LOOT_UI: &[&str] = &[
pub(crate) const LOOT_UI: &[&str] = &[
"Interface\\FrameXML\\GlobalStrings.lua",
"Interface\\FrameXML\\Fonts.xml",
"Interface\\FrameXML\\BasicControls.xml", // `TEXT`, which UnitPopup.lua reads at file scope