diff --git a/crates/benilla-app/assets/ui/OptionsFrame.xml b/crates/benilla-app/assets/ui/OptionsFrame.xml index f13c88c5..012362b5 100644 --- a/crates/benilla-app/assets/ui/OptionsFrame.xml +++ b/crates/benilla-app/assets/ui/OptionsFrame.xml @@ -223,7 +223,8 @@ -- -- · **`UpdateNameplates`.** `UIOptionsFrame.lua` l.768-780 — the reference's own home -- for it, and the four verbs it calls (Show/HideNameplates, Show/HideFriendNameplates) - -- are ours in `script/cvars.rs`, unchanged. + -- are ours in `script/cvars.rs`, unchanged. **One line of that body came back here** + -- (decision 2132) — see the re-declaration below; the de-duplication otherwise stands. -- -- ── THE ONE ASSIGNMENT THAT STAYS, AND WHY IT IS AN OVERRIDE ──────────────────────────── -- `AUTO_QUEST_WATCH` is the string `"1"` ON PURPOSE (decision 1136): the reference assigns @@ -288,6 +289,42 @@ end end + -- ── `UpdateNameplates` — OURS AGAIN, and the deviation is ONE LINE (decision 2132) ────── + -- The stock body is `UIOptionsFrame.lua` l.768-780, running off the chain since 2115. + -- This file loads after it (manifest l.21 vs this window's seat), so this re-declaration + -- wins. It is transcribed verbatim except that l.776 is dropped, and both halves of the + -- reason are benilla facts, not taste: + -- + -- · **l.774-776 is a stock typo.** `if ( FRIENDNAMEPLATES_ON ) then + -- ShowFriendNameplates(); HideFriendNameplates();` — show, then hide, so BOTH branches + -- end hidden. On 1.12 that only clears volatile engine bits, and the friendly global is + -- not even saved (its l.182 registers `NAMEPLATES_ON` a second time where + -- `FRIENDNAMEPLATES_ON` was meant). On benilla the four verbs write the CVar pair, and + -- the CVar pair IS the store (`vplates.rs`, `script/cvars.rs`) — so those two lines + -- erase a persisted setting at every world entry and untick this window's own + -- Friendly Name Plates row. + -- · **There is no volatile level here to replay into.** The reference clears + -- `[0xc4da34]` on every EnterWorld and this function restores it; `VPlateMode` is never + -- cleared, so the replay has nothing to restore. What it still owes is the other + -- direction — an addon that sets `NAMEPLATES_ON` and calls this expects plates to move + -- — which is why the body stays rather than becoming a no-op (1203's posture). + -- + -- The globals it reads are kept true by the host (`vplates::push_plate_globals`), seeded + -- ahead of `VARIABLES_LOADED` on the load edge, so in the steady state every branch here + -- writes a value the CVar already holds and queues nothing at all. + function UpdateNameplates() + if ( NAMEPLATES_ON ) then + ShowNameplates() + else + HideNameplates() + end + if ( FRIENDNAMEPLATES_ON ) then + ShowFriendNameplates() + else + HideFriendNameplates() + end + end + -- The static category tree (era Game tab, Classic tocs), in list order. Keys are the row -- name suffixes; the visible label doubles as the page title (the era does the same — diff --git a/crates/benilla-app/src/sound/mod.rs b/crates/benilla-app/src/sound/mod.rs index 9a2a18ea..eab5cd30 100644 --- a/crates/benilla-app/src/sound/mod.rs +++ b/crates/benilla-app/src/sound/mod.rs @@ -451,9 +451,15 @@ impl Default for AudioListener { /// The world soundscape is live: in the world AND seated on the avatar ([`Player::active`]). /// The state half is the session boundary — the world's followers must not keep tracking (or /// restarting) its audio from the glue screens after a logout. The seated half covers the edges: -/// after a logout the camera — and with it [`benilla_world::terrain_stream::CurrentArea`] — still sits at -/// the old spot until the next login's take-control, and following it would start the *previous* -/// session's soundscape for those frames. +/// after a logout the camera still sits at the old spot until the next login's take-control, +/// and following it would start the *previous* session's soundscape for those frames. +/// +/// It used to have to carry [`benilla_world::terrain_stream::CurrentArea`] too — that resource +/// had no way back to `None`, so it held the last character's zone across the whole boundary. +/// Decision 2130 gave it one (the area authority follows the body, and there is no body at a +/// glue screen), so this gate is about the camera alone now. The zone-channel walk was paying +/// for that same staleness one module over, behind its own private guard — which is what made +/// the lifetime the bug rather than either consumer. fn world_audio_live( state: Res>, player: Res, diff --git a/crates/benilla-app/src/tutorial.rs b/crates/benilla-app/src/tutorial.rs index bff5ebd0..c6f0bee1 100644 --- a/crates/benilla-app/src/tutorial.rs +++ b/crates/benilla-app/src/tutorial.rs @@ -143,8 +143,6 @@ pub(crate) struct Tutorials { /// Published ids (`id + 1`) whose event and cue are owed this frame. fired: Vec, sends: Vec, - /// Bank B changed since the VM last saw it. - dirty: bool, /// Item pushes for us since the last feed (`0x491a60`'s sites): `(entry, bag, slot)`. pushes: Vec<(u32, u8, u32)>, } @@ -154,7 +152,6 @@ impl Tutorials { pub(crate) fn apply_flags(&mut self, bytes: &[u8]) { self.fire_once = Some(bytes.to_vec()); self.acknowledged = Some(bytes.to_vec()); - self.dirty = true; } /// The bring-up `0x4b5330`: both banks zeroed if present (the bit count stays). @@ -165,7 +162,6 @@ impl Tutorials { { bank.iter_mut().for_each(|b| *b = 0); } - self.dirty = true; } /// The teardown `0x4b5380`: every pending timer flushed. @@ -206,7 +202,6 @@ impl Tutorials { set_bank_bit(bank_a, id); } self.sends.push(TutorialSend::Flag(id)); - self.dirty = true; } /// `ClearTutorials()`: the timers flushed, every bit of both banks set, the empty clear sent. @@ -219,7 +214,6 @@ impl Tutorials { bank.iter_mut().for_each(|b| *b = 0xFF); } self.sends.push(TutorialSend::Clear); - self.dirty = true; } /// `ResetTutorials()`: every bit of both banks cleared — the timers NOT flushed — the empty @@ -232,7 +226,6 @@ impl Tutorials { bank.iter_mut().for_each(|b| *b = 0); } self.sends.push(TutorialSend::Reset); - self.dirty = true; } /// `ProcessTutorialTimers`: every due node fires the same cue and event. @@ -285,7 +278,7 @@ const KEYRING_BAG: u8 = 0xff; const KEYRING_SLOTS: std::ops::RangeInclusive = 0x51..=0x70; /// Before the script tick: the sites' asks, the descriptor edges, the timers, the cue and the -/// event, and the acknowledged bank's push to the VM. +/// event, and the acknowledged bank's push to whichever VM is live. fn feed_tutorials( script: Option>, mut tutorials: ResMut, @@ -293,6 +286,7 @@ fn feed_tutorials( self_q: Query<&ObjectStore, With>, items: Res, mut sounds: ResMut, + mut told: Local>>>, ) { let now = Instant::now(); for ask in asks.read() { @@ -338,7 +332,16 @@ fn feed_tutorials( vec![ScriptValue::Int(i64::from(published))], ); } - if std::mem::take(&mut tutorials.dirty) { + // **The bank is owed to every VM, not to the process** (decision 2131). What the acknowledged + // bank holds is the app's; whether a VM has been *told* it is that VM's, so the memory sits + // behind a [`crate::ui_script::VmMemo`] and expires with the session it was written against + // (1290/1291 — a `/reload` is a logout and a login back to back). A plain "changed since the + // last push" flag is the same mistake 2113 names: it answers "did the bank move?" when the + // question is "does THIS VM know it?", and after a reload the answer was no for the rest of + // the session. + let told = told.get(&script); + if *told != tutorials.acknowledged { + told.clone_from(&tutorials.acknowledged); script.set_tutorial_bank(tutorials.acknowledged.clone()); } } @@ -702,4 +705,82 @@ mod tests { assert_eq!(bank_bit(a.unwrap(), 31), Some(false)); assert_eq!(bank_bit(a.unwrap(), 32), None, "past a 4-byte bank"); } + + /// A bare app around [`feed_tutorials`] — the one system that owes the VM the bank. + fn feeder() -> App { + let mut app = App::new(); + app.init_resource::() + .init_resource::() + .init_resource::() + .add_message::() + .add_systems(Update, feed_tutorials); + app + } + + fn enabled(app: &mut App) -> bool { + app.world_mut() + .non_send_resource_mut::() + .eval::("return TutorialsEnabled() ~= nil") + .unwrap() + } + + /// **The bank is owed to every VM** (decision 2131) — the `/reload` bug, reproduced. + /// + /// `ReloadUI()` is `end_ui_session` + the entry load back to back (1290/1291), and only the + /// first of those mints a VM: the world-entry path that fills the banks + /// ([`Tutorials::world_enter`]) never runs, so a push gated on "the bank moved" never fires + /// again. `TutorialsEnabled()` then answers nil for the rest of the session, the Show + /// Tutorials row loads OFF, and ticking it back on passes `OptionsFrame_SetTutorialsEnabled`'s + /// `~=` guard into `ResetTutorials()` — a `CMSG_TUTORIAL_RESET` that re-arms, account-wide, + /// every popup the player had already dismissed. + #[test] + fn a_rebuilt_vm_is_told_the_acknowledged_bank_again() { + let mut app = feeder(); + // The packet lands: one unacknowledged bit is all `TutorialsEnabled()` scans for. + let mut bank = vec![0xFFu8; 32]; + bank[0] = 0xFE; + app.world_mut() + .resource_mut::() + .apply_flags(&bank); + + app.insert_non_send_resource(UiScript::new().expect("VM")); + app.update(); + assert!( + enabled(&mut app), + "the VM that was live when the bank landed" + ); + + // `ReloadUI()`: a fresh VM, and nothing touches the app's banks. + app.insert_non_send_resource(UiScript::new().expect("VM")); + assert!( + !enabled(&mut app), + "a fresh VM starts knowing no bank — this is the state the row used to load in" + ); + app.update(); + assert!(enabled(&mut app), "…and must be told it again"); + } + + /// The push is per-VM, not per-frame: a steady session hands the bank over once. + #[test] + fn an_unchanged_bank_is_not_re_pushed_every_frame() { + let mut app = feeder(); + app.world_mut() + .resource_mut::() + .apply_flags(&[0u8; 32]); + app.insert_non_send_resource(UiScript::new().expect("VM")); + app.update(); + // A bank the VM has already been told is not handed over again — proven by clearing it + // behind the feed's back and watching the next frame leave it cleared. + app.world_mut() + .non_send_resource_mut::() + .set_tutorial_bank(None); + app.update(); + assert!(!enabled(&mut app), "unchanged ⇒ nothing pushed"); + // A real change is still pushed, on the same VM. + app.world_mut() + .resource_mut::() + .acknowledge(id::CHATTING); + app.update(); + assert!(enabled(&mut app), "a moved bank reaches the live VM"); + } } diff --git a/crates/benilla-app/src/ui_chat.rs b/crates/benilla-app/src/ui_chat.rs index 2d5c3eca..09bb9ee8 100644 --- a/crates/benilla-app/src/ui_chat.rs +++ b/crates/benilla-app/src/ui_chat.rs @@ -179,6 +179,12 @@ impl Plugin for UiChatPlugin { // drop that stays in-world for the reconnect. The disconnect twin is chained BEFORE the // walk so a drop and a walk landing on the same frame cannot re-diff against membership // the drop just invalidated. + // + // **After `AreaAuthoritySet`** (decision 2130), like the zone-text feed and the breath + // classifier — the other two systems that act on the leaf area. The walk turns the zone + // into packets, so reading last frame's answer is not a cosmetic lag: it joined the + // previous character's capital at login and then left it again, and the leave took the + // stock `ChatFrame_OnEvent`'s channel registration with it. .add_systems( Update, ( @@ -186,7 +192,8 @@ impl Plugin for UiChatPlugin { channels::auto_join_zone_channels .run_if(in_state(crate::char_select::ClientState::InWorld)), ) - .chain(), + .chain() + .after(benilla_world::terrain_stream::AreaAuthoritySet), ) .add_systems( OnExit(crate::char_select::ClientState::InWorld), diff --git a/crates/benilla-app/src/ui_chat/broadcast.rs b/crates/benilla-app/src/ui_chat/broadcast.rs index 9f4f4a34..d77c5f91 100644 --- a/crates/benilla-app/src/ui_chat/broadcast.rs +++ b/crates/benilla-app/src/ui_chat/broadcast.rs @@ -153,9 +153,7 @@ pub(crate) fn defense_targets( player_zone: Option, ) -> Vec { channels - .joined - .iter() - .flatten() + .iter_names() .filter(|name| { let Some(row) = channels.channels.row_for_name(name) else { return false; // a custom channel — no DBC row, no defense flag @@ -165,7 +163,7 @@ pub(crate) fn defense_targets( } row.flags & chan_flags::ZONE_DEP == 0 || player_zone == Some(subject_zone) }) - .cloned() + .map(str::to_string) .collect() } @@ -337,7 +335,10 @@ mod tests { fn state(joined: &[&str]) -> ChannelState { ChannelState { - joined: joined.iter().map(|n| Some((*n).to_string())).collect(), + joined: joined + .iter() + .map(|n| Some(crate::ui_chat::edit::ChannelSlot::joined(n))) + .collect(), channels: catalog(), ..Default::default() } diff --git a/crates/benilla-app/src/ui_chat/channels.rs b/crates/benilla-app/src/ui_chat/channels.rs index 6a5e4dfa..1482f083 100644 --- a/crates/benilla-app/src/ui_chat/channels.rs +++ b/crates/benilla-app/src/ui_chat/channels.rs @@ -37,7 +37,7 @@ use benilla_formats::ChatChannelsCatalog; use crate::area::AreaTableRes; use crate::net::{ClientCommand, NetCommands}; -use super::edit::ChannelState; +use super::edit::{ChannelState, SlotState}; /// `AreaTable.dbc` flag `0x08` — vmangos calls it `AREA_FLAG_SLAVE_CAPITAL`, with the comment /// *"Allow trade channel"* (`src/game/Database/DBCEnums.h:58`). In the shipped 5875 table exactly @@ -149,7 +149,7 @@ pub(super) fn seed_channels( return; }; if seeded.claim(&script) { - script.set_joined_channels(channels.joined.clone()); + script.set_joined_channels(channels.names()); } } @@ -215,15 +215,49 @@ pub(crate) fn wanted_channels( in_city: bool, city_word: Option<&str>, ) -> Vec { + composed_auto_rows(catalog, zone_name, city_word) + .filter(|(r, _)| in_city || !r.is_city_only()) + .map(|(_, name)| name) + .collect() +} + +/// **Every auto-join row that COMPOSES for this zone, city gate deliberately not applied** — +/// the set that takes slots (decision 2137). +/// +/// The reference registers the slot (`0x49b980`) *before* it asks whether the row is eligible: +/// `0x49a50d` precedes the city gate at `0x49a512` (wow-re `zone-chat-channel-autojoin.md` §7 pass +/// 2, §11). So a character standing outside a capital still holds a `Trade - City` slot — created, +/// never joined, state 3 — and it still occupies **number 2**, because the `N.` a channel line +/// carries is `slot[+0x00]`, the client-local index, never the `ChatChannels.dbc` ChannelID. +/// +/// [`wanted_channels`] is the subset the gate then lets us actually join. Filtering the city rows +/// out *before* they could take a number is what made our `/2` reach LocalDefense where the +/// reference's reaches Trade. +pub(crate) fn tracked_channels( + catalog: &ChatChannelsCatalog, + zone_name: &str, + city_word: Option<&str>, +) -> Vec { + composed_auto_rows(catalog, zone_name, city_word) + .map(|(_, name)| name) + .collect() +} + +/// The shared half: the auto-join rows whose name can actually be composed here, in table order. +/// Both callers derive from this so the resolvability rules cannot drift apart — the city gate is +/// the *only* thing that separates them. +fn composed_auto_rows<'a>( + catalog: &'a ChatChannelsCatalog, + zone_name: &'a str, + city_word: Option<&'a str>, +) -> impl Iterator + 'a { catalog .auto_join_rows() - .filter(|r| in_city || !r.is_city_only()) - .filter(|r| !r.takes_city_name() || city_word.is_some()) + .filter(move |r| !r.takes_city_name() || city_word.is_some()) // A zone-dependent name with no zone to put in it would join a channel called // "General - " — join nothing rather than something wrong. - .filter(|r| !r.is_zone_dependent() || !zone_name.is_empty()) - .map(|r| r.joinable_name(zone_name, city_word.unwrap_or_default())) - .collect() + .filter(move |r| !r.is_zone_dependent() || !zone_name.is_empty()) + .map(move |r| (r, r.joinable_name(zone_name, city_word.unwrap_or_default()))) } /// Is the zone under the player the one they are actually standing in? @@ -267,9 +301,12 @@ fn zone_is_settled(player: Option<&crate::player::Player>) -> bool { /// /// **Selection:** the client's live predicate is the *saved* `ZONECHANNELS` mask, not `flags & 1` /// read fresh — but that mask is **seeded from `flags & 1` when there is no usable chat-cache -/// file** (`0x4997fc`, gated `0x4997e8`). We persist no chat cache, so every session is that -/// fresh-character path and reading the DBC bit directly is exactly right. It stops being right -/// the day per-window channel settings are persisted. +/// file** (`0x4997fc`, gated `0x4997e8`). We now persist that file and seat the mask from it +/// ([`super::edit::ChannelState::zone_mask`], decision 2120), but the walk still reads the DBC bit +/// directly: making the mask the join predicate means a character whose mask is 0 joins nothing at +/// all, which 2120 deliberately left for its own change once the repair had swept through. Named +/// here rather than left implicit — it is the one place this file diverges from the reference on +/// purpose. /// Every `ChatChannels.dbc` row as the VM needs it (decision 1908; wow-re chat-cache-grammar.md /// §5-6): the id, the Shortcut the verbs compare a typed name against, the name composed for /// `zone_name` — `None` when the composition has nothing to substitute (a zone-dependent row with @@ -301,7 +338,7 @@ pub(crate) fn zone_channel_catalog( #[allow(clippy::too_many_arguments)] // a Bevy system's param list IS its dependency set pub(super) fn auto_join_zone_channels( commands: Res, - channels: Res, + mut channels: ResMut, areas: Option>, world: benilla_world::world_point::WorldPoint, // One param (clippy's argument ceiling), and they belong together: both answer "is the zone @@ -322,63 +359,84 @@ pub(super) fn auto_join_zone_channels( if entered.read().next().is_some() { walk.live = true; } - if !walk.live { - return; // no character session — see `ZoneChannelWalk::live` - } + // **A cinematic suppresses the rejoin, and it resumes when the shot ends.** Two of the ten // sites that read the reference's cinematic-state cell exist for exactly this — `0x49491e` // (the zone-text update) and `0x5ff566` (a `UPDATEFLAGS` reflex) both skip // `ZoneChannelRefresh` (`0x49a210`) while one runs, and `EndCinematic` calls it once at // `0x48f1d0` (wow-re `ui/scratch/cinematic-camera-law.md` §3.3, the complete 10-site census). // - // The walk stays armed, so "rejoin once at the end" is what falling through here the next - // frame already does — there is nothing to re-arm. It matters because a race intro flies the - // streaming focus hundreds of yards off the body, so `world.area()` changes under a player - // who has not moved, and the joins would otherwise fire against zones they are only - // *looking* at. - if cinematic.as_deref().is_some_and(|c| c.is_playing()) { - return; - } - let (Some(areas), false) = (areas, channels.channels.is_empty()) else { - return; - }; - // Not until the world under the player is the one they are actually standing in — see the - // "Timing" note above. - if !zone_is_settled(player.as_deref()) { - return; - } - // The zone under the player, or nothing to do yet (tiles not streamed, no body). - let Some(zone_row) = world - .area() - .and_then(|leaf| areas.0.top_zone(leaf)) - .and_then(|zone| areas.0.get(zone)) - else { - return; - }; - let at = ( - zone_row.name.clone(), - zone_row.flags & AREA_FLAG_TRADE_CHANNEL != 0, - ); - // The VM's zone-channel catalog — what `JoinChannelByName` resolves a typed name against and - // `EnumerateServerChannels` lists — is a function of the same zone; refed on a zone change - // and on a fresh VM, keyed like every other feed (1290). - if let Some(mut script) = script { - let fed = catalog_fed.get(&script); - if fed.as_ref() != Some(&at) { - script.set_zone_channel_catalog(zone_channel_catalog( - &channels.channels, - &at.0, - at.1, - city_word(&areas.0), - )); - *fed = Some(at.clone()); + // The walk stays armed and only its *zone* goes unknown, so "rejoin once at the end" is what + // the first frame after the shot already does — there is nothing to re-arm. It matters because + // a race intro flies the streaming focus hundreds of yards off the body, so `world.area()` + // changes under a player who has not moved, and the joins would otherwise fire against zones + // they are only *looking* at. + let cinematic_running = cinematic.as_deref().is_some_and(|c| c.is_playing()); + let areas = areas.as_deref(); + + // **The zone the walk may act on** — `None` whenever any gate says the leaf under the player + // is not the one they are standing in: no character session yet ([`ZoneChannelWalk::live`]), a + // cinematic flying the streaming focus off the body, a world still arriving ([`zone_is_settled`]), + // or an area authority with no answer for this session (decision 2130 made that a real state + // again rather than the previous character's zone). + let zone = (walk.live && !cinematic_running && zone_is_settled(player.as_deref())) + .then(|| { + let areas = areas?; + let row = world + .area() + .and_then(|leaf| areas.0.top_zone(leaf)) + .and_then(|zone| areas.0.get(zone))?; + Some((row.name.clone(), row.flags & AREA_FLAG_TRADE_CHANNEL != 0)) + }) + .flatten(); + + // **The VM's zone-channel catalog is a `ChatChannels.dbc` fact, and it is fed BEFORE any of + // those gates** — because the thing the verbs read out of it is the row **Shortcut**, which no + // zone ever changes. + // + // It used to be fed only once a zone was known, and an empty catalog is not "no zone yet" to + // any of its three readers — it is *"no such built-in channel"*. `AddChatWindowChannel(1, + // "General")` took the no-match leg and stored `("General", 0)` as a **custom** channel, which + // the chat cache then wrote out as a name in the window's `CHANNELS` block with its zone bit + // dropped from `ZONECHANNELS`; the director's own `Onewarrior` file carries exactly that + // damage, and a window that has lost a zone channel's id drops every line the stock + // `ChatFrame_OnEvent` matches by id (ref `ChatFrame.lua` l.1379). `JoinChannelByName("General")` + // was worse: `(0, nil)` is the custom-channel leg, so it **sent `CMSG_JOIN_CHANNEL("General")`** + // — a real custom channel of that name on the server. + // + // Fed zone-less, every zone-dependent row carries `resolved: None`, which is the reference's + // own "matched a row but there is no zone text yet" leg: `AddChatWindowChannel` stores nothing + // and answers nil, `JoinChannelByName` returns nil and sends nothing (chat-cache-grammar.md §5, + // decision 1908). Right answer instead of a wrong one, and the walk re-feeds the moment a zone + // lands. + let mut script = script; + if !channels.channels.is_empty() { + if let Some(script) = script.as_mut() { + let at = zone.clone().unwrap_or_default(); + let fed = catalog_fed.get(script); + if fed.as_ref() != Some(&at) { + script.set_zone_channel_catalog(zone_channel_catalog( + &channels.channels, + &at.0, + at.1, + areas.and_then(|a| city_word(&a.0)), + )); + *fed = Some(at); + } } } + + let (Some(at), Some(areas)) = (zone, areas) else { + return; // nothing to walk against yet + }; if walk.at.as_ref() == Some(&at) { return; // same zone as last frame — the common case, and free } let wanted = wanted_channels(&channels.channels, &at.0, at.1, city_word(&areas.0)); + // Every row that composes here — the set that takes SLOTS, city gate not applied + // ([`tracked_channels`], decision 2137). `wanted` is the subset we may actually join. + let tracked = tracked_channels(&channels.channels, &at.0, city_word(&areas.0)); // The diff is **per DBC row**, not per name — because a zone change does not add and remove // channels, it *renames* one: `General - Felwood` and `General - Winterspring` are the same @@ -404,25 +462,127 @@ pub(super) fn auto_join_zone_channels( // retail sniff shows the pairs adjacent on the wire in exactly that order: LEAVE // `General - Felwood` → JOIN `General - Winterspring` → LEAVE `LocalDefense - Felwood` → … // (wow-re `zone-chat-channel-autojoin.md` §5). - for name in &wanted { + // + // The renames are collected rather than applied inside the loop only because `row_of` borrows + // the catalog that lives beside the slots; the order on the wire is unchanged. + // + // **The walk is over `tracked`, not `wanted`** (decision 2137): every composable row gets a + // slot, and the city gate then decides join-versus-suspend. Registering first is what gives + // `Trade - City` number 2 on a character who is nowhere near a city. + let eligible = |name: &String| wanted.iter().any(|w| w == name); + let mut renamed: Vec<(String, String)> = Vec::new(); + let mut registered: Vec = Vec::new(); + let mut suspended: Vec = Vec::new(); + let mut bypassed: Vec = Vec::new(); + for name in &tracked { match walk.held.iter().find(|h| row_of(h) == row_of(name)) { - Some(old) if old == name => continue, // this row's name did not move - Some(old) => leave(old), - None => {} + // This row's name did not move. Normally nothing to do — **except** that a + // locally-suspended slot bypasses the name comparison entirely (`0x49a31c`, §7 pass 1 + // step 2), which is exactly how walking back into a city re-joins `Trade - City` under + // a name that never changed. + Some(old) if old == name => { + if eligible(name) { + if channels.slot_state(name) == Some(SlotState::Suspended) { + join(name); + // **And the slot leaves state 3 at SEND time, not when the notice lands.** + // The reference's rename runs on the bypass leg too, with the name + // unchanged, and writes `(old == 3) ? 1 : 2` — so a bypassed slot goes to + // **1**, not 2: a plain `YOU_JOINED` when it confirms, never + // `YOU_CHANGED`. Our `Joined` is that state 1 (see [`SlotState`]). + // + // Leaving it at 3 until the notice arrived would be wrong twice: a second + // zone change inside that window would take this arm again and send a + // duplicate join, and walking straight back out would find a slot that is + // not `Joined` and so skip the LEAVE for a channel we really had joined. + bypassed.push(name.clone()); + } + } else if channels.slot_state(name) == Some(SlotState::Joined) { + // Walking OUT of the city: the LEAVE goes out here, before the eligibility + // test suspends the slot — "the LEAVE still goes out, because it happens + // earlier in the same iteration" (§5). + leave(name); + suspended.push(name.clone()); + } + continue; + } + // The name moved. LEAVE(old) only from a server-confirmed slot (`0x49a35e`), then the + // rename, then the gate. + Some(old) => { + if channels.slot_state(old) == Some(SlotState::Joined) { + leave(old); + } + renamed.push((old.clone(), name.clone())); + } + // Never tracked: the slot is claimed BEFORE the gate. + None => registered.push(name.clone()), + } + if eligible(name) { + join(name); + } else { + suspended.push(name.clone()); } - join(name); } - // Rows that stopped applying entirely — walking out of a capital drops Trade with nothing to - // replace it. + // Rows that stopped COMPOSING entirely — a zone with no name at all, which drops the row from + // `tracked` rather than merely making it ineligible. The slot is kept and suspended, never + // freed, for the same reason as everything above. for old in walk .held .iter() - .filter(|h| !wanted.iter().any(|w| row_of(w) == row_of(h))) + .filter(|h| !tracked.iter().any(|t| row_of(t) == row_of(h))) { - leave(old); + if channels.slot_state(old) == Some(SlotState::Joined) { + leave(old); + } + suspended.push(old.clone()); } - walk.held = wanted; + // **The slot is renamed in place, at send time** — [`ChannelState::rename_slot`] carries the + // whole why. In one line: the server's `YOU_LEFT` for the old name must find no slot, or the + // stock `ChatFrame_OnEvent` deletes the window's registration for that channel and never + // rebuilds it. + for (old, new) in &renamed { + if let Some(n) = channels.rename_slot(old, new) { + debug!("chat: zone channel slot {n} renamed {old:?} → {new:?}"); + } + } + // **A row we have never tracked takes its slot here — before the gate decided anything** + // (`0x49b980` at `0x49a50d`, ahead of the city gate at `0x49a512`). In DBC row order, which is + // what makes the numbering `1 General`, `2 Trade`, `3 LocalDefense` whether or not Trade is + // joinable from where the player is standing. + // The state-3 bypass's own write, before the suspends below so the two can never fight over + // one slot in a single pass. + for name in &bypassed { + channels.confirm_slot(name); + } + for name in ®istered { + match channels.claim_slot(name) { + Some(n) => debug!("chat: zone channel {name:?} registered as slot {n}"), + None => warn!( + "chat: no free slot for zone channel {name:?} — all {} are taken", + super::edit::MAX_CHANNELS + ), + } + } + // **…and a row that stopped applying is SUSPENDED, not freed** (`0x49bcf0`, state 3). The slot + // and its number survive, so the `YOU_LEFT` that comes back carries the `SUSPENDED` token and + // the window keeps its registration — otherwise walking out of Stormwind kills Trade for the + // session exactly the way a border crossing used to kill General. + for old in &suspended { + if let Some(n) = channels.suspend_slot(old) { + debug!("chat: zone channel slot {n} ({old:?}) suspended — out of its city"); + } + } + if !(renamed.is_empty() && registered.is_empty()) { + if let Some(script) = script.as_mut() { + // `GetChannelName(n)` answers the new name from this frame on, as it does in the + // reference — the slot moved, not the numbering. + script.set_joined_channels(channels.names()); + } + } + + // `held` tracks every COMPOSED row, not only the joined ones — that is what makes the + // "name unchanged, eligibility changed" arms above reachable at all. + walk.held = tracked; walk.at = Some(at); } @@ -533,7 +693,11 @@ mod tests { at: Some(("Tanaris".into(), false)), ..Default::default() }; - channels.joined = walk.held.iter().cloned().map(Some).collect(); + channels.joined = walk + .held + .iter() + .map(|n| Some(crate::ui_chat::edit::ChannelSlot::joined(n))) + .collect(); // No VM in a unit test; the mirror leg is the one line this cannot reach, and // `end_session_channels` is a two-line wrapper over exactly this call. @@ -576,6 +740,141 @@ mod tests { assert_eq!(ids(&felwood), vec![1, 22]); } + /// **The slot number never moves across the whole city round trip** (decision 2137). + /// + /// The exact state sequence the walk drives for `Trade - City`: registered and joined on the + /// way in, suspended on the way out (record kept, `0x49bcf0`), and put back to *joined at send + /// time* on the way back in — the state-3 bypass, where the reference's rename writes + /// `(old == 3) ? 1 : 2` and so lands on **1**, not 2. Our `Joined` is that state 1, which is + /// why the re-join confirms as a plain `YOU_JOINED` rather than `Changed Channel`. + /// + /// Leaving the slot at `Suspended` until the notice arrived would break two things: a second + /// zone change inside that window would send a duplicate join, and walking straight back out + /// would skip the LEAVE for a channel we had really joined. + /// + /// **What this does NOT cover:** that the walk actually calls `confirm_slot` on the bypass leg. + /// The state machine is asserted here; the wiring is not, because the walk is a Bevy system + /// with no harness yet. That harness is the obvious next instrument for this module. + #[test] + fn a_city_round_trip_keeps_the_slot_and_its_number() { + let mut c = ChannelState::default(); + assert_eq!(c.claim_slot("General - Elwynn Forest"), Some(1)); + assert_eq!(c.claim_slot("Trade - City"), Some(2)); + assert_eq!(c.claim_slot("LocalDefense - Elwynn Forest"), Some(3)); + + // Out of the city: the record and the number survive. + assert_eq!(c.suspend_slot("Trade - City"), Some(2)); + assert_eq!(c.slot_state("Trade - City"), Some(SlotState::Suspended)); + assert_eq!(c.number_of("Trade - City"), Some(2)); + assert_eq!( + c.number_of("LocalDefense - Elwynn Forest"), + Some(3), + "and nothing above it renumbered — the slot went quiet, it did not go away" + ); + + // Back in: the bypass sends the join AND clears the suspension in the same pass. + c.confirm_slot("Trade - City"); + assert_eq!(c.slot_state("Trade - City"), Some(SlotState::Joined)); + assert_eq!(c.number_of("Trade - City"), Some(2), "still slot 2"); + } + + /// **A city-only row still takes a slot when you are nowhere near a city** (decision 2137). + /// + /// The reference registers the slot (`0x49b980` at `0x49a50d`) *before* the city gate + /// (`0x49a512`), so a character standing in Elwynn Forest holds three slots — General, a + /// created-but-never-joined `Trade - City`, and LocalDefense — and the `N.` a channel line + /// carries is `slot[+0x00]`, the client-local index. `/2` therefore reaches **Trade** there. + /// + /// We filtered the city rows out before they could take a number, so ours gave LocalDefense 2 + /// and `/2` reached a different channel than the reference's. Invisible inside a capital, where + /// all three join and the numbering agrees — which is why the director's screenshot, taken on a + /// login into Stormwind, does not show it. + #[test] + fn a_city_row_takes_its_number_even_outside_a_city() { + let cat = catalog(); + assert_eq!( + tracked_channels(&cat, "Elwynn Forest", CITY), + vec![ + "General - Elwynn Forest", + "Trade - City", + "LocalDefense - Elwynn Forest", + ], + "all three rows take slots in DBC order, city or no city" + ); + assert_eq!( + wanted_channels(&cat, "Elwynn Forest", false, CITY), + vec!["General - Elwynn Forest", "LocalDefense - Elwynn Forest"], + "…but only two of them are JOINED out here — the gate decides the packet, not the slot" + ); + + // Inside a capital the two lists agree, which is the case that hid this. + assert_eq!( + tracked_channels(&cat, "Stormwind City", CITY), + wanted_channels(&cat, "Stormwind City", true, CITY) + ); + } + + /// The two lists never disagree about *resolvability* — only about the city gate. A missing + /// city word or an unknown zone drops the row from both, so a slot is never taken for a name + /// we could not compose. + #[test] + fn the_slot_set_and_the_join_set_differ_only_by_the_city_gate() { + let cat = catalog(); + assert_eq!( + tracked_channels(&cat, "Stormwind City", None), + wanted_channels(&cat, "Stormwind City", true, None), + "no city word: the city-NAMED rows are unresolvable, so neither list carries them" + ); + assert!( + tracked_channels(&cat, "", CITY).is_empty(), + "no zone: nothing zone-dependent composes, so nothing takes a slot either" + ); + } + + /// **A catalog fed with no zone is not the same thing as no catalog** (decision 2130). + /// + /// The VM's three readers all treat an *empty* catalog as "no such built-in channel": + /// `AddChatWindowChannel` stores the name as a custom channel with id 0, and + /// `JoinChannelByName` returns `(0, nil)` — the custom leg — and **sends + /// `CMSG_JOIN_CHANNEL("General")`**. Fed zone-less instead, every zone-dependent row is present + /// but carries `resolved: None`, which is the reference's own "matched a row, no zone text yet" + /// leg: store nothing, answer nil, send nothing (chat-cache-grammar.md §5). + /// + /// That gap is what wrote `CHANNELS / General / LocalDefense` into the director's `Onewarrior` + /// window block with the zone bits stripped, and it was open on every login until the world + /// settled — because the catalog used to be fed only once a zone was known. + #[test] + fn a_zoneless_catalog_still_carries_every_row() { + let rows = zone_channel_catalog(&catalog(), "", false, None); + assert_eq!(rows.len(), 6, "every DBC row is present, zone or no zone"); + + let by = |s: &str| rows.iter().find(|r| r.shortcut == s).unwrap().clone(); + assert_eq!(by("General").id, 1, "…and each carries its ChannelID"); + assert_eq!( + by("General").resolved, + None, + "a zone-dependent row with no zone is UNRESOLVED — the nil leg, not the custom leg" + ); + assert_eq!(by("Trade").resolved, None); + assert_eq!( + by("WorldDefense").resolved, + Some("WorldDefense".to_string()), + "a row whose name carries no %s resolves with no zone at all" + ); + + // And the moment a zone lands, the same rows resolve. + let rows = zone_channel_catalog(&catalog(), "Elwynn Forest", false, CITY); + let by = |s: &str| rows.iter().find(|r| r.shortcut == s).unwrap().clone(); + assert_eq!( + by("General").resolved, + Some("General - Elwynn Forest".to_string()) + ); + assert!( + !by("Trade").listed, + "…and Trade is city-only, so it is not listed out here" + ); + } + /// arg7: the composed names resolve back to their `ChatChannels.dbc` id, custom ones to 0 — /// matched by the server's own substring rule. #[test] diff --git a/crates/benilla-app/src/ui_chat/edit.rs b/crates/benilla-app/src/ui_chat/edit.rs index 0897bf0a..1bbb34ec 100644 --- a/crates/benilla-app/src/ui_chat/edit.rs +++ b/crates/benilla-app/src/ui_chat/edit.rs @@ -123,11 +123,18 @@ pub(crate) const MAX_CHANNELS: usize = 10; /// A `Vec` cannot express that: `retain` closed the hole and renumbered everything above /// it, so walking out of a zone renamed *other* channels — the director saw General and /// LocalDefense trade numbers on one zone change, and a `/2` typed after that went somewhere else. +/// +/// **And each slot carries a state, because the reference's does** (`+0x9c`, decision 2130). We +/// model the one value of it that changes what the player sees: **3, locally suspended**. States +/// 0 (server-confirmed), 1 (join not yet acknowledged) and 2 (renamed, re-join pending) collapse +/// here, and that is sound rather than lazy — the reference reads 0 to decide whether a LEAVE goes +/// out, and our walk decides that from [`super::channels::ZoneChannelWalk::held`], the request side +/// (1284). State 3 does not collapse: it is the difference between keeping a channel and losing it. #[derive(Resource, Default)] pub(crate) struct ChannelState { /// Slot `i` is channel number `i + 1`; `None` is a freed slot, kept so the numbers above it /// do not move. Never longer than [`MAX_CHANNELS`]. - pub joined: Vec>, + pub joined: Vec>, /// `ChatChannels.dbc`, loaded once at Startup ([`super::channels::load_chat_channels`]). /// /// It lives here because both of its consumers are this type's own business: composing the @@ -164,6 +171,58 @@ pub(crate) struct ChannelState { pub zone_mask: u32, } +/// One joined-channel record — the reference's `[0xb4fe04] + n*0xa0` slot, in the fields this +/// client uses. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct ChannelSlot { + /// `+0x04` — the channel's current name. The zone walk RENAMES this in place + /// ([`ChannelState::rename_slot`]). + pub name: String, + /// `+0x9c` — the slot's state, in the three values that change what the player sees. + pub state: SlotState, +} + +/// The reference's per-slot state (`slot+0x9c`), modelled in the values whose **notice token** +/// differs — because the token is what the stock `ChatFrame_OnEvent` branches on, and one of those +/// branches deletes the window's channel registration. +/// +/// The complete writer census is wow-re `zone-chat-channel-autojoin.md` §6. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) enum SlotState { + /// `0` (server-confirmed) and `1` (join not yet acknowledged), which we cannot tell apart and + /// do not need to: the reference reads `0` to decide whether a LEAVE goes out, and our walk + /// decides that from [`super::channels::ZoneChannelWalk::held`], the request side (1284). + #[default] + Joined, + /// `2` — **renamed, re-join pending**: the zone walk moved this row's name because the player + /// crossed a border (`0x49bcd3`, inside the rename `0x49bc50`). The confirming `YOU_JOINED` + /// then carries the `YOU_CHANGED` token instead, which is `CHAT_YOU_CHANGED_NOTICE` — + /// *"Changed Channel: [%s]"*, a line the reference has and we never printed. + Renamed, + /// `3` — **locally suspended**: the row lost its eligibility, which in the 1.12 data means + /// exactly one thing, walking out of a capital with `Trade` joined (§5/§6). `0x49bcf0` sets it + /// and sends nothing; the LEAVE has already gone out earlier in the same iteration. + /// + /// The record and its number survive, and the arriving `YOU_LEFT` carries the `SUSPENDED` + /// token instead (`0x49c0e0`) — same rendered text, different arg1, which is exactly what stops + /// `ChatFrame_OnEvent` from deleting the registration. Walking back into a city then re-joins + /// through the state-3 bypass (§7 pass 1 step 2), where the name has not changed and the + /// comparison would otherwise say there was nothing to do. + Suspended, +} + +impl ChannelSlot { + /// A server-confirmed slot — the only way one is ever born here, because this client registers + /// a slot on `YOU_JOINED` rather than at send time (the reference's `0x49b980` does the latter, + /// at state 1; the difference is invisible to everything we model). + pub(crate) fn joined(name: &str) -> Self { + ChannelSlot { + name: name.to_string(), + state: SlotState::Joined, + } + } +} + /// The bit `id` occupies in a `ZONECHANNELS` word — `1 << (ChannelID - 1)`; nothing outside /// `1..=32` has one. pub(crate) fn zone_bit(id: u32) -> u32 { @@ -189,11 +248,107 @@ impl ChannelState { self.zone_mask &= !zone_bit(self.channels.zone_channel_id(name)); } + /// **Rename a slot in place — the zone walk crossing a border** (decision 2130). + /// + /// The reference's `0x49bc50(oldName, newName)`, called from `ZoneChannelRefresh`'s pass 1 + /// step 6 (wow-re `zone-chat-channel-autojoin.md` §7, VERIFIED): it copies the new name into + /// the slot and moves its state to "re-join pending" — **at send time**, before the server has + /// answered the `CMSG_LEAVE_CHANNEL` that went out a moment earlier. + /// + /// That ordering is the whole point, and it is not bookkeeping. When the server's `YOU_LEFT` + /// for the *old* name arrives, no slot carries that name any more — `0x49be90` is a pure name + /// scan with no state filter — so the marshaller (`0x49b0b0`) takes its NULL-slot leg + /// (`0x49b12f`) and defaults `arg7`, `arg8`, `arg9` and `arg10` to `0 / 0 / "" / 0` **together**, + /// and the stock `ChatFrame_OnEvent` finds no match and returns (`found == 0`). Which means it + /// never reaches its own `YOU_LEFT` arm, and that arm is the one that **deletes the window's + /// channel registration** (`ChatFrame.lua` l.1382-1384): + /// + /// ```lua + /// this.channelList[index] = nil; + /// this.zoneChannelList[index] = nil; + /// ``` + /// + /// Nothing in stock FrameXML ever re-adds one on a join, and **nothing in the engine does + /// either**: a closed census of event 395 (`0x18b`) leaves five fire sites in the image, none of + /// them reachable from a zone change, for guilded and unguilded players alike (wow-re §11.1, + /// VERIFIED). The rename is not one repopulation mechanism among several — it is the only thing + /// standing between a border crossing and a dead channel. + /// + /// So without it, one crossing costs the window General and LocalDefense **for the rest of the + /// session** — the replacement join notice dropped unprinted, and every line spoken in the new + /// zone with it. `ui_chat::tests::a_zone_change_must_not_deregister_the_channel_it_renames` is + /// that claim. + /// + /// Freeing and re-claiming instead would also renumber: the slot is the channel's `/N`. + /// + /// Returns the slot number when there was one to rename. + pub(crate) fn rename_slot(&mut self, old: &str, new: &str) -> Option { + let n = self.number_of(old)?; + let slot = self.joined[n as usize - 1].as_mut()?; + slot.name = new.to_string(); + // `0x49bcd3`: `(old == 3) ? 1 : 2` — either way the slot is awaiting a re-join, and the + // confirming notice is the one that resolves it. + slot.state = SlotState::Renamed; + Some(n) + } + + /// **Suspend the slot holding `name` — the row stopped applying** (`0x49bcf0`, state 3). + /// + /// Walking out of a capital with `Trade` joined is the only case the 1.12 data produces. The + /// LEAVE has already gone out; this is what keeps the record, so the notice that comes back is + /// `SUSPENDED` rather than `YOU_LEFT` and the window keeps its registration + /// ([`ChannelSlot::suspended`]). Freeing it instead is the same bug the rename fixes, on the + /// one row a rename cannot reach. + pub(crate) fn suspend_slot(&mut self, name: &str) -> Option { + let n = self.number_of(name)?; + self.joined[n as usize - 1].as_mut()?.state = SlotState::Suspended; + Some(n) + } + + /// **The confirming `YOU_JOINED` resolves the slot's state** — the reference's `0x49bb20` + /// writes `+0x9c = 0` unconditionally on that arm. Without this a renamed slot would stay in + /// [`SlotState::Renamed`] for good and every later join notice would read "Changed Channel". + /// + /// Separate from [`Self::claim_slot`] because the notice arrives for slots we already number — + /// a rename is exactly that case, and it is the one that must not take a second slot. + pub(crate) fn confirm_slot(&mut self, name: &str) { + if let Some(n) = self.number_of(name) { + if let Some(slot) = self.joined[n as usize - 1].as_mut() { + slot.state = SlotState::Joined; + } + } + } + + /// The state of the slot holding `name` — the notice arms' own split. A name we hold no slot + /// for answers `None`, which is the leg where the reference defaults every derived arg. + pub(crate) fn slot_state(&self, name: &str) -> Option { + self.number_of(name) + .and_then(|n| self.joined[n as usize - 1].as_ref()) + .map(|s| s.state) + } + + /// The roster as the VM's mirror wants it — `GetChannelName`/`GetChannelList` read names, and + /// the holes have to survive the trip or `/N` addresses the wrong channel. + pub(crate) fn names(&self) -> Vec> { + self.joined + .iter() + .map(|s| s.as_ref().map(|s| s.name.clone())) + .collect() + } + + /// Every joined channel's name, holes skipped. + pub(crate) fn iter_names(&self) -> impl Iterator { + self.joined.iter().flatten().map(|s| s.name.as_str()) + } + /// The 1-based number of `name` (case-insensitive), if joined. pub(crate) fn number_of(&self, name: &str) -> Option { self.joined .iter() - .position(|c| c.as_deref().is_some_and(|c| c.eq_ignore_ascii_case(name))) + .position(|c| { + c.as_ref() + .is_some_and(|c| c.name.eq_ignore_ascii_case(name)) + }) .map(|i| i as u32 + 1) } @@ -206,16 +361,23 @@ impl ChannelState { /// error-string table this build indexes by id, and the structural half is what matters. pub(crate) fn claim_slot(&mut self, name: &str) -> Option { if let Some(n) = self.number_of(name) { + // A confirmed join on a slot we already hold clears its suspension — the reference's + // `0x49bb20` writes state 0 unconditionally. This is the other half of the state-3 + // bypass: walking back into a capital re-joins `Trade - City` under the name the slot + // already carries. + if let Some(slot) = self.joined[n as usize - 1].as_mut() { + slot.state = SlotState::Joined; + } return Some(n); } if let Some(i) = self.joined.iter().position(Option::is_none) { - self.joined[i] = Some(name.to_string()); + self.joined[i] = Some(ChannelSlot::joined(name)); return Some(i as u32 + 1); } if self.joined.len() >= MAX_CHANNELS { return None; } - self.joined.push(Some(name.to_string())); + self.joined.push(Some(ChannelSlot::joined(name))); Some(self.joined.len() as u32) } diff --git a/crates/benilla-app/src/ui_chat/event.rs b/crates/benilla-app/src/ui_chat/event.rs index 14737ea4..4fc36bc0 100644 --- a/crates/benilla-app/src/ui_chat/event.rs +++ b/crates/benilla-app/src/ui_chat/event.rs @@ -406,6 +406,11 @@ pub(crate) struct ChatEvent { pub channel_number: u32, pub channel_base: String, pub notice: String, + /// The state of our own slot for this channel **as the notice arrived**, or `None` when we + /// hold no slot for it. Set once at the top of [`super::feed::deliver`], before that function + /// changes any of it — because the reference's notice arms read `slot+0x9c` to pick the token + /// and only then move it (`0x49c0c2` reads, `0x49bb20` writes). + pub slot_state: Option, } impl ChatEvent { @@ -433,7 +438,9 @@ impl ChatEvent { // one slot whose meaning is type-dependent (`ChatFrame_OnEvent` l.1416/1424 vs l.1396). let arg1 = match (self.kind, self.notice_byte()) { (Some(ChatEventKind::ChannelNotice | ChatEventKind::ChannelNoticeUser), Some(byte)) => { - notice_token(byte).unwrap_or_default().to_string() + notice_token(byte, self.slot_state) + .unwrap_or_default() + .to_string() } _ => self.text.clone(), }; @@ -577,15 +584,29 @@ pub(crate) fn event_name(kind: ChatEventKind) -> &'static str { /// [`super::feed::ChatLog::push_channel_notice`] drops it before it becomes an event), and anything /// past `0x1F` is outside vmangos's range. /// -/// **Two state-dependent tokens we do not model:** the client answers `"YOU_CHANGED"` for `0x02` -/// and `"SUSPENDED"` for `0x03` when its own channel record is in the matching state -/// (`rec+0x9c == 2` / `== 3`) — a per-channel state benilla keeps nothing equivalent to, so we -/// always send the plain `YOU_JOINED` / `YOU_LEFT`. Both alternates are `CHAT__NOTICE` strings -/// that exist in GlobalStrings (`CHAT_YOU_CHANGED_NOTICE`, `CHAT_SUSPENDED_NOTICE`). -pub(crate) fn notice_token(byte: u8) -> Option<&'static str> { +/// **Two of the arms are state-dependent** (decision 2130): the client answers `"YOU_CHANGED"` for +/// `0x02` and `"SUSPENDED"` for `0x03` when its own channel record is in the matching state +/// (`rec+0x9c == 2` at `0x49c087` / `== 3` at `0x49c0e0`; wow-re +/// `zone-chat-channel-autojoin.md` §11.3, VERIFIED), and both alternates are real +/// `CHAT__NOTICE` strings — +/// *"Changed Channel: [%s]"* and *"Left Channel: [%s]"*. We modelled neither until the zone walk's +/// registration loss made it matter: `arg1` is what the stock `ChatFrame_OnEvent` branches on, and +/// the `YOU_LEFT` branch **deletes the window's channel registration** (`ChatFrame.lua` +/// l.1382-1384). A suspended channel that answers the plain token loses its registration for good. +pub(crate) fn notice_token( + byte: u8, + state: Option, +) -> Option<&'static str> { + use super::edit::SlotState; use benilla_protocol::messages::channel_notice as n; Some(match byte { + // The confirming notice for a slot the zone walk renamed — crossing a zone border prints + // "Changed Channel: [1. General - Westfall]", not a leave and a join. + n::YOU_JOINED if state == Some(SlotState::Renamed) => "YOU_CHANGED", n::YOU_JOINED => "YOU_JOINED", + // Walking out of a capital: the record and its number stay, so this must NOT be the token + // that tears the registration down. + n::YOU_LEFT if state == Some(SlotState::Suspended) => "SUSPENDED", n::YOU_LEFT => "YOU_LEFT", n::WRONG_PASSWORD => "WRONG_PASSWORD", n::NOT_MEMBER => "NOT_MEMBER", diff --git a/crates/benilla-app/src/ui_chat/feed.rs b/crates/benilla-app/src/ui_chat/feed.rs index 4308e3e6..6b362dab 100644 --- a/crates/benilla-app/src/ui_chat/feed.rs +++ b/crates/benilla-app/src/ui_chat/feed.rs @@ -683,12 +683,21 @@ pub(super) fn deliver( let notice = event .notice_byte() .filter(|_| event.kind == Some(ChatEventKind::ChannelNotice)); + // **First, before anything below moves it.** The reference's notice arms read `slot+0x9c` to + // choose the token and only then write it (`0x49c0c2` reads, `0x49bb20` writes), and the two + // alternates are what keep a renamed or suspended channel registered with the window + // ([`super::event::notice_token`], decision 2130). + event.slot_state = channels.slot_state(&event.channel); if notice == Some(channel_notice::YOU_JOINED) { // The reference's `0x49bbaf`: the confirmed join is what sets the channel's // `ZONECHANNELS` bit, and it is the ONLY thing that grows that mask at runtime // (decision 2120). Outside the slot claim below because it is not about slots — a // re-confirmation of a channel we already number still owns the bit. channels.note_zone_channel_joined(&event.channel); + // …and the slot the walk RENAMED is already numbered, so the claim below skips it — but + // its state still has to come back to `Joined`, or the next notice on that row reads as + // another rename. A no-op for a channel we hold no slot for, which is the claim's case. + channels.confirm_slot(&event.channel); } if notice == Some(channel_notice::YOU_JOINED) && channels.number_of(&event.channel).is_none() { match channels.claim_slot(&event.channel) { @@ -697,7 +706,7 @@ pub(super) fn deliver( "chat: server confirms channel {:?} joined (slot {slot})", event.channel ); - script.set_joined_channels(channels.joined.clone()); + script.set_joined_channels(channels.names()); } // The reference's own ceiling, reached: ten slots, all taken. It answers with a chat // error and no record, so the channel stays unnumbered here too. @@ -715,11 +724,19 @@ pub(super) fn deliver( channels.stamp_channel(event); route(script, windows, event); if let Some(name) = leaving { - // Cleared in place, never compacted: slot 2 going empty must not make slot 3 into 2 - // ([`ChannelState`], 1286). - let freed = channels.free_slot(&name); - debug!("chat: server confirms channel {name:?} left (slot {freed:?} now free)"); - script.set_joined_channels(channels.joined.clone()); + // **A suspended slot survives its own leave** (`0x49c0e0`, decision 2130). The reference's + // `0x03` arm jumps past the teardown (`0x49c115`) when the record is in state 3, so walking + // out of a capital keeps `Trade`'s record AND its number — which is what lets walking back + // in re-join through the state-3 bypass, and what stops the stock handler deregistering it. + if event.slot_state == Some(super::edit::SlotState::Suspended) { + debug!("chat: channel {name:?} suspended — the record and its number stay"); + } else { + // Cleared in place, never compacted: slot 2 going empty must not make slot 3 into 2 + // ([`ChannelState`], 1286). + let freed = channels.free_slot(&name); + debug!("chat: server confirms channel {name:?} left (slot {freed:?} now free)"); + script.set_joined_channels(channels.names()); + } } } diff --git a/crates/benilla-app/src/ui_chat/frames.rs b/crates/benilla-app/src/ui_chat/frames.rs index ee7bcbb1..861310c4 100644 --- a/crates/benilla-app/src/ui_chat/frames.rs +++ b/crates/benilla-app/src/ui_chat/frames.rs @@ -251,7 +251,7 @@ pub(crate) fn compose_notice( kind: ChatEventKind, get: &dyn Fn(&str) -> Option, ) -> Option { - let token = notice_token(event.notice_byte()?)?; + let token = notice_token(event.notice_byte()?, event.slot_state)?; let template = get(&format!("CHAT_{token}_NOTICE"))?; let mut args = vec![Arg::S(event.channel.as_str())]; if kind == ChatEventKind::ChannelNoticeUser { diff --git a/crates/benilla-app/src/ui_chat/input/mod.rs b/crates/benilla-app/src/ui_chat/input/mod.rs index dc285b81..a17923d5 100644 --- a/crates/benilla-app/src/ui_chat/input/mod.rs +++ b/crates/benilla-app/src/ui_chat/input/mod.rs @@ -989,7 +989,9 @@ pub(super) fn drain_chat_input( .joined .iter() .enumerate() - .filter_map(|(i, c)| c.as_ref().map(|c| format!("{}. {c}", i + 1))) + .filter_map(|(i, c)| { + c.as_ref().map(|c| format!("{}. {}", i + 1, c.name)) + }) .collect(); let text = if roster.is_empty() { "You are not in any channels.".to_string() diff --git a/crates/benilla-app/src/ui_chat/settings.rs b/crates/benilla-app/src/ui_chat/settings.rs index 8d9d04ee..d073ad64 100644 --- a/crates/benilla-app/src/ui_chat/settings.rs +++ b/crates/benilla-app/src/ui_chat/settings.rs @@ -336,8 +336,24 @@ fn parse(text: &str, rows: &[(u32, String)]) -> Parsed { if is_end { block = Block::Top; } else if let Some((_, look)) = current.as_mut() { - // The loader takes the first word, id 0. - look.channels.push((head.to_string(), 0)); + // The loader takes the first word, id 0 — **unless the word is a + // `ChatChannels.dbc` Shortcut**, in which case it is a zone channel that lost + // its id and this restores it (decision 2130). + // + // Not a heuristic: the reference's engine resolves every name through the + // shortcut walk before it stores one (`AddChatWindowChannel 0x4a1000`, + // chat-cache-grammar.md §5), so a *custom* entry can never be spelled exactly + // like a built-in shortcut — all 34 stock files carry an empty per-window + // `CHANNELS` block beside a non-zero `ZONECHANNELS`. Ours could, because the + // VM's catalog was still empty when the window was written, and `General` came + // back as a custom channel with the id the id-match at `ChatFrame.lua:1379` + // needs. The write side is fixed; this is what heals the files it already + // damaged, and it is idempotent — a healed file has no such name to match. + let id = rows + .iter() + .find(|(_, shortcut)| shortcut.eq_ignore_ascii_case(head)) + .map_or(0, |(id, _)| *id); + look.channels.push((head.to_string(), id)); } continue; } @@ -482,10 +498,8 @@ fn shortcut_rows(channels: &super::edit::ChannelState) -> Vec<(u32, String)> { /// of. fn roster(channels: &super::edit::ChannelState) -> Vec<(String, u32)> { channels - .joined - .iter() - .flatten() - .map(|name| (name.clone(), channels.channels.zone_channel_id(name))) + .iter_names() + .map(|name| (name.to_string(), channels.channels.zone_channel_id(name))) .collect() } @@ -770,10 +784,14 @@ mod tests { } } + /// `ChatChannels.dbc` as the loader hands it to [`parse`] — a subset of the shipped six, but + /// carrying all three `INITIAL` rows, because the ids 1 / 2 / 22 are what the window blocks are + /// actually about. fn rows() -> Vec<(u32, String)> { vec![ (1, "General".to_string()), (2, "Trade".to_string()), + (22, "LocalDefense".to_string()), (24, "LookingForGroup".to_string()), ] } @@ -837,6 +855,43 @@ mod tests { assert_eq!(parsed.joined, vec!["MyChan".to_string()]); } + /// **A window `CHANNELS` name that IS a DBC shortcut is a zone channel, not a custom one** + /// (decision 2130) — the repair for the files our own writer damaged. + /// + /// The director's `Onewarrior` file carried this verbatim: `CHANNELS / General / LocalDefense + /// / END` beside `ZONECHANNELS 2`, so window 1 held General and LocalDefense with id **0** and + /// only Trade kept its bit. The stock `ChatFrame_OnEvent` matches a channel line by + /// `zoneChannelList[index] == arg7` (`ChatFrame.lua:1379`), so that character silently lost + /// every General and LocalDefense line — join notices and speech alike — while Trade still + /// worked. + /// + /// The write side no longer produces it (the VM's catalog is fed before any verb can ask), and + /// this heals what it already wrote. Idempotent by construction: a healed file writes the name + /// as a bit, so there is no name left here to match. + #[test] + fn a_window_channel_named_like_a_dbc_shortcut_regains_its_id() { + let text = "WINDOW 1\nSIZE 0\n\nMESSAGES\nEND\n\nCHANNELS\nGeneral\nMyChan\n LocalDefense\nEND\n\nZONECHANNELS 2\n\nEND\n"; + let parsed = parse(text, &rows()); + assert_eq!( + parsed.looks[0].1.channels, + vec![ + ("General".to_string(), 1), + ("MyChan".to_string(), 0), + ("LocalDefense".to_string(), 22), + ("Trade".to_string(), 2), + ], + "the two shortcuts come back as their DBC rows; the genuinely custom name keeps id 0, \ + and the ZONECHANNELS bit still contributes Trade" + ); + + // …and the next save writes them as bits again rather than as names, which is what makes + // the repair one-way. + let out = render(&[parsed.looks[0].1.clone()], &[], &[], 0x0020_0003, false); + let window = out.split("WINDOW 1").nth(1).unwrap(); + assert!(window.contains("CHANNELS\nMyChan\nEND"), "{out}"); + assert!(window.contains("ZONECHANNELS 2097155\n"), "{out}"); + } + /// A window's zone bits are masked by the joined set — a zone channel the client has left /// is not written for the window either, the way the writer ANDs the two words. #[test] diff --git a/crates/benilla-app/src/ui_chat/tests.rs b/crates/benilla-app/src/ui_chat/tests.rs index 9c5e470c..0b5e6b46 100644 --- a/crates/benilla-app/src/ui_chat/tests.rs +++ b/crates/benilla-app/src/ui_chat/tests.rs @@ -778,6 +778,283 @@ fn a_channel_notice_renders_in_the_channels_color_not_the_notice_row() { ); } +/// **Crossing a zone border must not deregister the channel it renames** (decision 2130). +/// +/// The walk sends `LEAVE(General - Elwynn Forest)` then `JOIN(General - Westfall)` — one DBC row, +/// renamed, and the retail sniff in wow-re `zone-chat-channel-autojoin.md` §7 shows exactly that +/// pair on the wire. The server answers each with a notice, and the stock `ChatFrame_OnEvent`'s +/// `YOU_LEFT` arm **deletes the window's registration for whatever it matched** +/// (`ChatFrame.lua` l.1382-1384): +/// +/// ```lua +/// this.channelList[index] = nil; +/// this.zoneChannelList[index] = nil; +/// ``` +/// +/// Nothing in stock FrameXML ever re-adds one on `YOU_JOINED` — `ChatFrame_AddChannel` is reachable +/// only from the `/join` popup and the chat-tab dropdown. So if our `YOU_LEFT` still resolves to a +/// slot, the window loses General for the rest of the session: the replacement join notice is +/// dropped unprinted, and so is every General line spoken in the new zone. That is the director's +/// *"sometimes I get no channel stuff"*, and this test is the observable. +/// +/// The window is registered here the way the chat cache registers it at login — the row's +/// **Shortcut** against its **ChannelID**, which is the id-match at `ChatFrame.lua:1379` — because +/// that is the registration the reference's own `chat-cache.txt` produces (`ZONECHANNELS` bits, not +/// names). +#[test] +fn a_zone_change_must_not_deregister_the_channel_it_renames() { + let _data = benilla_formats::wow_data_or_skip!(); + let mut s = chat_vm(); + let mut windows = super::frames::ChatWindows::default(); + let mut channels = super::edit::ChannelState { + channels: benilla_formats::ChatChannelsCatalog::from_rows(vec![ + benilla_formats::ChatChannelRow { + id: 1, + flags: 0x0_0003, + pattern: "General - %s".into(), + shortcut: "General".into(), + }, + ]), + ..Default::default() + }; + channels.claim_slot("General - Elwynn Forest"); + + // `ChatFrame_RegisterForChannels(GetChatWindowChannels(1))`, by hand: it does exactly this pair + // of writes, and calling it needs the `this` the event dispatch supplies. + s.run("ChatFrame1.channelList[1] = 'General' ChatFrame1.zoneChannelList[1] = 1") + .unwrap(); + + let channel_line = |name: &str| { + let mut e = ChatEvent::text_only(K::Channel, "anybody out here".into()); + e.sender = "Bob".into(); + e.channel = name.into(); + e + }; + let notice = |name: &str, byte: &str| { + let mut e = ChatEvent::text_only(K::ChannelNotice, String::new()); + e.channel = name.into(); + e.notice = byte.into(); + e + }; + + // The control: registered by id, a General line reaches the window. + let before = lines_in_window(&s); + super::feed::deliver( + &mut s, + &mut windows, + &mut channels, + &mut channel_line("General - Elwynn Forest"), + ); + assert_eq!( + lines_in_window(&s), + before + 1, + "the control must print — otherwise the assertion below proves nothing" + ); + + // The border crossing, in the order the walk actually produces it: `CMSG_LEAVE_CHANNEL(old)` + // goes out, **the slot is renamed in place before the answer can arrive** + // ([`super::edit::ChannelState::rename_slot`], the reference's `0x49bc50` at pass 1 step 6), + // `CMSG_JOIN_CHANNEL(new)` goes out, and only then do the two notices land. + let renamed = channels.rename_slot("General - Elwynn Forest", "General - Westfall"); + assert_eq!( + renamed, + Some(1), + "renamed in place — the slot number does not move" + ); + + let before = lines_in_window(&s); + super::feed::deliver( + &mut s, + &mut windows, + &mut channels, + &mut notice("General - Elwynn Forest", "3"), // YOU_LEFT + ); + assert_eq!( + lines_in_window(&s), + before, + "the leave prints NOTHING: no slot carries the old name any more, so arg7/arg8/arg9 come \ + out defaulted and the stock handler returns at `found == 0` — which is also why it never \ + reaches the arm that would deregister the channel" + ); + + super::feed::deliver( + &mut s, + &mut windows, + &mut channels, + &mut notice("General - Westfall", "2"), // YOU_JOINED + ); + + assert_eq!( + s.eval::>("return ChatFrame1.zoneChannelList[1]") + .unwrap(), + Some(1), + "the window must still be registered for ChannelID 1 after the rename — a nil here is \ + General going silent for the rest of the session" + ); + + // …and the observable that actually matters: speech from the NEW zone still lands. + let before = lines_in_window(&s); + super::feed::deliver( + &mut s, + &mut windows, + &mut channels, + &mut channel_line("General - Westfall"), + ); + assert_eq!( + lines_in_window(&s), + before + 1, + "a General line in the new zone must reach the window" + ); +} + +/// **Walking out of a capital SUSPENDS Trade — it does not free it** (decision 2130). +/// +/// The zone walk's other leave: a row that stops applying entirely, which in the 1.12 data means +/// exactly `Trade` when you step out of a city (wow-re `zone-chat-channel-autojoin.md` §5). The +/// `CMSG_LEAVE_CHANNEL` still goes out, but the client marks its own slot state 3 (`0x49bcf0`) and +/// keeps the record — so the notice comes back as the `SUSPENDED` token, the stock handler's +/// `YOU_LEFT` arm never runs, and the window keeps its registration. Walking back in re-joins +/// through the state-3 bypass onto the same slot, with the same number. +/// +/// Freeing it — which is what we did — cost Trade its registration on the way out and left the +/// re-join notice unprintable on the way back in. Same bug as the border crossing, one row over. +#[test] +fn leaving_a_capital_suspends_trade_rather_than_deregistering_it() { + let _data = benilla_formats::wow_data_or_skip!(); + let mut s = chat_vm(); + let mut windows = super::frames::ChatWindows::default(); + let mut channels = super::edit::ChannelState { + channels: benilla_formats::ChatChannelsCatalog::from_rows(vec![ + benilla_formats::ChatChannelRow { + id: 2, + flags: 0x0_003B, + pattern: "Trade - %s".into(), + shortcut: "Trade".into(), + }, + ]), + ..Default::default() + }; + channels.claim_slot("Trade - City"); + s.run("ChatFrame1.channelList[1] = 'Trade' ChatFrame1.zoneChannelList[1] = 2") + .unwrap(); + + let notice = |name: &str, byte: &str| { + let mut e = ChatEvent::text_only(K::ChannelNotice, String::new()); + e.channel = name.into(); + e.notice = byte.into(); + e + }; + + // The walk: LEAVE goes out, then the eligibility test suspends the slot. + assert_eq!(channels.suspend_slot("Trade - City"), Some(1)); + let before = lines_in_window(&s); + super::feed::deliver( + &mut s, + &mut windows, + &mut channels, + &mut notice("Trade - City", "3"), // YOU_LEFT + ); + s.resolve(); + + // The line still PRINTS — `CHAT_SUSPENDED_NOTICE` is "Left Channel: [%s]", the same text as + // `CHAT_YOU_LEFT_NOTICE`. Only arg1 differs, and arg1 is what the stock handler branches on. + // Asserted because a missing string would make `compose_notice` answer `None` and the line + // would vanish silently — the failure this whole area is prone to. + assert_eq!(lines_in_window(&s), before + 1); + assert!( + s.extract().iter().any(|q| matches!( + &q.content, + benilla_ui::script::QuadContent::Text { text: Some(t), .. } + if t == "Left Channel: [1. Trade - City]" + )), + "the suspended leave renders the same text as an ordinary one" + ); + + assert_eq!( + channels.number_of("Trade - City"), + Some(1), + "the record and its number survive — `/1` still addresses Trade, and the state-3 bypass \ + needs the slot to be there to bypass onto" + ); + assert_eq!( + s.eval::>("return ChatFrame1.zoneChannelList[1]") + .unwrap(), + Some(2), + "and the window is still registered for it: the notice carried the SUSPENDED token, so \ + the stock handler never reached the arm that deletes the registration" + ); + + // Walking back in: the same slot, the same number, and the notice prints again. + let before = lines_in_window(&s); + super::feed::deliver( + &mut s, + &mut windows, + &mut channels, + &mut notice("Trade - City", "2"), // YOU_JOINED + ); + assert_eq!( + lines_in_window(&s), + before + 1, + "the re-join prints — it could not have, with the registration gone" + ); + assert_eq!(channels.number_of("Trade - City"), Some(1)); + assert_eq!( + channels.slot_state("Trade - City"), + Some(super::edit::SlotState::Joined), + "and the slot is back to plain joined" + ); +} + +/// **A renamed row's confirming notice is `YOU_CHANGED`, and it renders "Changed Channel:"** +/// (decision 2130). +/// +/// `CHAT_YOU_CHANGED_NOTICE = "Changed Channel: [%s]"` is a string 1.12 ships and we had never +/// printed, because we modelled no per-slot state to select it with (`0x02`'s arm splits on +/// `rec+0x9c == 2`). It is what a zone-border crossing actually looks like in the reference: one +/// line, not a leave and a join. +#[test] +fn a_renamed_zone_channel_confirms_as_changed_not_joined() { + let _data = benilla_formats::wow_data_or_skip!(); + let mut s = chat_vm(); + let mut windows = super::frames::ChatWindows::default(); + let mut channels = super::edit::ChannelState { + channels: benilla_formats::ChatChannelsCatalog::from_rows(vec![ + benilla_formats::ChatChannelRow { + id: 1, + flags: 0x0_0003, + pattern: "General - %s".into(), + shortcut: "General".into(), + }, + ]), + ..Default::default() + }; + channels.claim_slot("General - Elwynn Forest"); + s.run("ChatFrame1.channelList[1] = 'General' ChatFrame1.zoneChannelList[1] = 1") + .unwrap(); + channels.rename_slot("General - Elwynn Forest", "General - Westfall"); + + let mut e = ChatEvent::text_only(K::ChannelNotice, String::new()); + e.channel = "General - Westfall".into(); + e.notice = "2".into(); // YOU_JOINED + super::feed::deliver(&mut s, &mut windows, &mut channels, &mut e); + s.resolve(); + + let want = "Changed Channel: [1. General - Westfall]"; + assert!( + s.extract().iter().any(|q| matches!( + &q.content, + benilla_ui::script::QuadContent::Text { text: Some(t), .. } if t == want + )), + "expected {want:?} in the window" + ); + assert_eq!( + channels.slot_state("General - Westfall"), + Some(super::edit::SlotState::Joined), + "and the confirming notice resolves the state — a second crossing must read as a rename \ + of its own, not as a leftover" + ); +} + /// **The leave line still knows its number, because the record dies after the line** (1275). /// /// [`super::feed::deliver`] is the ordering under test: we used to drop the channel from the joined @@ -809,7 +1086,7 @@ fn a_leave_notice_keeps_its_number_because_the_record_dies_after_the_line() { "Left Channel: [2. General - Elwynn Forest]" ); assert_eq!( - channels.joined, + channels.names(), [Some("World".to_string()), None], "and only THEN is the record gone — as a HOLE at slot 2, not a shortened list (1286)" ); @@ -920,7 +1197,7 @@ fn every_notice_token_resolves_and_the_tokenless_bytes_stay_silent() { s.lua().globals().get::(key).ok() }) }); - match super::event::notice_token(byte) { + match super::event::notice_token(byte, None) { Some(token) => assert!( rendered.is_some(), "notice {byte:#04x} has token {token} but CHAT_{token}_NOTICE resolves to nothing" diff --git a/crates/benilla-app/src/ui_saved.rs b/crates/benilla-app/src/ui_saved.rs index 3b3fb3c2..76e67133 100644 --- a/crates/benilla-app/src/ui_saved.rs +++ b/crates/benilla-app/src/ui_saved.rs @@ -41,13 +41,25 @@ impl Plugin for UiSavedPlugin { } } -/// Execute the saved-variables file into the VM and fire `VARIABLES_LOADED`. +/// Execute the saved-variables file into the VM, let the host restore what it keeps elsewhere, +/// and fire `VARIABLES_LOADED`. /// /// Called from the in-game UI load ([`crate::ui_script`]) at the reference's own seam — after the /// XML, before anything consumes it. A missing file is the normal first-run case (defaults stand); /// a malformed one warns and is left on disk untouched, so a hand edit that fails to parse costs /// this session's settings and not the file. -pub(crate) fn load_saved_variables(script: &mut UiScript) { +/// +/// **`host_settings` is the third store's turn** (decision 2132). A handful of the reference's +/// `RegisterForSave` globals are settings benilla persists in `config.toml` instead — the +/// nameplate pair is the first — and those have to land in the VM too, or the stock file reads +/// nil and acts on it. The seat is exact and both edges are load-bearing: **after** the chunk, so +/// a stale line in this file cannot outvote `config.toml` (0954's store), and **before** the +/// event, because `VARIABLES_LOADED` is precisely where the consumers run +/// (`UIParent_OnEvent` → `UpdateNameplates`). +pub(crate) fn load_saved_variables( + script: &mut UiScript, + host_settings: impl FnOnce(&mut UiScript), +) { // `None` = hermetic capture, or no install — session-only state, and the event below still fires. if let Some(path) = crate::local_state::saved_variables_path() { match std::fs::read_to_string(&path) { @@ -65,6 +77,7 @@ pub(crate) fn load_saved_variables(script: &mut UiScript) { Err(e) => warn!("saved variables: cannot read {}: {e}", path.display()), } } + host_settings(script); // `VARIABLES_LOADED` fires whether or not there was a file: it means "the settings are now what // they are going to be", which is as true on a first run or a hermetic capture as after a real // restore — the reference fires it as a step of the load sequence, not conditionally. A window @@ -146,7 +159,7 @@ mod tests { // The restart: this VM's own default is 1, the file says 7, and the file wins. let mut fresh = script("1"); - load_saved_variables(&mut fresh); + load_saved_variables(&mut fresh, |_| {}); assert_eq!(fresh.eval::("return KEPT").unwrap(), 7); assert_eq!( fresh.eval::("return VL_SEEN").unwrap(), @@ -157,7 +170,7 @@ mod tests { // A malformed file is left alone, and this session simply runs on defaults. crate::local_state::write_atomic(&path, "KEPT = = 3\n").unwrap(); let mut broken = script("1"); - load_saved_variables(&mut broken); + load_saved_variables(&mut broken, |_| {}); assert_eq!(broken.eval::("return KEPT").unwrap(), 1); assert_eq!(std::fs::read_to_string(&path).unwrap(), "KEPT = = 3\n"); std::fs::remove_dir_all(&tmp).ok(); @@ -180,7 +193,7 @@ mod tests { assert!(!tmp.exists(), "a capture must not plant a settings file"); // The load is a no-op too — but the event still fires, so a window waiting on it is not stuck. let mut fresh = script("1"); - load_saved_variables(&mut fresh); + load_saved_variables(&mut fresh, |_| {}); assert_eq!(fresh.eval::("return KEPT").unwrap(), 1); assert_eq!(fresh.eval::("return VL_SEEN").unwrap(), 1); } diff --git a/crates/benilla-app/src/ui_script/lifecycle.rs b/crates/benilla-app/src/ui_script/lifecycle.rs index e5ebcbaf..427496cc 100644 --- a/crates/benilla-app/src/ui_script/lifecycle.rs +++ b/crates/benilla-app/src/ui_script/lifecycle.rs @@ -444,9 +444,23 @@ pub(crate) fn load_ingame_ui_on_world_entry(world: &mut World) { // any `CHAT_MSG_*` (ref `ChatFrame.lua` l.1261-1273) — as an `Update` system it landed after // the session's first chat had already been routed, and the login MOTD went to a window // registered for nothing. - finish_ui_load_with(&mut script, |script| { - crate::ui_chat::restore_chat_looks(world, script); - }); + // The plate pair is read out of the world FIRST: the second closure borrows `world` for the + // chat-cache restore, and a `Copy` of two bools costs nothing next to fighting that borrow. + // Absent in a bare test world, where "both off" is also the resource's own default. + let plates = world + .get_resource::() + .copied() + .unwrap_or_default(); + finish_ui_load_with( + &mut script, + // `NAMEPLATES_ON` / `FRIENDNAMEPLATES_ON` (2132). This seat, and not the `Update` feed + // beside it, is what fixes the bug: `UIParent_OnEvent`'s first `UpdateNameplates()` runs + // inside the `VARIABLES_LOADED` fired at the end of this very call. + |script| crate::vplates::push_plate_globals(script, plates), + |script| { + crate::ui_chat::restore_chat_looks(world, script); + }, + ); // **Say it out loud when an addon didn't load** (decision 1495). Every failure the walk found // is retained now, but a log nobody knows to open does not fix silence — and silence is the // actual defect B293 reports: *"there are a lot of addons that still doesn't work"*, with @@ -758,7 +772,7 @@ pub(crate) fn shutdown_on_exit( /// restore has to sit inside it. A test that only wants the tail keeps the plain shape. #[cfg(test)] pub(crate) fn finish_ui_load(script: &mut UiScript) { - finish_ui_load_with(script, |_| {}); + finish_ui_load_with(script, |_| {}, |_| {}); } /// [`finish_ui_load`] with the one step that has to land **between** `VARIABLES_LOADED` and @@ -773,14 +787,23 @@ pub(crate) fn finish_ui_load(script: &mut UiScript) { /// an addon reading its chat colours out of a `VARIABLES_LOADED` handler would see the file's /// values where the reference shows it the boot ones. /// -/// A callback rather than a split pair because the budget re-arm and the two fires are one edge, -/// and a caller that forgets the middle step should not be able to compile. -pub(crate) fn finish_ui_load_with(script: &mut UiScript, between: impl FnOnce(&mut UiScript)) { +/// `host_settings` is the earlier of the two seams — **between the saved-variables chunk and +/// `VARIABLES_LOADED`** — where the settings benilla keeps in `config.toml` rather than in that +/// file are pushed into the VM (decision 2132; the rationale for the exact seat is on +/// [`crate::ui_saved::load_saved_variables`]). +/// +/// Callbacks rather than a split trio because the budget re-arm and the two fires are one edge, +/// and a caller that forgets a middle step should not be able to compile. +pub(crate) fn finish_ui_load_with( + script: &mut UiScript, + host_settings: impl FnOnce(&mut UiScript), + between: impl FnOnce(&mut UiScript), +) { // Still the load edge, so still bounded (1306) — re-armed because the walk's last addon left // an arbitrary amount on the counter, and the saved-variables chunk plus every PLAYER_LOGIN // handler deserve the full allowance. The entry edge disarms after this returns. script.set_instruction_budget(addons::LOAD_INSTRUCTION_BUDGET); - crate::ui_saved::load_saved_variables(script); + crate::ui_saved::load_saved_variables(script, host_settings); between(script); script.fire_event("PLAYER_LOGIN", vec![]); } diff --git a/crates/benilla-app/src/ui_script/options_tests.rs b/crates/benilla-app/src/ui_script/options_tests.rs index 83fcc187..72fc6220 100644 --- a/crates/benilla-app/src/ui_script/options_tests.rs +++ b/crates/benilla-app/src/ui_script/options_tests.rs @@ -1363,6 +1363,98 @@ fn the_world_detail_slider_writes_the_cvar_and_the_readout_names_its_stop() { /// The Nameplates page (0992 — live at last): the three 1.12 UnitName* checkbox rows read the /// table on select and write their flags on the interface panel's own click kit (checked → /// CheckBoxOn; these rows carry no soundQuirk). +/// **A world entry must leave the saved nameplate setting alone** (decision 2132) — the report, +/// on the real manifest with a real CVar table, which is the one combination no test had. +/// +/// `UIParent_OnEvent` calls `UpdateNameplates()` twice per entry — `UIParent.lua` l.234 +/// (VARIABLES_LOADED) and l.367 (PLAYER_ENTERING_WORLD) — and it acts on +/// `NAMEPLATES_ON`/`FRIENDNAMEPLATES_ON`. On benilla the four verbs it calls write the CVar pair, +/// and the CVar pair IS the store, so with the globals left nil every entry wrote `"0"` over the +/// player's `"1"` and `config.toml` then dropped the line as "at default". +/// +/// Two mechanisms are needed and this asserts both: the host seeding the globals ahead of +/// `VARIABLES_LOADED` (the enemy half), and our re-declared `UpdateNameplates` without the stock +/// `ShowFriendNameplates(); HideFriendNameplates();` typo (the friendly half). Drop either and the +/// matching row here goes back to `"0"`. +#[test] +fn a_world_entry_leaves_the_saved_nameplate_setting_alone() { + let _data = benilla_formats::wow_data_or_skip!(); + let mut s = UiScript::new().unwrap(); + s.set_screen_size(1024.0, 768.0); + // A manifest load with no player is a state the client never reaches (1848). + s.set_unit( + "player", + Some(benilla_ui::script::UnitState { + exists: true, + name: Some("Probefour".into()), + level: 60, + ..Default::default() + }), + ); + let failures = super::load_default_ui(&s); + assert!(failures.is_empty(), "loader errors: {failures:?}"); + let _ = s.errors(); + + // The player has both halves on, restored from `config.toml` — a host write, so nothing is + // queued and the file is not re-dirtied by having been read. + let plates = crate::vplates::VPlateMode { + enemies: true, + friends: true, + }; + let saved = |s: &mut UiScript| { + s.set_cvar_host(crate::vplates::CVAR_ENEMIES, "1"); + s.set_cvar_host(crate::vplates::CVAR_FRIENDS, "1"); + assert!(s.take_cvar_changes().is_empty(), "the host write is silent"); + }; + + // ── The bug, kept as the control: with the globals nil the entry erases both halves ──────── + saved(&mut s); + s.fire_event("VARIABLES_LOADED", vec![]); + s.fire_event("PLAYER_ENTERING_WORLD", vec![]); + assert_eq!( + ( + s.cvar(crate::vplates::CVAR_ENEMIES), + s.cvar(crate::vplates::CVAR_FRIENDS) + ), + (Some("0".into()), Some("0".into())), + "nil globals ⇒ the replay writes the store off — this is what the report saw" + ); + let _ = s.take_cvar_changes(); + + // ── The fix: the host seeds the globals, and the entry is a no-op end to end ─────────────── + saved(&mut s); + crate::vplates::push_plate_globals(&s, plates); + assert_eq!( + s.eval::("return NAMEPLATES_ON").unwrap(), + 1, + "the reference's own truthiness — the NUMBER 1, never the truthy `0`" + ); + s.fire_event("VARIABLES_LOADED", vec![]); + s.fire_event("PLAYER_ENTERING_WORLD", vec![]); + assert_eq!( + ( + s.cvar(crate::vplates::CVAR_ENEMIES), + s.cvar(crate::vplates::CVAR_FRIENDS) + ), + (Some("1".into()), Some("1".into())), + "the player's setting survives the entry" + ); + assert!( + s.take_cvar_changes().is_empty(), + "and queues nothing, so `config.toml` is never dirtied by a world entry" + ); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); + + // The other direction still works: an addon that moves a global and calls the verb. + s.run("FRIENDNAMEPLATES_ON = nil UpdateNameplates()") + .unwrap(); + assert_eq!( + s.take_cvar_changes(), + vec![(crate::vplates::CVAR_FRIENDS.to_string(), "0".to_string())], + "UpdateNameplates keeps its contract — it is adapted, not stubbed" + ); +} + #[test] fn the_nameplates_page_toggles_the_unit_name_cvars() { let mut s = harness_on(audio_harness()); diff --git a/crates/benilla-app/src/ui_script/test_ui.rs b/crates/benilla-app/src/ui_script/test_ui.rs index b50c9d92..ec372007 100644 --- a/crates/benilla-app/src/ui_script/test_ui.rs +++ b/crates/benilla-app/src/ui_script/test_ui.rs @@ -227,8 +227,9 @@ pub(super) const UIPARENT_STAND_INS: &str = r#" RaidOptionsFrame_UpdatePartyFrames = RaidOptionsFrame_UpdatePartyFrames or function() end LocalizeFrames = LocalizeFrames or function() end updateContainerFrameAnchors = updateContainerFrameAnchors or function() end - -- Ours, not the reference's file: 1.12 keeps UpdateNameplates in UIOptionsFrame.lua and - -- benilla's own options window carries it (the options family stays ours). + -- 1.12 keeps UpdateNameplates in UIOptionsFrame.lua, which a kit reaches only at manifest + -- l.21; our own OptionsFrame.xml re-declares it below that (decision 2132). Both are plain + -- `function X()` writes, so a full kit ends on ours and a short one keeps this no-op. UpdateNameplates = UpdateNameplates or function() end CloseAllBags = CloseAllBags or function() end OpenBackpack = OpenBackpack or function() end diff --git a/crates/benilla-app/src/vplates.rs b/crates/benilla-app/src/vplates.rs index aefa3c59..4ed495c9 100644 --- a/crates/benilla-app/src/vplates.rs +++ b/crates/benilla-app/src/vplates.rs @@ -120,7 +120,7 @@ mod border; /// re-weighed as a default. The `Default` is derived now that the claim is "both false", and it is /// still a *claim*: `cvars::tests` welds it to the registered `nameplateShowEnemies`/`Friendly` /// defaults and asserts the pair is off in as many words, so the derive cannot drift quietly. -#[derive(Resource, Default)] +#[derive(Resource, Default, Clone, Copy)] pub(crate) struct VPlateMode { pub(crate) enemies: bool, pub(crate) friends: bool, @@ -328,6 +328,60 @@ pub(crate) fn text_px(h: f32, basis: f32) -> f32 { (h * basis).round().min(32.0) } +/// **The FrameXML mirror of the two toggles** — `NAMEPLATES_ON` and `FRIENDNAMEPLATES_ON` +/// (decision 2132), pushed into whichever VM is live. +/// +/// The reference keeps the plate state in **two** levels: the engine bitmask `[0xc4da34]`, which +/// is volatile and cleared on every `EnterWorld`, and those two FrameXML globals, which are the +/// saved store. `UpdateNameplates` (`UIOptionsFrame.lua` l.768) replays the store into the +/// bitmask, from `UIParent_OnEvent`'s VARIABLES_LOADED and PLAYER_ENTERING_WORLD arms +/// (`UIParent.lua` l.234, l.367). +/// +/// **benilla has only one level.** [`VPlateMode`] is the state and the [`CVAR_ENEMIES`] / +/// [`CVAR_FRIENDS`] pair is its persistence, and nothing clears it at a world entry — so on this +/// engine the replay has nothing to restore and everything to break. With the globals left nil +/// (nothing here ever wrote them) `UpdateNameplates` took its else branch and called +/// `HideNameplates()`, which writes the CVar, which IS the store: the player's setting was erased +/// at every world entry and `config.toml` lost the line as "at default". +/// +/// So the globals are kept TRUE, which makes the stock replay a value-preserving no-op — and is +/// what a third-party addon reading `NAMEPLATES_ON` is owed anyway, which was 2115's whole +/// argument for loading the stock window in the first place. +pub(crate) fn push_plate_globals(script: &benilla_ui::script::UiScript, mode: VPlateMode) { + // The reference's own truthiness for these two: the number `1`, or nil. Never `0` — a Lua + // `0` is truthy, so `NAMEPLATES_ON = 0` would read as ON in `UpdateNameplates`'s `if`. + let on = |b: bool| b.then_some(1i64); + let g = script.lua().globals(); + if let Err(e) = g + .set("NAMEPLATES_ON", on(mode.enemies)) + .and_then(|()| g.set("FRIENDNAMEPLATES_ON", on(mode.friends))) + { + warn!("nameplates: FrameXML globals: {e}"); + } +} + +/// The globals kept in step with the mode for the life of each VM ([`push_plate_globals`]). +/// +/// Behind a [`crate::ui_script::VmMemo`] (1290) because "this VM has been told" is a fact about +/// the VM, not about the process. The world-entry load seeds them earlier still — ahead of +/// `VARIABLES_LOADED`, which no `Update` system can reach — so this is the steady-state half: +/// a V press, an options checkbox, a `/console` write, an addon's `SetCVar`. +fn feed_plate_globals( + script: Option>, + mode: Res, + mut told: Local>>, +) { + let Some(script) = script else { + return; + }; + let now = (mode.enemies, mode.friends); + let told = told.get(&script); + if *told != Some(now) { + *told = Some(now); + push_plate_globals(&script, *mode); + } +} + /// NAMEPLATES / FRIENDNAMEPLATES / ALLNAMEPLATES through the binding table (0997; defaults V / /// SHIFT-V / CTRL-V). The typing gate and 0585's modifier law live in the dispatch now. /// @@ -1047,7 +1101,14 @@ impl Plugin for VPlatesPlugin { ) .chain() .in_set(VPlateSet), - ); + ) + // **Outside [`VPlateSet`] deliberately.** Its only ordering need is to be ahead of the + // script tick, and three sets order *after* `VPlateSet` while `drive_vplates` sits in + // `UiQuadAppend` — pulling the whole set in front of `UiInput` to carry one system + // would rewire all of that. A V press can therefore reach the globals a frame late, + // which costs nothing: their only readers are `UpdateNameplates` at the two world-entry + // events and an addon that calls it, never a per-frame path. + .add_systems(Update, feed_plate_globals.before(crate::ui_script::UiInput)); } } @@ -1187,6 +1248,55 @@ mod tests { .is_empty()); } + /// **The FrameXML mirror is owed to every VM** (decision 2132) — the mode's two bits reach + /// `NAMEPLATES_ON`/`FRIENDNAMEPLATES_ON` as the reference's `1`-or-nil, follow a change, and + /// are handed to a rebuilt VM (a `/reload`) without one. + #[test] + fn the_plate_globals_follow_the_mode_and_survive_a_rebuilt_vm() { + let read = |app: &mut App| { + let s = app + .world_mut() + .non_send_resource_mut::(); + ( + s.lua().globals().get::>("NAMEPLATES_ON").ok(), + s.lua() + .globals() + .get::>("FRIENDNAMEPLATES_ON") + .ok(), + ) + }; + let mut app = App::new(); + app.add_systems(Update, feed_plate_globals) + .init_resource::() + .insert_non_send_resource(benilla_ui::script::UiScript::new().unwrap()); + + app.update(); + assert_eq!(read(&mut app), (Some(None), Some(None)), "both off ⇒ nil"); + + app.world_mut().resource_mut::().enemies = true; + app.update(); + assert_eq!( + read(&mut app), + (Some(Some(1)), Some(None)), + "the reference's own truthiness: the NUMBER 1, never a truthy `0`" + ); + + // `ReloadUI()`: a fresh VM, and the mode did not move. The memo is keyed on the VM's + // session (1290), so the new one is told again rather than inheriting the old one's claim. + app.insert_non_send_resource(benilla_ui::script::UiScript::new().unwrap()); + assert_eq!( + read(&mut app), + (Some(None), Some(None)), + "a fresh VM knows nothing" + ); + app.update(); + assert_eq!( + read(&mut app), + (Some(Some(1)), Some(None)), + "…and is handed the mode again" + ); + } + /// The palette selector follows `0x7cbaa0`'s exact test order — notably reaction 2 /// (unfriendly) is YELLOW with 3, not hostile red, and a hostile player is red before the /// player-blue test. diff --git a/crates/benilla-app/src/world_state_ui.rs b/crates/benilla-app/src/world_state_ui.rs index 35c96376..707d4769 100644 --- a/crates/benilla-app/src/world_state_ui.rs +++ b/crates/benilla-app/src/world_state_ui.rs @@ -81,9 +81,7 @@ fn load_world_state_ui(mut commands: Commands, assets: Option>) fn defense_channel_joined(channels: &crate::ui_chat::ChannelState) -> bool { const REQUIRED: u32 = chan::ZONE_DEP | chan::DEFENSE; channels - .joined - .iter() - .flatten() + .iter_names() .filter_map(|name| channels.channels.row_for_name(name)) .any(|row| row.flags & REQUIRED == REQUIRED) } diff --git a/crates/benilla-app/tests/common/mod.rs b/crates/benilla-app/tests/common/mod.rs index 3f16608e..378990f6 100644 --- a/crates/benilla-app/tests/common/mod.rs +++ b/crates/benilla-app/tests/common/mod.rs @@ -84,8 +84,9 @@ const UIPARENT_STAND_INS: &str = r#" RaidOptionsFrame_UpdatePartyFrames = RaidOptionsFrame_UpdatePartyFrames or function() end LocalizeFrames = LocalizeFrames or function() end updateContainerFrameAnchors = updateContainerFrameAnchors or function() end - -- Ours, not the reference's file: 1.12 keeps UpdateNameplates in UIOptionsFrame.lua and - -- benilla's own options window carries it (the options family stays ours). + -- 1.12 keeps UpdateNameplates in UIOptionsFrame.lua, which a kit reaches only at manifest + -- l.21; our own OptionsFrame.xml re-declares it below that (decision 2132). Both are plain + -- `function X()` writes, so a full kit ends on ours and a short one keeps this no-op. UpdateNameplates = UpdateNameplates or function() end CloseAllBags = CloseAllBags or function() end OpenBackpack = OpenBackpack or function() end diff --git a/crates/benilla-world/src/terrain_stream/queries.rs b/crates/benilla-world/src/terrain_stream/queries.rs index 3b43ef7f..0079ad9a 100644 --- a/crates/benilla-world/src/terrain_stream/queries.rs +++ b/crates/benilla-world/src/terrain_stream/queries.rs @@ -13,12 +13,21 @@ use super::TerrainStreamer; /// resident chunk's MCNK `areaId` (decision 0070: drives zone music/ambience/reverb; later the /// minimap zone text). `None` until the ground tile is resident (or off-terrain). Written each /// frame by [`update_current_area`]; consumers change-detect on the inner value. +/// +/// **It is scoped to the character session, not to the process** (decision 2130). It goes back to +/// `None` the moment there is no avatar to measure from, so "we have not answered yet" is never +/// spelled the same way as "here is the character before this one". #[derive(Resource, Default, PartialEq, Eq)] pub struct CurrentArea(pub Option); /// Ordering handle on [`update_current_area`] — the leaf-authority write. The zone-text feed /// (`crate::area`) orders after it (which itself orders after the interior claim), so leaf + /// indoor bit + names always come from one coherent frame — the client's single-pass resolve. +/// +/// **Every consumer that ACTS on the area belongs after it**, and the zone-channel walk is the +/// case that proves it (decision 2130): unordered, it read whatever the previous frame had +/// published and paid for the difference in `CMSG_JOIN_CHANNEL`/`CMSG_LEAVE_CHANNEL` traffic for +/// zones the player was never in. #[derive(SystemSet, Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct AreaAuthoritySet; @@ -153,7 +162,26 @@ pub(super) fn update_current_area( wmo_areas: Option>, ) { let Some(wow) = focus.body_pos() else { - return; // no avatar — the area authority follows the character, and there is none + // **No avatar — and the authority DIES with the character session** (decision 2130). + // + // The hold below is a within-session convenience (a tile-edge crossing must not flicker + // through `None`). Holding across the *session* boundary is a different thing entirely: + // this resource had exactly one writer, which only ever assigned `Some`, so once set it + // could never return to `None` for the life of the process — and the next character's + // login read the PREVIOUS character's zone until their own tiles decoded. The zone-channel + // walk believed it and joined `General - Stormwind City` for a character standing in the + // Eastern Plaguelands, then left it again a beat later; `sound/mod.rs`'s `world_audio_live` + // already carried a hand-written guard against the same staleness, which is the tell that + // the defect was the lifetime and not either consumer. + // + // `body_pos()` is `Some` only for a live avatar (`ViewFocus::body`/`detached`) and `None` + // for both glue-screen focuses, so this is the session edge without a state transition to + // subscribe to. A recoverable disconnect keeps the body as the local puppet (0065), so the + // area correctly survives one. + if area.0.is_some() { + *area = CurrentArea(None); + } + return; }; if !focus.body_settled() { // …and one whose own world is still arriving has no area worth publishing: the leaf under @@ -183,3 +211,81 @@ pub(super) fn update_current_area( *area = CurrentArea(found); } } + +#[cfg(test)] +mod tests { + use super::*; + use bevy::ecs::system::RunSystemOnce; + + /// A world with every resource [`update_current_area`] reads, and no terrain at all — so the + /// only thing under test is what the system does about the *body*, not what it finds under one. + fn bare_world() -> World { + let mut w = World::new(); + w.init_resource::(); + w.init_resource::(); + w.init_resource::(); + w.init_resource::>(); + w.init_resource::(); + w + } + + /// **The area authority dies with the character session** (decision 2130). + /// + /// It had exactly one writer, which only ever assigned `Some`, and nothing reset it — so once + /// a character had published a zone it stayed published for the life of the *process*. The + /// next login read it before its own tiles decoded, and the zone-channel walk turned that into + /// `CMSG_JOIN_CHANNEL` for the previous character's capital: the director logged into a + /// character in the Eastern Plaguelands and watched Stormwind City's three channels join and + /// then leave again. + #[test] + fn no_avatar_means_no_area() { + let mut w = bare_world(); + // A character session that published its zone… + w.insert_resource(CurrentArea(Some(1519))); // Stormwind City + w.insert_resource(crate::terrain_stream::ViewFocus::body( + [-8900.0, -130.0, 80.0], + true, + )); + w.run_system_once(update_current_area).unwrap(); + assert_eq!( + w.resource::().0, + Some(1519), + "with a body and no resident tile the last real answer is HELD — that is the \ + tile-edge/flicker guard, and it stays" + ); + + // …and then logged out. `ViewFocus` carries no body at either glue screen. + w.insert_resource(crate::terrain_stream::ViewFocus::camera()); + w.run_system_once(update_current_area).unwrap(); + assert_eq!( + w.resource::().0, + None, + "the authority follows the character, and there is none — a held value here is the \ + NEXT character's login reading THIS character's zone" + ); + + // And the entry-window focus (decision 0777) is the same: a picked row is not a body. + w.insert_resource(CurrentArea(Some(1519))); + w.insert_resource(crate::terrain_stream::ViewFocus::entry( + 0, + [-8900.0, -130.0, 80.0], + )); + w.run_system_once(update_current_area).unwrap(); + assert_eq!(w.resource::().0, None); + } + + /// A **recoverable** disconnect keeps the avatar as the local puppet (0065), so the area must + /// survive one: nothing about the world under the player's feet changed when the socket died. + /// This is why the clear hangs off "is there a body" rather than off the net session. + #[test] + fn a_body_that_survives_a_drop_keeps_its_area() { + let mut w = bare_world(); + w.insert_resource(CurrentArea(Some(12))); + w.insert_resource(crate::terrain_stream::ViewFocus::body( + [0.0, 0.0, 0.0], + false, + )); + w.run_system_once(update_current_area).unwrap(); + assert_eq!(w.resource::().0, Some(12)); + } +}