diff --git a/crates/benilla-app/src/ui_battlefield_score.rs b/crates/benilla-app/src/ui_battlefield_score.rs index 297049ef..a9203b02 100644 --- a/crates/benilla-app/src/ui_battlefield_score.rs +++ b/crates/benilla-app/src/ui_battlefield_score.rs @@ -91,7 +91,7 @@ fn resolve_board( pub(crate) fn feed_battlefield_score( script: Option>, - board: Res, + mut board: ResMut, mut queue: ResMut, catalog: Option>, names: Res, @@ -104,20 +104,38 @@ pub(crate) fn feed_battlefield_score( let now = Instant::now(); script.set_battlefield_run_time_ms(queue.run_time_ms(now)); - let fresh = board.log.as_ref().and_then(|log| { - let rows = resolve_board(log, &names, &commands)?; - let columns = queue - .active_map() - .zip(catalog.as_deref()) - .map(|(map, cat)| score_columns(cat, map)) - .unwrap_or_default(); - Some(BattlefieldScores { - rows, - ended: log.ended, - winner: log.winner.unwrap_or(0), - columns, + // Leaving the battleground ends the board, and `SMSG_BATTLEFIELD_STATUS` is the packet that + // says so: the active slot's map to zero, or that same slot losing its instance. The queue + // handler already zeroes the run-time clocks on both, so the winner goes with them. Left set, + // `GetBattlefieldWinner()` answers the finished battleground's winner in the open world, the + // stock frame shows the end-of-battleground panel again on the next score event + // (`WorldStateFrame.lua:351-355`), and `LeaveBattlefield` targets a map nobody is in — the + // button that cannot leave anything. + let left = queue.take_board_cleared(); + if left { + *board = BattlefieldScoreboard::default(); + } + + let fresh = if left { + // The empty board has to be pushed, not just withheld: the VM keeps whatever board it was + // last handed, so a clear the feed does not push leaves `GetBattlefieldWinner()` standing. + Some(BattlefieldScores::default()) + } else { + board.log.as_ref().and_then(|log| { + let rows = resolve_board(log, &names, &commands)?; + let columns = queue + .active_map() + .zip(catalog.as_deref()) + .map(|(map, cat)| score_columns(cat, map)) + .unwrap_or_default(); + Some(BattlefieldScores { + rows, + ended: log.ended, + winner: log.winner.unwrap_or(0), + columns, + }) }) - }); + }; let last = last.get(&script); let status_rebuild = queue.take_score_dirty(); let changed = fresh.is_some() && fresh != *last; @@ -204,6 +222,7 @@ impl Plugin for BattlefieldScorePlugin { mod tests { use super::*; use benilla_formats::{WorldStateUiCatalog, WorldStateUiRow}; + use benilla_protocol::messages::BattlefieldStatus; fn row(map_id: u32, ui_type: u32, text: &str) -> WorldStateUiRow { WorldStateUiRow { @@ -271,4 +290,126 @@ mod tests { assert!(board.log.is_none()); assert!(board.last_request.is_none()); } + + fn status(slot: u32, map_id: u32, in_progress: Option<(u32, u32)>) -> BattlefieldStatus { + BattlefieldStatus { + slot, + map_id, + in_progress, + bracket: 0, + instance_id: 0, + status: 0, + time_ms: None, + queued: None, + } + } + + /// The regression: a finished battleground, the worldport out of it, and the end-of-battleground + /// panel back in the open world with a leave button that has nothing to leave. + /// + /// Stock Lua gates the panel on `GetBattlefieldWinner()` (`WorldStateFrame.lua:311-318`, + /// `:351-355`) and only ever hides the panel itself on `PLAYER_ENTERING_WORLD` or a tab click + /// — which the reference fires once per session, not per teleport. So the board has to stop + /// answering a winner the moment the player is out of the battleground, and the empty board has + /// to be pushed to the VM rather than withheld, because the VM keeps whatever it was last handed. + #[test] + fn leaving_the_battleground_stops_the_board_answering_a_winner() { + use crate::ui_script::world_state_tests::harness; + + benilla_formats::wow_data_or_skip!(); + let mut app = App::new(); + app.init_resource::() + .init_resource::() + .init_resource::(); + let (tx, _rx) = crossbeam_channel::unbounded(); + app.insert_resource(crate::net::NetCommands(tx)); + app.insert_non_send_resource(harness()); + app.add_systems(Update, feed_battlefield_score); + + let winner = |app: &mut App| { + app.world_mut() + .non_send_resource_mut::() + .eval::>("return GetBattlefieldWinner()") + .expect("the VM answers") + }; + + // A finished Alterac Valley, empty roster: no name lookups stand between the packet and + // the board, so the feed pushes it on the first frame. + app.world_mut() + .resource_mut::() + .apply(PvpLogData { + ended: true, + winner: Some(1), + rows: Vec::new(), + }); + app.world_mut() + .resource_mut::() + .apply(status(0, 471, Some((0, 420_000)))); + app.update(); + assert_eq!( + winner(&mut app), + Some(1), + "inside the battleground: the board answers" + ); + + // The leave lands. The same slot, its map gone to zero. + app.world_mut() + .resource_mut::() + .apply(status(0, 0, None)); + app.update(); + assert_eq!( + winner(&mut app), + None, + "out of the battleground: the winner must be gone, or the panel returns" + ); + assert!( + app.world() + .resource::() + .log + .is_none(), + "the finished board must not outlive the instance" + ); + } + + /// The other packet that ends an instance: the slot survives with its map but loses + /// `in_progress` (`WorldStateFrame`'s `GetBattlefieldStatus` third value). + #[test] + fn losing_the_instance_zeroes_the_board_too() { + use crate::ui_script::world_state_tests::harness; + + benilla_formats::wow_data_or_skip!(); + let mut app = App::new(); + app.init_resource::() + .init_resource::() + .init_resource::(); + let (tx, _rx) = crossbeam_channel::unbounded(); + app.insert_resource(crate::net::NetCommands(tx)); + app.insert_non_send_resource(harness()); + app.add_systems(Update, feed_battlefield_score); + + app.world_mut() + .resource_mut::() + .apply(PvpLogData { + ended: true, + winner: Some(1), + rows: Vec::new(), + }); + app.world_mut() + .resource_mut::() + .apply(status(0, 471, Some((0, 420_000)))); + app.update(); + app.world_mut() + .resource_mut::() + .apply(status(0, 471, None)); + app.update(); + + assert!( + app.world_mut() + .non_send_resource_mut::() + .eval::>("return GetBattlefieldWinner()") + .expect("the VM answers") + .is_none(), + "the instance is gone, so the winner is gone" + ); + } } diff --git a/crates/benilla-app/src/ui_dialog_verbs.rs b/crates/benilla-app/src/ui_dialog_verbs.rs index cebb87bb..3a7af2eb 100644 --- a/crates/benilla-app/src/ui_dialog_verbs.rs +++ b/crates/benilla-app/src/ui_dialog_verbs.rs @@ -185,6 +185,10 @@ pub(crate) struct BattlefieldQueue { instance_expiration: Option, /// Status 3 fires `UPDATE_BATTLEFIELD_SCORE` before `_STATUS` (`0x4aaa5a`, then `0x4aab05`). score_dirty: bool, + /// The player left the battleground: the active slot lost its instance, or the same slot's map + /// went to zero. The handler already zeroes the run-time clocks on both, so the scoreboard's + /// winner goes with them — [`crate::ui_battlefield_score::feed_battlefield_score`] takes this. + board_cleared: bool, /// Tutorials the handler triggers (`0x2f` on queued, `0x30` on confirm), owed to the next feed. tutorials: Vec, } @@ -205,6 +209,7 @@ impl BattlefieldQueue { if self.active.is_some_and(|(i, _)| i == index) { self.run_started = None; self.instance_expiration = None; + self.board_cleared = true; } *slot = None; self.changed = true; @@ -231,6 +236,7 @@ impl BattlefieldQueue { self.instance_expiration = None; if self.active.is_some_and(|(i, _)| i == index) { self.active = None; + self.board_cleared = true; } } } @@ -269,6 +275,11 @@ impl BattlefieldQueue { std::mem::take(&mut self.score_dirty) } + /// Take the "the player left the battleground" mark. One packet, one board to zero. + pub(crate) fn take_board_cleared(&mut self) -> bool { + std::mem::take(&mut self.board_cleared) + } + /// The map id `AcceptBattlefieldPort` sends for a 1-based slot, if the slot holds a queue. fn map_id(&self, index: u8) -> Option { self.slots diff --git a/crates/benilla-app/src/ui_script/world_state_tests.rs b/crates/benilla-app/src/ui_script/world_state_tests.rs index 8643244e..c7749e2e 100644 --- a/crates/benilla-app/src/ui_script/world_state_tests.rs +++ b/crates/benilla-app/src/ui_script/world_state_tests.rs @@ -1,7 +1,9 @@ //! The always-up world-state readout, stock `WorldStateFrame.lua`, fed as the client feeds it: //! rows pushed, then `UPDATE_WORLD_STATES`. Which rows exist is [`crate::world_state_ui`]'s. -use benilla_ui::script::{QuadContent, ScriptValue, UiScript, WorldStateUiView}; +use benilla_ui::script::{ + BattlefieldScoreRow, BattlefieldScores, QuadContent, ScriptValue, UiScript, WorldStateUiView, +}; use super::test_ui::load_ui as load_xml; @@ -265,6 +267,54 @@ fn a_capture_point_row_is_a_bar_not_a_line_of_text() { assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } +/// The end-of-battleground modal is `WorldStateScoreFrame`, and the stock handler has exactly one +/// hide: `PLAYER_ENTERING_WORLD` (`WorldStateFrame.lua:226-230`). Every other arrival that reaches +/// `WorldStateScoreFrame_OnEvent` re-checks `GetBattlefieldWinner()` and re-runs the update while it +/// is non-nil (`:221-224`), and the update shows the panel again on a non-nil winner (`:351-355`). +/// +/// So stock Lua offers the board as the only defence: whatever keeps `GetBattlefieldWinner()` +/// answering after the worldport brings the modal back, with its leave button. This pins that +/// contract; the fix is on the board, in +/// [`crate::ui_battlefield_score::tests::leaving_the_battleground_stops_the_board_answering_a_winner`]. +#[test] +fn the_panel_returns_whilever_the_board_answers_a_winner() { + benilla_formats::wow_data_or_skip!(); + let mut s = harness(); + s.fire_event("PLAYER_ENTERING_WORLD", vec![ScriptValue::Str("".into())]); + let shown = |s: &mut UiScript| { + s.eval::("return WorldStateScoreFrame:IsShown()") + .expect("the score frame answers") + }; + + s.set_battlefield_scores(BattlefieldScores { + rows: vec![BattlefieldScoreRow { + name: "Bob".into(), + faction: 0, + ..Default::default() + }], + ended: true, + winner: 1, + columns: Vec::new(), + }); + s.tick(0.0); + s.fire_event("UPDATE_BATTLEFIELD_SCORE", vec![]); + assert!(shown(&mut s), "the finished board shows the modal"); + + // The worldport out of the battleground: the one hide stock has. + s.fire_event("PLAYER_ENTERING_WORLD", vec![ScriptValue::Str("".into())]); + assert!(!shown(&mut s), "PLAYER_ENTERING_WORLD hides the modal"); + + // The server re-sends the world states after the teleport, and the readout has changed with the + // map, so `UPDATE_WORLD_STATES` follows — with no board change, no score event in between. + push(&mut s, Vec::new()); + assert!( + shown(&mut s), + "the panel is gated on the winner alone: a board that still answers one puts the modal back \ + in the open world, which is why the board has to be zeroed on the way out" + ); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} + /// An out-of-range index answers the single number 0 and a non-number argument raises /// (`0x4c5a40`, `0x4c5a70`). #[test]