zone channels survive a border crossing, and nameplates survive a login

- crossing a zone border prints one Changed Channel line instead of a leave and a join, and General and LocalDefense keep printing in the new zone; walking out of a capital keeps Trade's number and its registration instead of dropping it (2130)
- outside a capital the channel numbers are the reference's: a fresh character holds 1 General, 2 Trade (suspended until a city) and 3 LocalDefense, so /2 reaches the same channel it does on the real client (2137)
- logging into a character reads that character's zone, not the previous one's, so a login in the Eastern Plaguelands no longer joins and leaves Stormwind's channels first; a chat cache with General saved as a custom name gets its id back (2130)
- the Show Tutorials option survives a /reload: it loaded unchecked and clicking it reset every dismissed tutorial account-wide; a world entry no longer switches off nameplates you had turned on (2131, 2132)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Sam
2026-09-11 20:51:41 +07:00
co-authored by Claude Fable 5.1
parent 374bc5898d
commit fad9ba02c3
21 changed files with 1444 additions and 135 deletions
+38 -1
View File
@@ -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 —
+9 -3
View File
@@ -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<State<crate::char_select::ClientState>>,
player: Res<Player>,
+90 -9
View File
@@ -143,8 +143,6 @@ pub(crate) struct Tutorials {
/// Published ids (`id + 1`) whose event and cue are owed this frame.
fired: Vec<u32>,
sends: Vec<TutorialSend>,
/// 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<u32> = 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<NonSendMut<UiScript>>,
mut tutorials: ResMut<Tutorials>,
@@ -293,6 +286,7 @@ fn feed_tutorials(
self_q: Query<&ObjectStore, With<SelfPlayer>>,
items: Res<crate::items::Items>,
mut sounds: ResMut<crate::sound::MessageSounds>,
mut told: Local<crate::ui_script::VmMemo<Option<Vec<u8>>>>,
) {
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::<Tutorials>()
.init_resource::<crate::items::Items>()
.init_resource::<crate::sound::MessageSounds>()
.add_message::<TutorialEvent>()
.add_systems(Update, feed_tutorials);
app
}
fn enabled(app: &mut App) -> bool {
app.world_mut()
.non_send_resource_mut::<UiScript>()
.eval::<bool>("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::<Tutorials>()
.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::<Tutorials>()
.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::<UiScript>()
.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::<Tutorials>()
.acknowledge(id::CHATTING);
app.update();
assert!(enabled(&mut app), "a moved bank reaches the live VM");
}
}
+8 -1
View File
@@ -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),
+6 -5
View File
@@ -153,9 +153,7 @@ pub(crate) fn defense_targets(
player_zone: Option<u32>,
) -> Vec<String> {
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()
}
+365 -66
View File
@@ -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<String> {
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<String> {
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<Item = (&'a benilla_formats::ChatChannelRow, String)> + '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<NetCommands>,
channels: Res<ChannelState>,
mut channels: ResMut<ChannelState>,
areas: Option<Res<AreaTableRes>>,
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<String> = Vec::new();
let mut suspended: Vec<String> = Vec::new();
let mut bypassed: Vec<String> = 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 &registered {
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]
+166 -4
View File
@@ -123,11 +123,18 @@ pub(crate) const MAX_CHANNELS: usize = 10;
/// A `Vec<String>` 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<Option<String>>,
pub joined: Vec<Option<ChannelSlot>>,
/// `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<u32> {
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<u32> {
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<SlotState> {
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<Option<String>> {
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<Item = &str> {
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<u32> {
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<u32> {
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)
}
+28 -7
View File
@@ -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<super::edit::SlotState>,
}
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_<X>_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_<X>_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<super::edit::SlotState>,
) -> 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",
+23 -6
View File
@@ -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());
}
}
}
+1 -1
View File
@@ -251,7 +251,7 @@ pub(crate) fn compose_notice(
kind: ChatEventKind,
get: &dyn Fn(&str) -> Option<String>,
) -> Option<String> {
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 {
+3 -1
View File
@@ -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()
+61 -6
View File
@@ -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]
+279 -2
View File
@@ -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::<Option<i64>>("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::<Option<i64>>("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::<String>(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"
+18 -5
View File
@@ -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::<i64>("return KEPT").unwrap(), 7);
assert_eq!(
fresh.eval::<i64>("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::<i64>("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::<i64>("return KEPT").unwrap(), 1);
assert_eq!(fresh.eval::<i64>("return VL_SEEN").unwrap(), 1);
}
+31 -8
View File
@@ -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::<crate::vplates::VPlateMode>()
.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![]);
}
@@ -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::<i64>("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());
+3 -2
View File
@@ -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
+112 -2
View File
@@ -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<NonSendMut<benilla_ui::script::UiScript>>,
mode: Res<VPlateMode>,
mut told: Local<crate::ui_script::VmMemo<Option<(bool, bool)>>>,
) {
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::<benilla_ui::script::UiScript>();
(
s.lua().globals().get::<Option<i64>>("NAMEPLATES_ON").ok(),
s.lua()
.globals()
.get::<Option<i64>>("FRIENDNAMEPLATES_ON")
.ok(),
)
};
let mut app = App::new();
app.add_systems(Update, feed_plate_globals)
.init_resource::<VPlateMode>()
.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::<VPlateMode>().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.
+1 -3
View File
@@ -81,9 +81,7 @@ fn load_world_state_ui(mut commands: Commands, assets: Option<Res<WorldAssets>>)
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)
}
+3 -2
View File
@@ -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
@@ -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<u32>);
/// 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<Res<crate::wmo_portal::WmoAreas>>,
) {
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::<CurrentArea>();
w.init_resource::<crate::terrain_stream::ViewFocus>();
w.init_resource::<TerrainStreamer>();
w.init_resource::<Assets<AdtTile>>();
w.init_resource::<crate::wmo_portal::CurrentAreaInterior>();
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::<CurrentArea>().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::<CurrentArea>().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::<CurrentArea>().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::<CurrentArea>().0, Some(12));
}
}