From 4ae99c0609fefdede007a7a899c9ca56a85d5242 Mon Sep 17 00:00:00 2001 From: Sam Sung <51686767+samwhosung@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:58:27 +0700 Subject: [PATCH] raid roster: every raid roster packet fires RAID_ROSTER_UPDATE (#708) `RAID_ROSTER_UPDATE` fired only when a member's guid, rank, subgroup or online bit moved. A raid list that changed only a death, or an identical re-send, fired nothing, so the raid pullouts and addons listening on it went stale. After each raid `SMSG_GROUP_LIST` rebuild, the reference's raid roster writer `0x4ba5f0` signals `RAID_ROSTER_UPDATE` whenever no member name is pending (`0x4babdf`-`0x4babef`). A list without the raid flag signals only when it drops a held raid (`0x4ba57b`). `GroupState` now counts these per list, and the feed fires one event per list since its last look, as #691 does for `PARTY_MEMBERS_CHANGED`. The key compare stays as the edge for world entry, which fills our own row. Test: with the default UI and `Blizzard_RaidUI` loaded, a death-only raid list turns the pullout row red. Fixes #690 Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/benilla-app/src/ui_party/feed.rs | 64 +++- .../src/ui_party/feed/roster_update_tests.rs | 357 ++++++++++++++++++ crates/benilla-app/src/ui_party/mod.rs | 154 +++++++- crates/benilla-app/src/ui_party/net.rs | 86 ++++- crates/benilla-app/src/ui_script/test_ui.rs | 2 +- 5 files changed, 648 insertions(+), 15 deletions(-) create mode 100644 crates/benilla-app/src/ui_party/feed/roster_update_tests.rs diff --git a/crates/benilla-app/src/ui_party/feed.rs b/crates/benilla-app/src/ui_party/feed.rs index 9b0e1334..9fa1e110 100644 --- a/crates/benilla-app/src/ui_party/feed.rs +++ b/crates/benilla-app/src/ui_party/feed.rs @@ -44,7 +44,10 @@ pub(super) struct FedParty { raid_units: Vec>, /// `partypet1..4` and `raidpet1..40`, with the `UNIT_PET` edges (see [`pets`]). pets: pets::FedPets, - /// The raid roster's identity (guid, rank, subgroup, online): `RAID_ROSTER_UPDATE`'s edge. + /// The group lists' `RAID_ROSTER_UPDATE`s the feed has answered. + roster_updates: u32, + /// The raid roster's identity (guid, rank, subgroup, online): the edge that stands in for the + /// `RAID_ROSTER_UPDATE` sites no list carries. raid_key: Vec<(u64, u32, u32, bool)>, saved: Vec, saved_answers: u32, @@ -77,6 +80,17 @@ fn members_changed_owed(applied: u32, seen: u32, fresh_vm: bool) -> u32 { } } +/// The `RAID_ROSTER_UPDATE`s the lists applied since the feed's last look owe: one each, as the +/// reference signals once per list (`0x4babef`, `0x4ba57b`). A fresh VM is owed none, since the +/// roster edge gives the raid it finds its one catch-up; a count that ran backwards is a new +/// session's, all of whose lists are new. +fn roster_updates_owed(applied: u32, seen: u32, fresh_vm: bool) -> u32 { + if fresh_vm { + return 0; + } + applied.checked_sub(seen).unwrap_or(applied) +} + /// The `raid1..raid40` unit tokens, one per `MAX_RAID_MEMBERS` (40, `RaidFrame.lua:2`). #[rustfmt::skip] pub(crate) const RAID_TOKENS: [&str; 40] = [ @@ -471,14 +485,26 @@ pub(super) fn feed_party( } // ── RAID_ROSTER_UPDATE ────────────────────────────────────────────────── // - // Fired when the roster's identity moves (members, order, rank, subgroup, online), not on - // level or health, which `RaidGroupFrame_OnEvent` re-reads on `UNIT_LEVEL` and `UNIT_HEALTH`. - // The reference fires it after each raid `SMSG_GROUP_LIST` rebuild (`0x4babef`, or `0x4bada6` - // once the member names it waits on land), on leaving a raid (`0x4ba57b`) and when world entry - // fills in our own row (`0x4ba1a2`). - if raid_key != fed.raid_key { + // The reference signals it from four places, and `GroupState` counts the two that a list + // drives; each list in this drain fires its own. A raid list's roster rebuild signals with no + // member name pending, whatever the list changed (`0x4babef`; nothing gates the signal on a difference), + // and a list that is not a raid signals for the roster it drops (`0x4ba57b`). The other two + // have no list behind them: the last pending name's answer (`0x4bada6`) and world entry filling + // in our own row (`0x4ba1a2`). The roster's identity (members, order, rank, subgroup, online) + // stands in for them when no list signalled: it moves as our own row arrives, and a list held + // back by a pending name fires on it when it moves. Level and health are not in it, which + // `RaidGroupFrame_OnEvent` re-reads on `UNIT_LEVEL` and `UNIT_HEALTH`. + let owed = roster_updates_owed(group.roster_updates, fed.roster_updates, vm_reset); + fed.roster_updates = group.roster_updates; + let roster_moved = raid_key != fed.raid_key; + fed.raid_key = raid_key; + if owed != 0 { + gate.audit("feed_party", "a raid roster list"); + for _ in 0..owed { + script.fire_event("RAID_ROSTER_UPDATE", vec![]); + } + } else if roster_moved { gate.audit("feed_party", "the raid-roster edge"); - fed.raid_key = raid_key; script.fire_event("RAID_ROSTER_UPDATE", vec![]); } @@ -1026,8 +1052,12 @@ fn test_apply_local( self_guid: Option, target_guid: Option, ) -> bool { + let was_raid = group.group_type == GROUPTYPE_RAID; + let handled = apply_test_intent(group, req, self_guid, target_guid); // The server answers each of these with a fresh `SMSG_GROUP_LIST`, which the reference signals - // `PARTY_MEMBERS_CHANGED` for whether or not it changed anything (`0x5e6c61`). + // `PARTY_MEMBERS_CHANGED` for whether or not it changed anything (`0x5e6c61`). A raid's list + // also signals `RAID_ROSTER_UPDATE` (`0x4babef`), and the one that ends a raid for the roster + // it drops (`0x4ba57b`). if matches!( req, PartyRequest::Leave @@ -1043,7 +1073,20 @@ fn test_apply_local( | PartyRequest::AssistantLeader { .. } ) { group.count_list(); + if was_raid || group.group_type == GROUPTYPE_RAID { + group.count_roster_update(); + } } + handled +} + +/// [`test_apply_local`]'s mirror edit: the intent's effect on the group, whatever list answers it. +fn apply_test_intent( + group: &mut GroupState, + req: &PartyRequest, + self_guid: Option, + target_guid: Option, +) -> bool { match req { PartyRequest::Leave => { // The all-zero list's reset: the group facts and sandbox flag, not the session state. @@ -1441,6 +1484,9 @@ pub(crate) fn synthetic_raid( lines } +#[cfg(test)] +mod roster_update_tests; + #[cfg(test)] mod tests { use super::super::pets::tests::{app, app_with, flag, frame, member, party, stream, ME}; diff --git a/crates/benilla-app/src/ui_party/feed/roster_update_tests.rs b/crates/benilla-app/src/ui_party/feed/roster_update_tests.rs new file mode 100644 index 00000000..e13da0d1 --- /dev/null +++ b/crates/benilla-app/src/ui_party/feed/roster_update_tests.rs @@ -0,0 +1,357 @@ +//! `RAID_ROSTER_UPDATE` as the reference signals it: once per group list that rebuilds the raid +//! roster with no member name pending (`0x4babef`) and once per list that drops the roster held +//! (`0x4ba57b`), with the roster edge standing in for the two signals no list carries. + +use super::super::pets::tests::{app, app_with, flag, frame, member, ME}; +use super::*; + +const ON: u8 = member_status::ONLINE; +const DEAD: u8 = member_status::ONLINE | member_status::DEAD; +const GHOST: u8 = member_status::ONLINE | member_status::GHOST; + +/// A frame that counts the `RAID_ROSTER_UPDATE`s the VM is told of; [`fired`] takes them. +fn watch(app: &mut App) { + let script = app.world_mut().non_send_resource_mut::(); + script + .run( + r#" + ROSTER = 0 + local f = CreateFrame("Frame") + f:RegisterEvent("RAID_ROSTER_UPDATE") + f:SetScript("OnEvent", function() ROSTER = ROSTER + 1 end) + "#, + ) + .unwrap(); +} + +/// Run a frame and count the `RAID_ROSTER_UPDATE`s it fired. +fn fired(app: &mut App) -> u32 { + frame(app); + let script = app.world_mut().non_send_resource_mut::(); + let fired: f64 = script.eval("return ROSTER").unwrap(); + script.run("ROSTER = 0").unwrap(); + fired as u32 +} + +/// `SMSG_GROUP_LIST` as the server sends it: members `1..` with their status byte, led by us, of +/// the given group type. `names_pending` is what the name cache told the handler. +fn send(app: &mut App, group_type: u8, statuses: &[u8], names_pending: bool) { + let list = statuses + .iter() + .enumerate() + .map(|(i, &status)| GroupMemberEntry { + name: format!("M{}", i + 1), + guid: member(i as u64 + 1), + status, + flags: 0, + }) + .collect(); + app.world_mut() + .resource_mut::() + .apply_list_awaiting(group_type, 0, list, ME, None, Some(ME), names_pending); +} + +fn raid(app: &mut App, statuses: &[u8]) { + send(app, GROUPTYPE_RAID, statuses, false); +} + +fn party(app: &mut App, statuses: &[u8]) { + send(app, 0, statuses, false); +} + +/// The all-zero list: "not in a group". +fn all_zero(app: &mut App) { + app.world_mut() + .resource_mut::() + .apply_list(0, 0, vec![], 0, None, Some(ME)); +} + +/// `GetNumRaidMembers()`, the rows the VM reads. +fn rows(app: &mut App) -> f64 { + let script = app.world_mut().non_send_resource::(); + script.eval("return GetNumRaidMembers()").unwrap() +} + +/// `GetRaidRosterInfo`'s ninth return for row `row`. +fn dead(app: &mut App, row: u32) -> Option { + flag( + app, + &format!( + "(function() local _, _, _, _, _, _, _, _, dead = GetRaidRosterInfo({row}) \ + return dead end)()" + ), + ) +} + +/// The reported shape: a raid member dies, then releases, out of view. The list that says so moves +/// their status alone, no guid, rank, subgroup or online bit, and the writer signals all the same +/// (`0x4babef`, with no compare against the old roster). +#[test] +fn a_raid_list_that_moves_only_a_members_dead_bit_fires_raid_roster_update() { + let mut app = app(); + watch(&mut app); + raid(&mut app, &[ON, ON]); + assert_eq!(fired(&mut app), 1, "the first look answers the list"); + assert_eq!(fired(&mut app), 0, "a steady frame is silent"); + assert_eq!(dead(&mut app, 1), None); + + raid(&mut app, &[DEAD, ON]); + assert_eq!(fired(&mut app), 1, "member 1 died"); + assert_eq!(dead(&mut app, 1), Some(1.0), "and the roster says so"); + raid(&mut app, &[GHOST, ON]); + assert_eq!(fired(&mut app), 1, "member 1 released"); + assert_eq!(dead(&mut app, 1), None); + assert_eq!(fired(&mut app), 0); +} + +/// Nothing gates the writer's signal on a difference: a list that says what the last said signals. +#[test] +fn an_identical_raid_list_fires_raid_roster_update_again() { + let mut app = app(); + watch(&mut app); + raid(&mut app, &[ON, ON]); + fired(&mut app); + + raid(&mut app, &[ON, ON]); + assert_eq!(fired(&mut app), 1); + raid(&mut app, &[ON, ON]); + assert_eq!(fired(&mut app), 1); + assert_eq!(fired(&mut app), 0, "a frame with no list is silent"); +} + +/// A party's list runs no roster writer: the raid leg of the handler is the only caller (`0x5e6eb4`), +/// and the other leg finds no roster to drop (`0x4ba550`). +#[test] +fn a_party_list_fires_no_raid_roster_update() { + let mut app = app(); + watch(&mut app); + party(&mut app, &[ON, ON]); + assert_eq!(fired(&mut app), 0, "the list that seats the party"); + party(&mut app, &[ON, ON]); + assert_eq!(fired(&mut app), 0, "an identical one"); + party(&mut app, &[DEAD, ON]); + assert_eq!(fired(&mut app), 0, "a status alone"); + all_zero(&mut app); + assert_eq!( + fired(&mut app), + 0, + "and the all-zero list with no raid held" + ); +} + +/// With a member name pending the writer signals nothing (`0x4babdf`); the name answer does later +/// (`0x4bada6`), which is not built, so the roster edge signals a list that moves the roster's +/// identity, as it always has. +#[test] +fn a_raid_list_with_a_name_pending_fires_only_on_the_roster_edge() { + let mut app = app(); + watch(&mut app); + raid(&mut app, &[ON, ON]); + fired(&mut app); + + send(&mut app, GROUPTYPE_RAID, &[DEAD, ON], true); + assert_eq!(fired(&mut app), 0, "a status alone, a name pending"); + send(&mut app, GROUPTYPE_RAID, &[DEAD, ON], true); + assert_eq!(fired(&mut app), 0, "an identical list, a name pending"); + send(&mut app, GROUPTYPE_RAID, &[DEAD, ON, ON], true); + assert_eq!(fired(&mut app), 1, "a member joined: the roster moved"); + send(&mut app, GROUPTYPE_RAID, &[DEAD, ON, ON], false); + assert_eq!(fired(&mut app), 1, "the names held: the list fires"); +} + +/// A list that is not a raid, with a raid held, drops it and signals once (`0x4ba57b`): the raid +/// turned party and the all-zero list alike, each list of the drain apiece. +#[test] +fn a_list_that_drops_the_raid_fires_raid_roster_update_once() { + let mut app = app(); + watch(&mut app); + raid(&mut app, &[ON, ON]); + fired(&mut app); + + party(&mut app, &[ON, ON]); + assert_eq!(fired(&mut app), 1, "the raid became a party"); + party(&mut app, &[ON, ON]); + assert_eq!(fired(&mut app), 0, "no raid held"); + + raid(&mut app, &[ON, ON]); + assert_eq!(fired(&mut app), 1); + all_zero(&mut app); + assert_eq!(fired(&mut app), 1, "the group ended"); + all_zero(&mut app); + assert_eq!(fired(&mut app), 0, "and nothing to end"); + + // In one drain: the list that rebuilds, then the one that drops. + raid(&mut app, &[ON, ON]); + fired(&mut app); + raid(&mut app, &[DEAD, ON]); + all_zero(&mut app); + assert_eq!(fired(&mut app), 2, "one apiece"); +} + +/// Each packet signals, so the lists one drain applies fire once apiece. +#[test] +fn each_raid_list_of_one_drain_fires_its_own_raid_roster_update() { + let mut app = app(); + watch(&mut app); + raid(&mut app, &[ON, ON]); + fired(&mut app); + + raid(&mut app, &[DEAD, ON]); + raid(&mut app, &[DEAD, DEAD]); + raid(&mut app, &[ON, ON]); + assert_eq!(fired(&mut app), 3); +} + +/// World entry fills in our own row (`0x4ba1a2`), with no list behind it: the roster edge fires as +/// our row arrives, once. +#[test] +fn our_own_row_arriving_fires_raid_roster_update_once() { + let mut app = app(); + watch(&mut app); + raid(&mut app, &[ON, ON]); + assert_eq!(fired(&mut app), 1, "the list, before we are in the world"); + assert_eq!(rows(&mut app), 2.0); + + app.world_mut().spawn(( + Guid(ME), + ObjectStore(benilla_protocol::messages::ObjectFields::from_pairs(&[])), + SelfPlayer, + )); + assert_eq!(fired(&mut app), 1, "our row filled in"); + assert_eq!(rows(&mut app), 3.0, "us, then the two"); + assert_eq!(fired(&mut app), 0); +} + +/// A VM minted after the lists landed was told of none: the raid it finds gets its one catch-up +/// from the roster edge, never the history, and a player who holds no raid gets nothing, however +/// many lists ended raids before. +#[test] +fn a_fresh_vm_finds_a_raid_once_and_a_party_not_at_all() { + let mut app = app(); + watch(&mut app); + raid(&mut app, &[ON, ON]); + raid(&mut app, &[DEAD, ON]); + raid(&mut app, &[DEAD, DEAD]); + assert_eq!(fired(&mut app), 1); + assert_eq!(fired(&mut app), 0); + + let mut ended = self::app(); + watch(&mut ended); + raid(&mut ended, &[ON, ON]); + party(&mut ended, &[ON, ON]); + assert_eq!(fired(&mut ended), 0, "the raid ended before the first look"); + + let mut solo = self::app(); + watch(&mut solo); + assert_eq!(fired(&mut solo), 0); +} + +#[test] +fn the_roster_updates_owed_are_the_count_since_the_last_look() { + assert_eq!(roster_updates_owed(3, 3, false), 0, "nothing landed"); + assert_eq!(roster_updates_owed(5, 3, false), 2, "one apiece"); + assert_eq!( + roster_updates_owed(2, 4, false), + 2, + "the session ended and began again" + ); + assert_eq!( + roster_updates_owed(7, 0, true), + 0, + "a fresh VM's catch-up is the roster edge's" + ); +} + +/// The sandbox stands in for the server, whose echo of each group intent is a list: a raid's list +/// signals `RAID_ROSTER_UPDATE` though the intent moves nobody (a loot threshold), the one that +/// ends the raid signals once, and a party's list signals none. +#[test] +fn the_sandbox_answers_a_raid_intent_with_a_raid_roster_update() { + let mut app = app(); + watch(&mut app); + synthetic_roster(&mut app.world_mut().resource_mut::(), None); + assert_eq!(fired(&mut app), 0, "a party's list"); + + let intent = |app: &mut App, req: PartyRequest| { + let mut group = app.world_mut().resource_mut::(); + assert!(test_apply_local(&mut group, &req, Some(ME), None)); + }; + intent(&mut app, PartyRequest::LootThreshold(4)); + assert_eq!(fired(&mut app), 0, "a party intent"); + intent(&mut app, PartyRequest::ConvertToRaid); + assert_eq!(fired(&mut app), 1, "the party became a raid"); + intent(&mut app, PartyRequest::LootThreshold(4)); + assert_eq!(fired(&mut app), 1, "a raid intent that moves no member"); + intent(&mut app, PartyRequest::PromoteUnit("party1".into())); + assert_eq!( + fired(&mut app), + 1, + "once, though the leader's rank moved too" + ); + intent( + &mut app, + PartyRequest::SetRaidTarget { + unit: "player".into(), + index: 1, + }, + ); + assert_eq!(fired(&mut app), 0, "a mark is no list"); + intent(&mut app, PartyRequest::Leave); + assert_eq!(fired(&mut app), 1, "the raid ended"); +} + +/// The stock raid UI, loaded whole off the player's own chain: a raid member dies out of view and +/// the list that says so moves nothing but their dead bit. The raid tab's grid also repaints on +/// `PARTY_MEMBERS_CHANGED` (`RaidFrame_OnEvent`), which every list signals, but a group pullout +/// re-reads its rows on `RAID_ROSTER_UPDATE` alone (`RaidPullout_OnEvent`); this harness sends no +/// member stats, so no `UNIT_HEALTH` repaints the row in its place. +#[test] +fn the_stock_raid_ui_repaints_a_member_who_dies_out_of_view() { + benilla_formats::wow_data_or_skip!(); + let mut script = UiScript::new().unwrap(); + script.set_screen_size(1024.0, 768.0); + let failures = crate::ui_script::load_default_ui(&script); + assert!(failures.is_empty(), "load failures: {failures:?}"); + crate::ui_script::test_ui::seat_chain_addon(&mut script, "Blizzard_RaidUI"); + let mut app = app_with(script); + // A warrior, so the alive colour is the class's and the dead one is not. + app.world_mut().resource_mut::().insert_player( + member(1), + "M1".into(), + Some((1, 1, 0)), + ); + let colour = |app: &mut App, call: &str| { + let (r, g, b): (f32, f32, f32) = app + .world_mut() + .non_send_resource::() + .eval(&format!("return {call}")) + .unwrap(); + [r, g, b].map(|c| (c * 100.0).round() as i32) + }; + let text_colour = |app: &mut App| colour(app, "RaidGroupButton1Name:GetTextColor()"); + let pullout_colour = |app: &mut App| colour(app, "RaidPullout1Button1Name:GetTextColor()"); + let (warrior, red) = ([78, 61, 43], [100, 10, 10]); + + // The first list loads the raid UI (`RaidFrame_LoadUI`); the drag on group 1's label, whose + // handler runs with `this` set, opens the group's pullout (`Blizzard_RaidUI.xml:172`). + raid(&mut app, &[ON, ON]); + frame(&mut app); + app.world_mut() + .non_send_resource_mut::() + .run("this = RaidGroup1Label RaidPullout_GenerateGroupFrame()") + .unwrap(); + assert_eq!(text_colour(&mut app), warrior, "the grid's row"); + assert_eq!(pullout_colour(&mut app), warrior, "the pullout's row"); + + raid(&mut app, &[DEAD, ON]); + frame(&mut app); + assert_eq!(text_colour(&mut app), red, "the grid repaints"); + assert_eq!(pullout_colour(&mut app), red, "and so does the pullout"); + + raid(&mut app, &[ON, ON]); + frame(&mut app); + assert_eq!(text_colour(&mut app), warrior); + assert_eq!(pullout_colour(&mut app), warrior); + let errors = app.world().non_send_resource::().errors(); + assert!(errors.is_empty(), "script errors: {errors:?}"); +} diff --git a/crates/benilla-app/src/ui_party/mod.rs b/crates/benilla-app/src/ui_party/mod.rs index 8f121cd7..66ffc4e8 100644 --- a/crates/benilla-app/src/ui_party/mod.rs +++ b/crates/benilla-app/src/ui_party/mod.rs @@ -56,6 +56,12 @@ pub struct GroupState { /// reference signals `PARTY_MEMBERS_CHANGED` after each (`0x5e6c61`), with no compare against /// the roster it holds, and an empty list reaches it too (`0x5e6b3c`). pub lists_applied: u32, + /// How many `RAID_ROSTER_UPDATE`s the lists signalled, which the feed answers one for one. A raid + /// list signals after its roster rebuild once no member name is pending (`0x4babef`), with no + /// compare against the roster it replaces; a list that is not a raid signals once for the roster + /// it drops (`0x4ba57b`). A raid list with a name pending signals nothing then, and the name + /// answer signals later (`0x4bada6`). + pub roster_updates: u32, /// The leave ack, `SMSG_GROUP_UNINVITE` and `SMSG_GROUP_DESTROYED` empty the reference's party /// slots and leader (`0x4e84a0`, `0x4e8250(0)`) and keep its raid roster; the next list then /// finds no slot held. @@ -97,7 +103,8 @@ pub struct ListOutcome { impl GroupState { /// Apply one `SMSG_GROUP_LIST` and return what it shows, in the reference's order: the party /// slots' joins, the new-group line, the party slots' leaves, then the raid roster's lines. - /// Kick, leave (`0x5e690b`) and disband lines come from their opcodes. + /// Kick, leave (`0x5e690b`) and disband lines come from their opcodes. Every name is taken as + /// held: [`Self::apply_list_awaiting`] is the one that says otherwise. pub fn apply_list( &mut self, group_type: u8, @@ -106,6 +113,24 @@ impl GroupState { leader: u64, loot: Option, self_guid: Option, + ) -> ListOutcome { + self.apply_list_awaiting( + group_type, own_flags, members, leader, loot, self_guid, false, + ) + } + + /// [`Self::apply_list`] told what the name cache holds: `names_pending` when a row of a raid + /// list has no cached name yet, the reference's pending count (`[0xb713f0]`, raised at `0x4ba9d0` + /// for each row whose name lookup misses), which holds `RAID_ROSTER_UPDATE` back (`0x4babdf`). + pub fn apply_list_awaiting( + &mut self, + group_type: u8, + own_flags: u8, + members: Vec, + leader: u64, + loot: Option, + self_guid: Option, + names_pending: bool, ) -> ListOutcome { // Every path of the handler reaches the event, the leave list and a party's list alike. self.count_list(); @@ -173,9 +198,15 @@ impl GroupState { } else { out.lines.push(UiError::key("ERR_RAID_YOU_JOINED")); } + // The rebuild ends in the signal unless a name is pending (`0x4babdf`-`0x4babef`). + if !names_pending { + self.count_roster_update(); + } } else if raid_held { - // `0x4ba550`: a list that is not a raid drops the held roster (`0x4ba55f`). + // `0x4ba550`: a list that is not a raid drops the held roster (`0x4ba55f`), and says so + // (`0x4ba57b`). With no roster held it signals no `RAID_ROSTER_UPDATE`. out.lines.push(UiError::key("ERR_RAID_YOU_LEFT")); + self.count_roster_update(); } // The all-zero list means "not in a group" (vmangos `Server/Packets/Group.h:257`); only @@ -212,6 +243,11 @@ impl GroupState { self.lists_applied = self.lists_applied.wrapping_add(1); } + /// Count one `RAID_ROSTER_UPDATE` a list signals, which the feed answers with one. + pub(super) fn count_roster_update(&mut self) { + self.roster_updates = self.roster_updates.wrapping_add(1); + } + /// The `party1..party4` slots: our own subgroup in packet order, at most four (`0x5e6baa` /// compares the flags under `0x7f`). A slot's member can change on any resync. pub fn party_slots(&self) -> impl Iterator { @@ -389,6 +425,7 @@ impl GroupState { pending_invite, // Session state the feed fires edges on: kept until `clear_session`. lists_applied: _, + roster_updates: _, saved_instances: _, saved_instances_answers: _, ready_check: _, @@ -519,6 +556,119 @@ mod tests { assert_eq!(g.lists_applied, 0); } + /// The raid roster writer signals after each rebuild (`0x4babef`), and nothing gates the signal + /// on a difference from the roster it replaces, so a list that moves a member's status alone, or + /// nothing, counts. + #[test] + fn every_raid_list_counts_a_roster_update_whatever_it_changes() { + use benilla_protocol::messages::member_status; + let mut g = GroupState::default(); + let raid = |g: &mut GroupState, members: Vec| { + g.apply_list(1, 0, members, 1, None, Some(ME)); + g.roster_updates + }; + assert_eq!(g.roster_updates, 0, "no list yet"); + + let (alice, bob) = (sub("Alice", 1, 0), sub("Bob", 2, 1)); + assert_eq!( + raid(&mut g, vec![alice.clone(), bob.clone()]), + 1, + "seats the raid" + ); + assert_eq!( + raid(&mut g, vec![alice.clone(), bob.clone()]), + 2, + "the same list again" + ); + let dead = GroupMemberEntry { + status: member_status::ONLINE | member_status::DEAD, + ..bob + }; + assert_eq!(raid(&mut g, vec![alice.clone(), dead]), 3, "a status alone"); + assert_eq!(raid(&mut g, vec![alice]), 4, "a member gone"); + assert_eq!( + raid(&mut g, vec![]), + 5, + "a raid list naming nobody still rebuilds" + ); + + g.leave_group(); + assert_eq!(g.roster_updates, 5, "leaving is not a list"); + g.clear_session(); + assert_eq!(g.roster_updates, 0); + } + + /// A list that is not a raid runs no roster writer, so a party's list signals nothing; one that + /// finds a raid held drops it and signals once (`0x4ba57b`), the all-zero list among them, and + /// with none held there is nothing to say. + #[test] + fn a_list_that_is_not_a_raid_counts_a_roster_update_only_for_the_raid_it_drops() { + let mut g = GroupState::default(); + let list = |g: &mut GroupState, group_type: u8, members: Vec, leader| { + g.apply_list(group_type, 0, members, leader, None, Some(ME)); + g.roster_updates + }; + let party = vec![sub("Alice", 1, 0)]; + + assert_eq!(list(&mut g, 0, party.clone(), 1), 0, "a party's list"); + assert_eq!(list(&mut g, 0, party.clone(), 1), 0, "and again"); + assert_eq!( + list(&mut g, 0, vec![], 0), + 0, + "the all-zero list with no raid held" + ); + + assert_eq!( + list(&mut g, 1, party.clone(), 1), + 1, + "the party becomes a raid" + ); + assert_eq!( + list(&mut g, 0, party.clone(), 1), + 2, + "the raid becomes a party" + ); + assert_eq!(list(&mut g, 0, party.clone(), 1), 2, "nothing held to drop"); + + assert_eq!(list(&mut g, 1, party, 1), 3); + assert_eq!( + list(&mut g, 0, vec![], 0), + 4, + "the all-zero list ends the raid" + ); + assert_eq!( + list(&mut g, 0, vec![], 0), + 4, + "and a second has none to end" + ); + } + + /// The rebuild counts the rows whose name is still to come and signals only with none + /// (`0x4babdf`); the list lands all the same, and a list that drops a raid asks no name. + #[test] + fn a_raid_list_with_a_name_pending_counts_no_roster_update() { + let mut g = GroupState::default(); + let raid = vec![sub("Alice", 1, 0), sub("Bob", 2, 1)]; + let awaiting = |g: &mut GroupState, group_type: u8, pending: bool| { + g.apply_list_awaiting(group_type, 0, raid.clone(), 1, None, Some(ME), pending); + g.roster_updates + }; + + assert_eq!(awaiting(&mut g, 1, true), 0, "a name pending"); + assert_eq!( + g.group_type, GROUPTYPE_RAID, + "the roster is rebuilt regardless" + ); + assert_eq!(g.members.len(), 2); + assert_eq!(awaiting(&mut g, 1, false), 1, "every name held"); + assert_eq!(awaiting(&mut g, 1, true), 1, "pending again"); + assert_eq!( + awaiting(&mut g, 0, true), + 2, + "the drop does not wait on a name" + ); + } + #[test] fn leaving_the_group_keeps_the_session_tickets() { let lockout = benilla_protocol::messages::RaidInstanceEntry { diff --git a/crates/benilla-app/src/ui_party/net.rs b/crates/benilla-app/src/ui_party/net.rs index 521e313f..838e1398 100644 --- a/crates/benilla-app/src/ui_party/net.rs +++ b/crates/benilla-app/src/ui_party/net.rs @@ -10,7 +10,7 @@ use bevy::prelude::*; use benilla_protocol::{SessionEvent, SessionEventKind}; -use super::GroupState; +use super::{GroupState, GROUPTYPE_RAID}; use crate::names::NameCache; use crate::net::{ClientCommand, GuidIndex, NetCommands, NetHandlerApp, ObjectStore, SelfGuid}; use crate::sound::MessageSounds; @@ -238,6 +238,9 @@ fn leader_changed( /// `SMSG_GROUP_LIST`: apply the roster, seat new members' records and re-ask the questgiver sweep /// (shared-quest availability follows the roster). Every member is name-queried once: the /// answer's race, class and gender are the only source of them for a member we never see streamed. +/// A raid's rebuild counts the rows whose name is still to come (`0x4ba9b6`-`0x4ba9d0`), ours +/// among them once others are listed (`0x5e6e73`), and signals `RAID_ROSTER_UPDATE` only with none +/// (`0x4babdf`-`0x4babef`). fn list( group: &mut GroupState, errors: &mut UiErrorKeys, @@ -253,8 +256,14 @@ fn list( index: &GuidIndex, net_commands: &NetCommands, ) { + let mut names_pending = false; for m in &members { - let _ = names.resolve(m.guid, net_commands); + names_pending |= names.resolve(m.guid, net_commands).is_none(); + } + if group_type == GROUPTYPE_RAID && !members.is_empty() { + if let Some(own) = self_guid.0 { + names_pending |= names.resolve(own, net_commands).is_none(); + } } // Taken before `apply_list` consumes the list; `seat_new_records` then treats a member with // no record as new, the reference's `srcRec == 0`. @@ -262,7 +271,15 @@ fn list( .iter() .map(|m| (m.guid, m.status & member_status::ONLINE != 0)) .collect(); - let shown = group.apply_list(group_type, own_flags, members, leader, loot, self_guid.0); + let shown = group.apply_list_awaiting( + group_type, + own_flags, + members, + leader, + loot, + self_guid.0, + names_pending, + ); push_group_lines(errors, shown.lines); if shown.invite_accept { sounds.push_cue(INVITE_ACCEPT_SOUND); @@ -448,6 +465,69 @@ mod tests { assert_eq!(names.player_traits(leader), Some((1, 4, 1))); } + /// The rebuild counts the rows whose name has not landed (`0x4ba9b6`-`0x4ba9d0`), ours among + /// them once others are listed (`0x5e6e73`), and the roster event waits for none (`0x4babdf`). + #[test] + fn a_raid_list_signals_the_roster_only_once_every_name_is_held() { + let (tx, _rx) = crossbeam_channel::unbounded(); + let net = NetCommands(tx); + let (mut group, mut errors, mut quest) = ( + GroupState::default(), + UiErrorKeys::default(), + QuestGiver::default(), + ); + let mut names = NameCache::default(); + let player_guid = |counter: u64| counter | (u64::from(guid::HIGH_PLAYER) << 48); + let (me, a, b) = (player_guid(1), player_guid(7), player_guid(8)); + let mut raid = |group: &mut GroupState, names: &NameCache, group_type: u8| { + list( + group, + &mut errors, + &mut MessageSounds::default(), + &mut quest, + group_type, + 0, + vec![member(a, "Aldwyn"), member(b, "Brisca")], + a, + None, + &SelfGuid(Some(me)), + names, + &GuidIndex::default(), + &net, + ); + group.roster_updates + }; + + assert_eq!(raid(&mut group, &names, 1), 0, "no name held"); + names.insert_player(a, "Aldwyn".into(), Some((1, 4, 1))); + assert_eq!( + raid(&mut group, &names, 1), + 0, + "one member's name still to come" + ); + names.insert_player(b, "Brisca".into(), Some((1, 5, 0))); + assert_eq!( + raid(&mut group, &names, 1), + 0, + "our own row's name still to come" + ); + names.insert_player(me, "Me".into(), None); + assert_eq!(raid(&mut group, &names, 1), 1, "every name held"); + assert_eq!(raid(&mut group, &names, 1), 2, "and the same list again"); + + // A party's list runs no roster writer; it names nothing, and drops the raid it finds. + assert_eq!( + raid(&mut group, &NameCache::default(), 0), + 3, + "the raid dropped" + ); + assert_eq!( + raid(&mut group, &NameCache::default(), 0), + 3, + "a party's list" + ); + } + #[test] fn a_join_queues_the_invite_accept_chime_and_a_resync_does_not() { let (tx, _rx) = crossbeam_channel::unbounded(); diff --git a/crates/benilla-app/src/ui_script/test_ui.rs b/crates/benilla-app/src/ui_script/test_ui.rs index 73da98d2..f2436e9b 100644 --- a/crates/benilla-app/src/ui_script/test_ui.rs +++ b/crates/benilla-app/src/ui_script/test_ui.rs @@ -811,7 +811,7 @@ fn world_point(s: &mut UiScript) -> (f32, f32) { /// Seat one of the reference's LoadOnDemand Blizzard addons off the chain, so a harness's /// `UIParentLoadAddOn(name)` loads it as the app does. Needs client data. -pub(super) fn seat_chain_addon(s: &mut UiScript, name: &str) { +pub(crate) fn seat_chain_addon(s: &mut UiScript, name: &str) { let toc = super::reference_ui::read(&format!("Interface/AddOns/{name}/{name}.toc")) .map(|b| benilla_ui::toc::Toc::parse(&benilla_ui::source::decode(&b))) .unwrap_or_else(|| panic!("{name}: no toc off the chain"));