names persist, the GM notice is a toast, and the target reads Dead

- the name cache persists to `benilla-config/cache/<realm>.tsv`, because the reference's `namecache.wdb` / `petnamecache.wdb` do and that is why it never sees the races we lost every time — unstabling a pet raised `Usage: SetText("text")` off an unresolved `UnitName("pet")`; the header carries magic, format, build, locale and realm and any mismatch discards the file (no checksum, no TTL: eviction is explicit only, via `SMSG_INVALIDATE_PLAYER`, decoded now and never sent by vmangos), negatives persist too, written on a 10 s debounce and at exit; a stable list also seeds the pet-name cache from the names it already carries
- `SMSG_NOTIFICATION` is the red UIErrorsFrame toast, not a chat line — `.gm on` printed twice because vmangos sends both a system message and a notification and both were rendered as chat; the stand-in dated from before the errors frame existed; `SMSG_AREA_TRIGGER_MESSAGE` moves with it on the yellow arm (its handler ends `xor edx,edx` — flag 0 — where the notification's is flag 1)
- the target frame's dead text is the GlobalString `DEAD` = "Dead" resolved through `text=` like the target-of-target frame, not the key typed into `SetText` in caps, and it takes `UnitIsConnected` so a disconnected player does not read as dead; a shipped-script sweep now refuses any key-shaped literal handed to a text sink; and the Help window is one click deep — Home's button opens the ticket text box, Auto-Unstuck sits beside it, and a ticket files as category 0, which a GM's queue names "Unknown" (a guard added days earlier refused 0 on reasoning from the enum rather than from the function that reads it)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Sam
2026-08-29 17:23:05 +07:00
co-authored by Claude Fable 5
parent 0cd0a28942
commit 55b40b5257
28 changed files with 1399 additions and 1120 deletions
File diff suppressed because it is too large Load Diff
+21 -8
View File
@@ -805,14 +805,22 @@
TargetFrame_CheckClassification()
end
-- DEAD text (target only): shown at zero health (ref TargetFrame_CheckDead). Blanked
-- via SetText when alive — a site that predates region Show/Hide (region.rs has the
-- real pair now, the level slot above uses it); equivalent either way.
-- DEAD text (target only), ref TargetFrame_CheckDead (l.220-228) whole: Show/Hide a
-- region whose word the XML already carries. This site used to SetText("DEAD") a
-- LITERAL and blank it when alive, which put the KEY on screen in caps — `DEAD` is a
-- GlobalStrings key (l.898 = "Dead"), not the string, and typing a key as display text
-- is the "CREATE_MACROS" failure of 0983/0991 again. Nothing here names the word now:
-- it comes from the FontString's `text="DEAD"` attribute, which the loader resolves
-- through the global (loader/mod.rs resolve_text), locale and all.
--
-- `UnitIsConnected` is the reference's own second term and is what tells dead from
-- LINK-dead: a disconnected player reads 0 health here and is not dead. The ToT twin
-- (TargetofTarget_CheckDead) has carried it since it was written; this leg had not.
if frame.deadText then
if UnitHealth(unit) <= 0 then
frame.deadText:SetText("DEAD")
if UnitHealth(unit) <= 0 and UnitIsConnected(unit) then
frame.deadText:Show()
else
frame.deadText:SetText("")
frame.deadText:Hide()
end
end
@@ -1599,8 +1607,13 @@
<Anchor point="CENTER" relativePoint="CENTER"><Offset><AbsDimension x="63" y="-16"/></Offset></Anchor>
</Anchors>
</Texture>
<!-- DEAD, CENTER (-50,3), shown at zero health (ref l.173-181). Blank while alive. -->
<FontString name="$parentDeadText" inherits="GameFontNormalSmall">
<!-- DEAD, CENTER (-50,3), shown at zero health (ref l.173-181), spelled
the reference's own way: `text="DEAD"` is a GLOBAL-STRING LOOKUP, so
the word rendered is GlobalStrings.lua l.898's `DEAD = "Dead"` — the
key never reaches the screen. `hidden` like the ToT twin below, a
deliberate divergence: the reference leaves the region shown and
relies on CheckDead landing before the frame does. -->
<FontString name="$parentDeadText" inherits="GameFontNormalSmall" text="DEAD" hidden="true">
<Anchors>
<Anchor point="CENTER" relativePoint="CENTER"><Offset><AbsDimension x="-50" y="3"/></Offset></Anchor>
</Anchors>
+3
View File
@@ -95,6 +95,7 @@ mod run_mode;
mod screenshot;
mod shaders;
mod name_persist;
/// Where "the client is going down" may be observed, and why that is `Last` and not `Update`
/// (decision 1528). Every system that persists state on the way out registers through it.
mod shutdown;
@@ -176,6 +177,7 @@ use entities::EntitiesPlugin;
use fishing_line::FishingLinePlugin;
use footprints::FootprintsPlugin;
use loading_screen::LoadingScreenPlugin;
use name_persist::NamePersistPlugin;
use net::NetPlugin;
use player::PlayerPlugin;
use portrait::PortraitPlugin;
@@ -734,6 +736,7 @@ pub fn run(build: BuildId) -> AppExit {
// copies) read in the reference reader window over the shared ask-once item-text cache.
.add_plugins(UiItemTextPlugin)
.add_plugins(UiSavedPlugin)
.add_plugins(NamePersistPlugin)
.add_plugins(UiStablePlugin)
.add_plugins(UiTrainerPlugin)
// The taxi map (decision 0484 phases 1-2): the SMSG_SHOWTAXINODES-fed TaxiState resource, the
+27 -2
View File
@@ -15,8 +15,11 @@
//! resolve took your config with it. benilla reads a WoW install; it does not live in one. One
//! behaviour improves as a result: the state folder no longer depends on finding the install at
//! all. Everything else about 0954 stands, including this module being the only place in the tree
//! that may compute a persistence path (grep `local_state::` for every resident) — new files get a
//! name here and a line in 0954's layout table.
//! that may compute a persistence path (grep `local_state::` for every resident) — a new file gets
//! a path fn here with a doc comment saying what it holds and why it is scoped the way it is.
//! **Those doc comments are the layout table** (decision 1689): 0954's own table is a
//! point-in-time snapshot of the law it set and, like every decision record, immutable — it lists
//! none of the residents added since, and it should not.
//!
//! Resolution, in order — deliberately the **same three-step shape** as
//! [`benilla_formats::wow_data`], because it is one law over two folders:
@@ -244,6 +247,28 @@ pub(crate) fn layout_character_path(realm: &str, character: &str) -> Option<Path
)))
}
/// `benilla-config/cache/<realm>.tsv` — the **name cache**: the player, creature and pet names the
/// server has already answered for, kept across sessions (decision 1689).
///
/// The reference's residents are `WDB/namecache.wdb`, `creaturecache.wdb` and `petnamecache.wdb`
/// — three files **inside the install**, which is exactly where benilla may not write (the install
/// is read-only, the contract's hard rule), so ours lives here like every other thing we persist.
/// One file rather than three because our three stores share a lifetime and a realm; the
/// reference's split follows its `DBCache<T>` template instantiation, not a property of the data.
///
/// **Realm-scoped, where the reference's is not** — and that is a correctness fix, not a
/// preference. Every key here is realm-local: a player guid, a creature template entry and a pet
/// number all mean something different on another realm, so one shared file would serve a second
/// realm another realm's names. The reference gets away with it because a 1.12 install typically
/// saw one realm.
pub(crate) fn name_cache_path(realm: &str) -> Option<PathBuf> {
Some(
home()?
.join("cache")
.join(format!("{}.tsv", file_token(realm))),
)
}
/// `benilla-config/shots.txt` — the framing instrument's appended camera poses (decision 0600). A dev
/// affordance (`/shot`, compiled out by `--no-default-features` since 1179), but it persists on a
/// real machine, so it resolves here like everything else rather than through a private path.
+160
View File
@@ -0,0 +1,160 @@
//! **The name cache, kept across sessions** (decision 1689) — the load/save half of
//! [`crate::names::NameCache`], and benilla's answer to the reference's `WDB/*.wdb` files.
//!
//! ## Why this exists at all
//!
//! Three of benilla's caches are filled by a server round trip: player names, creature templates
//! and pet names. The reference persists all three (`namecache.wdb`, `creaturecache.wdb`,
//! `petnamecache.wdb`), so on a machine that has played before, an answer is usually *already
//! there* when the packet that needs it arrives. benilla asked fresh every login, which made a
//! race the reference has practically closed wide open for us — the director hit it as a Lua error
//! when unstabling a pet, where `PetStable_Update` hands a nil `UnitName("pet")` to a
//! `GameTooltip:SetText` whose signature requires a string (decision 1688 closed that one path by
//! seeding from the stable list; this closes the class).
//!
//! ## The law, from the carve
//!
//! wow-re carved the whole `DBCache.cpp` machine (`system/dbcache/dbcache.md`, T3 — all 12 record
//! decoders diffed bit-exact). Two of its contracts are the ones a re-implementation must honour,
//! and both are about what the cache does *not* do:
//!
//! - **The header is compared by equality, and carries no checksum, no timestamp and no TTL.** Its
//! 20 bytes are `[FourCC | build 0x16f3 | locale | recordSize | version 1]`. A mismatch discards
//! the file wholesale. Ours is a header line with the same job ([`NameCache::to_tsv`]).
//! - **Eviction is explicit only** — a high-bit key in a response, or `SMSG_INVALIDATE_PLAYER`
//! (`0x31C` → remove-by-key `0x556ff0`). Nothing ages out. So a persisted name lives until the
//! server says otherwise, which is why that opcode had to be wired before anything was written
//! to disk: in memory a stale name costs a session, on disk it costs forever.
//!
//! ## Where it lives, and the one place we deviate
//!
//! The reference writes `WDB/` **inside the install**, which is precisely where benilla may not
//! write (the contract's read-only rule). Ours goes to `benilla-config/cache/<realm>.tsv` through
//! [`crate::local_state`], like every other thing we persist — and it is **realm-scoped**, which
//! the reference's is not. That is a fix rather than a preference: every key is realm-local (a
//! player guid, a creature entry, a pet number), so one shared file would serve another realm's
//! names to this one.
use std::path::PathBuf;
use bevy::prelude::*;
use crate::char_select::Roster;
use crate::names::NameCache;
/// How long a run of landed answers is allowed to accumulate before it is written. The cache is
/// worth keeping but never worth a stall: names arrive in bursts (a city's worth of creature
/// queries on zone-in), and writing the whole file per answer would turn a burst into hundreds of
/// rewrites. A crash inside the window costs re-asking, which is exactly what the cache is for.
const SAVE_DEBOUNCE: f32 = 10.0;
pub(crate) struct NamePersistPlugin;
impl Plugin for NamePersistPlugin {
fn build(&self, app: &mut App) {
app.init_resource::<NameCacheFile>()
.add_systems(Update, (load_name_cache, save_name_cache).chain());
// The exit edge (decision 1528): the last burst of answers is worth the one write, and it
// is the only chance to take it — the debounce above will not fire again.
crate::shutdown::on_app_exit(app, save_on_exit.into_configs());
}
}
/// Where this realm's cache lives and what we last wrote there.
#[derive(Resource, Default)]
pub(crate) struct NameCacheFile {
/// The realm the loaded file belongs to; `None` before the first login. A *change* is what
/// triggers a load — logging into a second realm must not keep the first's names.
realm: Option<String>,
path: Option<PathBuf>,
/// [`NameCache::generation`] as of the last successful write. The cache's own landed-answer
/// counter is the dirty bit — it ticks on an arrival and never on an ask, which is exactly the
/// edge worth writing on.
saved_generation: u64,
since_save: f32,
}
/// Load the realm's cache the first frame its identity is known, and on any realm change.
fn load_name_cache(
roster: Res<Roster>,
mut file: ResMut<NameCacheFile>,
mut names: ResMut<NameCache>,
) {
let Some((realm, _)) = crate::ui_macro::identity(&roster) else {
return;
};
if file.realm.as_deref() == Some(realm.as_str()) {
return;
}
let path = crate::local_state::name_cache_path(&realm);
file.realm = Some(realm.clone());
file.path = path.clone();
file.saved_generation = names.generation();
let Some(path) = path else {
return; // a hermetic capture, or no state folder — session-only, exactly as before
};
let Ok(text) = std::fs::read_to_string(&path) else {
return; // no cache yet: the ordinary first-run path, not an error
};
match NameCache::from_tsv(&text, &realm) {
Some(loaded) => {
let n = loaded.len();
// Replace rather than merge: this is the *start* of a realm's session, so there is
// nothing of this realm's in memory to preserve, and a merge would silently keep the
// previous realm's records alive under their own keys.
*names = loaded;
file.saved_generation = names.generation();
debug!(
"names: loaded {n} cached records for {realm} from {}",
path.display()
);
}
// The header did not match — a different build, locale or format. Discarding the whole
// file is the reference's own rule, and the file is left on disk to be overwritten by the
// first save rather than deleted out from under a player who may be switching back.
None => debug!(
"names: discarding {} — header is not this build/locale/format",
path.display()
),
}
}
/// Write the cache when answers have landed since the last write and the debounce has elapsed.
fn save_name_cache(time: Res<Time>, mut file: ResMut<NameCacheFile>, names: Res<NameCache>) {
file.since_save += time.delta_secs();
if file.since_save < SAVE_DEBOUNCE {
return;
}
file.since_save = 0.0;
write_now(&mut file, &names);
}
/// The exit-edge write (decision 1528) — unconditional on the debounce, because there is no next
/// frame to defer to.
fn save_on_exit(mut file: ResMut<NameCacheFile>, names: Res<NameCache>) {
write_now(&mut file, &names);
}
fn write_now(file: &mut NameCacheFile, names: &NameCache) {
if names.generation() == file.saved_generation {
return; // nothing landed since the last write
}
let (Some(path), Some(realm)) = (file.path.clone(), file.realm.clone()) else {
return;
};
if let Some(dir) = path.parent() {
if let Err(e) = std::fs::create_dir_all(dir) {
warn!("names: cannot create {}: {e}", dir.display());
return;
}
}
match crate::local_state::write_atomic(&path, &names.to_tsv(&realm)) {
Ok(()) => {
file.saved_generation = names.generation();
debug!("names: wrote {} records to {}", names.len(), path.display());
}
// A cache we could not write is a cache we re-fill next session — worth a line, never
// worth failing a session over.
Err(e) => warn!("names: cannot write {}: {e}", path.display()),
}
}
+345
View File
@@ -317,6 +317,182 @@ impl NameCache {
}
}
// ─────────────────────────────────────────────────────────────────────────────────────────────
// Persistence — the `.wdb` law, in benilla's own folder (decision 1689)
// ─────────────────────────────────────────────────────────────────────────────────────────────
/// The file's first line. Every field is compared by **equality** on load and any mismatch
/// discards the whole file — the reference's own rule for its `.wdb` header, whose 20 bytes are
/// `[FourCC | build 0x16f3 | locale | recordSize | version 1]` and carry **no checksum, no
/// timestamp and no TTL** (wow-re `system/dbcache/dbcache.md`, Contracts). Reproducing the
/// *absence* matters as much as the presence: a cache that expired entries on a clock would
/// re-ask for names the server has no reason to have changed, and one that trusted a checksum
/// over a build would deserialize last patch's record layout into this one's struct.
const CACHE_MAGIC: &str = "benilla-namecache";
/// Our own record-layout version — the analogue of the header's `recordSize`+`version` pair. Bump
/// it whenever a column below is added, removed or reordered; the old file is then discarded
/// rather than misparsed.
const CACHE_FORMAT: u32 = 1;
/// The client build the names were answered by — the reference's `0x16f3`. A different build is a
/// different server protocol, so its answers are not ours.
const CACHE_BUILD: u32 = 5875;
/// The locale the names are in. benilla is enUS-only today (every DBC read takes locale slot 0),
/// so this is a constant — but it is *in* the header because that is the reference's law, and the
/// day benilla speaks a second locale a stale enUS cache must be discarded rather than shown.
const CACHE_LOCALE: &str = "enUS";
impl NameCache {
/// Serialize the cache as the TSV this module persists — a header line, then one line per
/// record. Tab-separated because a name may contain spaces but never a tab or a newline, and
/// because a player who opens `benilla-config/` should be able to read what we kept about them.
///
/// **`pending_*` is deliberately not written.** An in-flight ask is a fact about this
/// connection, not about the names; persisting it would make a cache reloaded mid-query
/// believe an answer was coming that never will.
pub(crate) fn to_tsv(&self, realm: &str) -> String {
let mut out =
format!("{CACHE_MAGIC}\t{CACHE_FORMAT}\t{CACHE_BUILD}\t{CACHE_LOCALE}\t{realm}\n");
for (guid, name) in &self.players {
match name {
// `P` — a known player. The traits ride the same line they arrived on.
Some(name) => {
let (race, class, gender) =
self.player_traits.get(guid).copied().unwrap_or((0, 0, 0));
out.push_str(&format!("P\t{guid}\t{race}\t{class}\t{gender}\t{name}\n"));
}
// `p` — asked, and the server did not know. Worth keeping for the same reason it
// is worth caching in memory: it stops us re-asking a dead guid forever.
None => out.push_str(&format!("p\t{guid}\n")),
}
}
for (entry, rec) in &self.creatures {
match rec {
Some(r) => out.push_str(&format!(
"C\t{entry}\t{}\t{}\t{}\t{}\t{}\t{}\t{}\t{}\t{}\n",
r.creature_type,
r.pet_family,
r.rank,
r.type_flags,
u8::from(r.civilian),
u8::from(r.racial_leader),
r.display_id,
r.name,
r.subname.as_deref().unwrap_or("")
)),
None => out.push_str(&format!("c\t{entry}\n")),
}
}
for (pet_number, name) in &self.pets {
out.push_str(&format!("E\t{pet_number}\t{name}\n"));
}
out
}
/// Rebuild a cache from [`Self::to_tsv`]'s output, or `None` if the header does not match this
/// build/locale/format **exactly**. A malformed or truncated line is skipped rather than
/// failing the load: a half-written cache should cost a re-query, never a broken session.
pub(crate) fn from_tsv(text: &str, realm: &str) -> Option<Self> {
let mut lines = text.lines();
let header: Vec<&str> = lines.next()?.split('\t').collect();
// The equality gate. Five fields, all of them, no tolerance — see `CACHE_MAGIC`.
if header.len() != 5
|| header[0] != CACHE_MAGIC
|| header[1] != CACHE_FORMAT.to_string()
|| header[2] != CACHE_BUILD.to_string()
|| header[3] != CACHE_LOCALE
|| header[4] != realm
{
return None;
}
let mut cache = NameCache::default();
for line in lines {
let f: Vec<&str> = line.split('\t').collect();
match f.first().copied() {
Some("P") if f.len() >= 6 => {
let Ok(guid) = f[1].parse::<u64>() else {
continue;
};
// The name is the LAST field, not `f[5]`: a name cannot contain a tab, so the
// split is exact — but taking the tail keeps this honest if a column is ever
// added before it and the format version is forgotten.
cache.players.insert(guid, Some(f[5].to_string()));
if let (Ok(race), Ok(class), Ok(gender)) =
(f[2].parse(), f[3].parse(), f[4].parse())
{
cache.player_traits.insert(guid, (race, class, gender));
}
}
Some("p") if f.len() >= 2 => {
if let Ok(guid) = f[1].parse::<u64>() {
cache.players.insert(guid, None);
}
}
Some("C") if f.len() >= 11 => {
let Ok(entry) = f[1].parse::<u32>() else {
continue;
};
let (Ok(creature_type), Ok(pet_family), Ok(rank), Ok(type_flags)) =
(f[2].parse(), f[3].parse(), f[4].parse(), f[5].parse())
else {
continue;
};
let Ok(display_id) = f[8].parse() else {
continue;
};
cache.creatures.insert(
entry,
Some(CreatureRecord {
name: f[9].to_string(),
// An empty subname is NO subname, the same mapping the wire decode
// makes — `Some("")` would paint an empty tooltip line.
subname: (!f[10].is_empty()).then(|| f[10].to_string()),
creature_type,
pet_family,
rank,
type_flags,
civilian: f[6] == "1",
racial_leader: f[7] == "1",
display_id,
}),
);
}
Some("c") if f.len() >= 2 => {
if let Ok(entry) = f[1].parse::<u32>() {
cache.creatures.insert(entry, None);
}
}
Some("E") if f.len() >= 3 => {
if let Ok(pet_number) = f[1].parse::<u32>() {
cache.pets.insert(pet_number, f[2].to_string());
}
}
_ => {}
}
}
Some(cache)
}
/// Drop a player's cached name (`SMSG_INVALIDATE_PLAYER`) so the next resolve re-asks — the
/// reference's remove-by-key (`0x556ff0`, wow-re `dbcache.md` Contracts: eviction is
/// **explicit only**, there is no TTL).
///
/// This is the safety valve persistence needs. In memory a stale name lasts a session; on disk
/// it lasts forever, so the one packet that says "forget this guid" has to be honoured. Note
/// that vmangos never sends it — so against our own server this is the mechanism being present
/// and correct rather than a path we exercise.
pub(crate) fn invalidate_player(&mut self, guid: u64) {
self.players.remove(&guid);
self.player_traits.remove(&guid);
self.pending_players.remove(&guid);
self.generation = self.generation.wrapping_add(1);
}
/// How many records the cache holds — the persistence layer's "is this worth writing" read.
pub(crate) fn len(&self) -> usize {
self.players.len() + self.creatures.len() + self.pets.len()
}
}
#[cfg(test)]
mod tests {
use super::*;
@@ -396,6 +572,175 @@ mod tests {
assert_eq!(cache.peek(voidwalker), None);
}
/// **A stable list warms the pet-name cache, so an unstabled pet is named on arrival**
/// (decision 1688). The list carries the same `(pet_number, name)` pair the pet-name query
/// would answer with, and the guid slot benilla reads that number out of is the very one
/// vmangos packs it into (`Object::_Create(guidlow, petNumber, HIGHGUID_PET)`).
///
/// The window this closes is a Lua error the director hit: `PetStable_Update` hands a bare
/// `UnitName("pet")` to `GameTooltip:SetText`, whose byte-pinned signature REQUIRES a string
/// and raises on nil. The reference's own line is unguarded too — it survives because
/// `petnamecache.wdb` persists across sessions and benilla has no such file.
#[test]
fn a_stable_list_names_the_pet_before_it_is_ever_summoned() {
let (cmds, rx) = commands();
let mut cache = NameCache::default();
// The guid the pet will have once summoned: its pet number rides the ENTRY slot.
let rex = compose(guid::HIGH_PET, 7, 42);
// Cold: nil, and an ask goes out — the state that raised the error.
assert_eq!(cache.resolve(rex, &cmds), None);
assert!(matches!(
rx.try_recv(),
Ok(ClientCommand::PetNameQuery { pet_number: 7, .. })
));
// The list lands and seeds the name. Now the pet resolves with NO further query, which is
// the whole point: it is named the frame it is summoned, not a round trip later.
let mut warm = NameCache::default();
warm.insert_pet(7, "Rex".into());
assert_eq!(warm.resolve(rex, &cmds), Some("Rex"));
assert!(
matches!(rx.try_recv(), Err(TryRecvError::Empty)),
"a seeded pet must not re-ask"
);
}
/// **The round trip keeps every record kind and every shape of answer** (decision 1689) —
/// including the two "the server said no" negatives, which are the ones a naive serializer
/// drops: cached as absent, they are what stops us re-asking a dead guid or an unknown entry
/// on every single login, forever.
#[test]
fn the_cache_round_trips_through_its_file_format() {
let (cmds, _rx) = commands();
let mut cache = NameCache::default();
cache.insert_player(0x11, "Sam".into(), Some((1, 2, 0)));
cache.insert_player(0x22, String::new(), None); // the server did not know
cache.insert_creature(
69,
Some(CreatureRecord {
name: "Stable Master Kitrik".into(),
subname: Some("Stable Master".into()),
creature_type: 7,
pet_family: 0,
rank: 1,
type_flags: 0x10,
civilian: true,
racial_leader: false,
display_id: 533,
}),
);
cache.insert_creature(1234, None); // the server flagged the entry unknown
cache.insert_pet(7, "Rex".into());
let text = cache.to_tsv("Hydraxian Waterlords");
let mut back = NameCache::from_tsv(&text, "Hydraxian Waterlords").expect("header matches");
assert_eq!(back.resolve(0x11, &cmds), Some("Sam"));
assert_eq!(back.player_traits(0x11), Some((1, 2, 0)));
// A cached negative must survive as a negative — present-and-None, not absent.
assert_eq!(back.resolve(0x22, &cmds), None);
assert_eq!(back.peek(0x22), None);
let rec = back.creature_record(69).expect("creature survived");
assert_eq!(rec.name, "Stable Master Kitrik");
assert_eq!(rec.subname.as_deref(), Some("Stable Master"));
assert_eq!((rec.creature_type, rec.rank, rec.type_flags), (7, 1, 0x10));
assert_eq!((rec.civilian, rec.racial_leader), (true, false));
assert_eq!(rec.display_id, 533, "the model a stabled pet is drawn from");
assert!(back.creature_record(1234).is_none());
assert_eq!(back.len(), 5);
// An EMPTY subname must come back as no subname, the same mapping the wire decode makes —
// `Some("")` paints an empty tooltip line whose zero-extent slot spills the ones below it.
cache.insert_creature(
70,
Some(CreatureRecord {
name: "Boar".into(),
subname: None,
creature_type: 1,
pet_family: 5,
rank: 0,
type_flags: 0,
civilian: false,
racial_leader: false,
display_id: 1,
}),
);
let back = NameCache::from_tsv(&cache.to_tsv("R"), "R").expect("header");
assert_eq!(back.creature_record(70).unwrap().subname, None);
}
/// **The header is an equality gate with no tolerance** — the reference's own `.wdb` rule, and
/// the reason its cache carries no checksum and no clock. Each field is rejected on its own:
/// a different build speaks a different protocol, a different locale means different strings,
/// a different format version means these columns are not those columns, and a different realm
/// means every key in the file addresses somebody else.
#[test]
fn a_header_that_differs_in_any_field_discards_the_whole_file() {
let mut cache = NameCache::default();
cache.insert_player(0x11, "Sam".into(), None);
let good = cache.to_tsv("Hydraxian Waterlords");
assert!(NameCache::from_tsv(&good, "Hydraxian Waterlords").is_some());
// The realm — the field the reference does NOT have, and the one that would silently
// serve another realm's names.
assert!(NameCache::from_tsv(&good, "Another Realm").is_none());
let head = good.lines().next().unwrap();
for (field, bad) in [
(0, "not-benilla"),
(1, "999"), // format version
(2, "5876"), // client build
(3, "frFR"), // locale
] {
let mut parts: Vec<&str> = head.split('\t').collect();
parts[field] = bad;
let text = format!("{}\n{}", parts.join("\t"), good.lines().nth(1).unwrap());
assert!(
NameCache::from_tsv(&text, "Hydraxian Waterlords").is_none(),
"header field {field} = {bad} must discard the file"
);
}
// Garbage and truncation cost a re-query, never a broken session.
assert!(NameCache::from_tsv("", "R").is_none());
assert!(NameCache::from_tsv("nonsense", "R").is_none());
// A good header with a mangled record line keeps the header's other records.
let text = format!("{}\nP\tnotanumber\t1\t2\t0\tX\nE\t7\tRex\n", head);
let back = NameCache::from_tsv(&text, "Hydraxian Waterlords").expect("header still good");
assert_eq!(back.len(), 1, "the bad line is skipped, the good one kept");
}
/// `SMSG_INVALIDATE_PLAYER`'s eviction — the safety valve persistence needs. The cache ages
/// nothing out, so without this a name written to disk would outlive any change to it.
#[test]
fn invalidating_a_player_makes_the_next_resolve_ask_again() {
let (cmds, rx) = commands();
let mut cache = NameCache::default();
cache.insert_player(0x11, "Sam".into(), Some((1, 2, 0)));
assert_eq!(cache.resolve(0x11, &cmds), Some("Sam"));
assert!(matches!(rx.try_recv(), Err(TryRecvError::Empty)), "no ask");
let before = cache.generation();
cache.invalidate_player(0x11);
assert!(
cache.generation() != before,
"an eviction is a landed change"
);
assert_eq!(
cache.player_traits(0x11),
None,
"the traits go with the name"
);
// The next resolve is a genuine miss again — it asks.
assert_eq!(cache.resolve(0x11, &cmds), None);
assert!(matches!(
rx.try_recv(),
Ok(ClientCommand::NameQuery { guid: 0x11 })
));
}
#[test]
fn creature_miss_queries_once_then_serves_the_answer() {
let (cmds, rx) = commands();
+8 -6
View File
@@ -295,9 +295,10 @@ pub(super) fn apply_net_updates(
ResMut<crate::ui_action::AutoRepeatActive>,
// Our own running channel (the IsCurrentAction channel leg, decision 0137 phase 4).
ResMut<crate::ui_cast::ActiveChannel>,
// Pre-formatted red error lines (the death durability notice — drained by the
// container feed beside EquipErrors).
ResMut<crate::ui_items::UiErrorLines>,
// Pre-formatted UIErrorsFrame lines — text the wire already resolved, so there is no
// GlobalStrings key to look up: the death durability notice, `SMSG_NOTIFICATION`, and
// `SMSG_AREA_TRIGGER_MESSAGE`. Drained by `ui_action::feed_actions` beside UiErrorKeys.
ResMut<crate::ui_action::UiErrorTexts>,
// The queued on-next-swing strike (the melee-slot half of the cast tracking) — the
// wire resolves it here: GO fires it, a failing result/interrupt kills it.
ResMut<crate::ui_cast::QueuedMeleeSpell>,
@@ -977,9 +978,9 @@ pub(super) fn apply_net_updates(
chat::chat_player_not_found(&name, &mut chat_log)
}
SessionEvent::ChatWrongFaction => chat::chat_wrong_faction(&mut chat_log),
SessionEvent::Notification { text } => chat::notification(text, &mut chat_log),
SessionEvent::Notification { text } => chat::notification(text, &mut ui_actions.14),
SessionEvent::AreaTriggerMessage { text } => {
chat::area_trigger_message(text, &mut chat_log)
chat::area_trigger_message(text, &mut ui_actions.14)
}
SessionEvent::PlayedTime { total, level } => {
// BOTH halves, and they are not redundant. The chat breakdown is our stand-in for
@@ -1636,11 +1637,12 @@ pub(super) fn apply_net_updates(
SessionEvent::TrainerBuyFailed { error, .. } => {
npc::trainer_buy_failed(error, &mut ui_actions.8 .0)
}
SessionEvent::InvalidatePlayer { guid } => names::invalidate_player(guid, &mut names),
SessionEvent::ListStabledPets {
npc,
num_stable_slots,
pets,
} => npc::list_stabled_pets(npc, num_stable_slots, pets, &mut stable_open),
} => npc::list_stabled_pets(npc, num_stable_slots, pets, &mut stable_open, &mut names),
SessionEvent::StableResult { result } => npc::stable_result(
result,
&mut stable_open,
+92 -16
View File
@@ -1,11 +1,16 @@
//! Chat-window arm bodies for [`super::apply_net_updates`]'s dispatch match — the spoken line
//! itself, plus the server notices and query answers that render as chat lines (the channel
//! roster, whisper refusals, `SMSG_NOTIFICATION`, `/played`). Each `pub(super)` fn here is exactly
//! one arm's body; the match at the call site stays the dispatcher, one call per arm.
//! roster, whisper refusals, `/played`). Each `pub(super)` fn here is exactly one arm's body; the
//! match at the call site stays the dispatcher, one call per arm.
//!
//! Two arms here do NOT render as chat: `SMSG_NOTIFICATION` and `SMSG_AREA_TRIGGER_MESSAGE` are
//! the reference's UIErrorsFrame toasts, and queue onto [`UiErrorTexts`]. They live in this file
//! because they are text-carrying server notices, not because they share a sink.
use benilla_protocol::messages::{ChatMessage, CHAT_MSG_WHISPER};
use bevy::prelude::*;
use crate::ui_action::UiErrorTexts;
use crate::ui_chat::{ChatEvent, ChatEventKind, ChatLog};
use crate::ui_social::SocialState;
@@ -172,24 +177,39 @@ pub(super) fn chat_wrong_faction(chat_log: &mut ChatLog) {
));
}
/// A server notice (`SMSG_NOTIFICATION`). The ref flashes it in the red UIErrorsFrame
/// (center-screen); benilla has no error frame yet, so the chat feed carries it — an honest
/// divergence, chosen over silence (a dropped notice hid the language-gate bug for weeks:
/// a rejected send looked like nothing at all).
pub(super) fn notification(text: String, chat_log: &mut ChatLog) {
chat_log.push_event(ChatEvent::text_only(ChatEventKind::System, text));
/// A server notice (`SMSG_NOTIFICATION`) — the **red UIErrorsFrame line**, never a chat line.
///
/// Byte-verified in the reference: opcode `0x1cb`'s handler is `0x401800` (registered at
/// `0x40172f`, torn down at `0x401f09`), whose whole body is "read the cstring, then
/// `mov edx,1; lea ecx,[buf]; call 0x4945b0`" plus a console log (`0x63cd00(…, 3, buf)` — our
/// `info!` below). `0x4945b0(text, 1)` fires FrameScript event `0xe0` = `UI_ERROR_MESSAGE`
/// (wow-re `system/ui/ui.md` l.2459). Nothing on this path touches the chat composer.
///
/// **This used to push into the chat feed**, as a stand-in from before benilla had an errors
/// frame. It has had a real one for a long time (`assets/ui/ErrorsFrame.xml`, the ref
/// `UIErrorsFrame` as a genuine `MessageFrame`), and the stand-in outlived its reason: vmangos
/// `Player::SetGameMaster` answers `.gm on|off` with **both** `SendSysMessage` and
/// `SendNotification` (`Objects/Player.cpp:2676-2677`/`2701-2702`), so a client that sinks the
/// notification into chat prints "GM mode is ON" **twice** where the reference prints it once.
pub(super) fn notification(text: String, errors: &mut UiErrorTexts) {
// The handler's own console leg, and the same reason 0651 logs the dot-command answers: a
// toast that flashed for five seconds and one that never arrived look identical afterwards.
info!("net: notification — {text}");
errors.error(text);
}
/// An area trigger's refusal (`SMSG_AREA_TRIGGER_MESSAGE`) — "You must be at least level 58 to
/// enter…", "You cannot enter … while in ghost form." The reference sends it to the **same** sink
/// as [`notification`] (its `0x2b8` arm and the system-message table's kinds 1/2 both end at
/// `0x4945b0`), so it goes wherever that one goes. Without it, a refused portal is silent, which
/// reads exactly like a portal that is still broken.
pub(super) fn area_trigger_message(text: String, chat_log: &mut ChatLog) {
/// enter…", "You cannot enter … while in ghost form." The same frame as [`notification`], the
/// **yellow** arm: `0x2b8` shares the multi-opcode handler `0x48f690`, whose arm at `0x48f8ff`
/// ends `xor edx,edx; call 0x4945b0` — flag 0, so event `0xe1` = `UI_INFO_MESSAGE`. (The older
/// comment here called it the same sink as the notification and left it at that; the sink is the
/// same, the arm is not.) Without it, a refused portal is silent, which reads exactly like a
/// portal that is still broken.
pub(super) fn area_trigger_message(text: String, errors: &mut UiErrorTexts) {
// Logged for the same reason 0651 logs the server's dot-command answers: a trigger that
// refused and a trigger the client never noticed look identical from outside.
info!("net: area-trigger message — {text}");
chat_log.push_event(ChatEvent::text_only(ChatEventKind::System, text));
errors.info(text);
}
/// The `/played` answer (`SMSG_PLAYED_TIME`) — TIME_PLAYED_TOTAL/LEVEL over
@@ -215,7 +235,7 @@ mod tests {
use bevy::ecs::system::RunSystemOnce;
use super::*;
use benilla_protocol::messages::{CHAT_MSG_PARTY, LANGUAGE_ADDON};
use benilla_protocol::messages::{CHAT_MSG_PARTY, CHAT_MSG_SYSTEM, LANGUAGE_ADDON};
fn a_party_line(language: u32, text: &str) -> ChatMessage {
ChatMessage {
@@ -232,6 +252,11 @@ mod tests {
/// Run one inbound line through the real arm body and report whether it reached the chat log.
fn reaches_the_chat_window(m: ChatMessage) -> bool {
chat_lines(m) > 0
}
/// How many lines one inbound message put in the chat window.
fn chat_lines(m: ChatMessage) -> usize {
let mut world = World::new();
world.init_resource::<Messages<ServerSaidMessage>>();
let (tx, _rx) = crossbeam_channel::unbounded();
@@ -254,7 +279,7 @@ mod tests {
},
)
.unwrap();
world.resource::<ChatLog>().pending_len() > 0
world.resource::<ChatLog>().pending_len()
}
/// B215 / decision 1029: an addon broadcast never reaches the chat window. It arrives as an
@@ -330,4 +355,55 @@ mod tests {
);
}
}
/// **The GM-mode double line** — the director's report, pinned end to end.
///
/// vmangos answers `.gm on|off` with **two** packets, not one: `Player::SetGameMaster` calls
/// `SendSysMessage(LANG_GM_ON)` *and* `GetSession()->SendNotification(LANG_GM_ON)`
/// (`src/game/Objects/Player.cpp:2676-2677` and `2701-2702`). They carry the same words down
/// different roads, and only the first is a chat line: the reference's `SMSG_NOTIFICATION`
/// handler `0x401800` is "read the cstring, `mov edx,1`, `call 0x4945b0`" — event `0xe0`
/// `UI_ERROR_MESSAGE`, the UIErrorsFrame toast — plus a console log, and it never reaches the
/// chat composer. Benilla used to park the notice in the chat feed for want of an errors
/// frame; it has had one since `assets/ui/ErrorsFrame.xml`, and the stand-in was what printed
/// "GM mode is ON" twice.
///
/// The pair is the test: BOTH halves of one toggle, one chat line, one toast.
#[test]
fn toggling_gm_mode_prints_one_chat_line_and_one_toast() {
let sys_line = ChatMessage {
chat_type: CHAT_MSG_SYSTEM,
..a_party_line(0, "GM mode is ON")
};
assert_eq!(
chat_lines(sys_line),
1,
"the SendSysMessage half is the chat line, and it stays"
);
let mut errors = UiErrorTexts::default();
notification("GM mode is ON".to_string(), &mut errors);
assert_eq!(
errors.0,
[("GM mode is ON".to_string(), false)],
"the SendNotification half is the RED toast (0x4945b0's flag 1), never a second line"
);
}
/// The notification's sibling arm shares the frame and **not** the colour: `0x2b8`
/// `SMSG_AREA_TRIGGER_MESSAGE` runs the multi-opcode handler `0x48f690`, whose arm at
/// `0x48f8ff` ends `xor edx,edx; call 0x4945b0` — flag 0, so event `0xe1` `UI_INFO_MESSAGE`,
/// the yellow line. This is the half the old "same sink as the notification" comment missed.
#[test]
fn an_area_trigger_refusal_takes_the_yellow_arm() {
let mut errors = UiErrorTexts::default();
area_trigger_message(
"You must be at least level 58 to enter.".to_string(),
&mut errors,
);
assert_eq!(
errors.0,
[("You must be at least level 58 to enter.".to_string(), true)]
);
}
}
+3 -5
View File
@@ -10,7 +10,7 @@ use bevy::prelude::*;
use crate::death::{CorpsePoint, DeathNet, ResurrectOffer};
use crate::net::MoveModeMessage;
use crate::ui_items::UiErrorLines;
use crate::ui_action::UiErrorTexts;
use super::super::SelfGuid;
@@ -89,10 +89,8 @@ pub(super) fn spirit_healer_confirm(npc: u64, death_net: &mut DeathNet) {
/// `SMSG_DURABILITY_DAMAGE_DEATH` — the red line, verbatim GlobalStrings `DURABILITYDAMAGE_DEATH`
/// (the `%%` unescaped).
pub(super) fn durability_damage_death(lines: &mut UiErrorLines) {
lines
.0
.push("Your equipped items suffer a 10% durability loss.".to_string());
pub(super) fn durability_damage_death(errors: &mut UiErrorTexts) {
errors.error("Your equipped items suffer a 10% durability loss.".to_string());
}
/// **The ack'd movement-mode family** (decision 0866) — root, water-walk, feather-fall, hover. The
+13
View File
@@ -3,6 +3,8 @@
//! asked for them). Each `pub(super)` fn here is exactly one arm's body; the match at the call site
//! stays the dispatcher, one call per arm.
use bevy::prelude::debug;
use crate::names::{CreatureRecord, NameCache};
/// `SMSG_NAME_QUERY_RESPONSE` — a player's name, plus the race/gender/class that ride the same
@@ -24,6 +26,17 @@ pub(super) fn pet_name(pet_number: u32, name: String, names: &mut NameCache) {
names.insert_pet(pet_number, name);
}
/// `SMSG_INVALIDATE_PLAYER` — drop this guid so the next resolve re-asks (decision 1689).
///
/// The name cache has **no TTL** (wow-re `dbcache.md`: eviction is explicit only), so this is the
/// one thing that unsticks a player's name. It matters more to benilla than it did before the
/// cache persisted: in memory a stale name lasted a session, on disk it lasts until something
/// evicts it.
pub(super) fn invalidate_player(guid: u64, names: &mut NameCache) {
debug!("net: invalidating cached name for player {guid:#x}");
names.invalidate_player(guid);
}
/// `SMSG_CREATURE_QUERY_RESPONSE` — the template's name plus the hover line's fields. A `None` name
/// is the server's "no such entry", cached as such so the ask never repeats; the remaining fields
/// then carry their miss defaults.
+23
View File
@@ -8,6 +8,7 @@ use benilla_protocol::messages::{
};
use bevy::prelude::*;
use crate::names::NameCache;
use crate::ui_bank::{BankErrors, BankOpen};
use crate::ui_gossip::GossipState;
use crate::ui_merchant::{MerchantErrors, MerchantOpen, MerchantRefusal};
@@ -203,11 +204,33 @@ pub(super) fn list_stabled_pets(
num_stable_slots: u8,
pets: Vec<StabledPet>,
stable_open: &mut StableOpen,
names: &mut NameCache,
) {
debug!(
"net: stable master {npc:#x} listed {} pets ({num_stable_slots} slots bought)",
pets.len()
);
// **Seed the pet-name cache from the list** (decision 1688). Every row carries the pet's own
// number and the name its owner gave it — the exact `(pet_number, name)` pair
// `SMSG_PET_NAME_QUERY_RESPONSE` would answer with — so the pet a player unstables has a
// resolvable `UnitName("pet")` the moment it is summoned, instead of after a round trip.
//
// The window this closes is a real one the director hit: `PetStable_Update` sets the current
// pet's button tooltip to a bare `UnitName("pet")` (`PetStable.lua:161`, transcribed
// unguarded because the reference is unguarded), and hands it to `GameTooltip:SetText`, whose
// byte-pinned signature REQUIRES a string (`0x531b90`) and raises otherwise. Nil name ⇒ Lua
// error dialog, once per unstable.
//
// Why the reference does not trip over its own unguarded line: its pet-name cache is
// `petnamecache.wdb`, which **persists across sessions**, so a pet you have owned before is
// already warm before the packet arrives. benilla has no such file, so a window the reference
// has practically closed is wide open for us. Seeding is not a workaround for the missing
// cache — it is using the answer the server has already sent us in this very packet.
for pet in &pets {
if !pet.name.is_empty() {
names.insert_pet(pet.pet_number, pet.name.clone());
}
}
stable_open.open(npc, num_stable_slots, pets);
}
+31 -1
View File
@@ -11,7 +11,12 @@
//! ("Requires Herbalism", decision 0545) — the latter carry [`UiError`]'s `%s`/`%d`
//! argText fills, resolved by [`ui_error_text`].
//!
//! All three drain in `super::feed_actions`, firing `UI_ERROR_MESSAGE` per resolved line;
//! - [`UiErrorTexts`] — the same route for lines that arrive already resolved (the server's own
//! `SMSG_NOTIFICATION` / `SMSG_AREA_TRIGGER_MESSAGE` text, the death durability notice), so
//! there is no key to look up — only the arm to pick.
//!
//! All four drain in `super::feed_actions`, firing `UI_ERROR_MESSAGE` (or, for the info arm,
//! `UI_INFO_MESSAGE`) per resolved line;
//! every string comes from the VM's own loaded `GlobalStrings.lua`, never hardcoded, so an
//! absent key shows nothing (the reference's data-suppression face) and localization rides
//! for free.
@@ -134,6 +139,31 @@ impl UiError {
#[derive(Resource, Default)]
pub(crate) struct UiErrorKeys(pub Vec<UiError>);
/// [`UiErrorKeys`]' twin for lines that arrive **already resolved** — text with no GlobalStrings
/// key to look up, because the server (or a fixed literal) already wrote it. Same frame, same two
/// events, same drain.
///
/// The wire tenants are the reference's own: `SMSG_NOTIFICATION` (`0x1cb`, handler `0x401800` —
/// `mov edx,1; call 0x4945b0` → `UI_ERROR_MESSAGE`) and `SMSG_AREA_TRIGGER_MESSAGE` (`0x2b8`, the
/// shared handler `0x48f690`'s arm at `0x48f8ff` — `xor edx,edx; call 0x4945b0` →
/// `UI_INFO_MESSAGE`). `0x4945b0(text, flag)` is the whole sink: null/empty guard, then
/// `neg edx; sbb edx,edx; add edx,0xe1` = event `0xe1` when the flag is 0 and `0xe0` when it is 1
/// (wow-re `system/ui/ui.md` l.2459). So the flag IS [`UiError::info`] under another name.
#[derive(Resource, Default)]
pub(crate) struct UiErrorTexts(pub Vec<(String, bool)>);
impl UiErrorTexts {
/// The red `UI_ERROR_MESSAGE` arm (`0x4945b0(text, 1)`).
pub(crate) fn error(&mut self, text: String) {
self.0.push((text, false));
}
/// The yellow `UI_INFO_MESSAGE` arm (`0x4945b0(text, 0)`).
pub(crate) fn info(&mut self, text: String) {
self.0.push((text, true));
}
}
/// Resolve one [`UiError`] to its displayed text — `GetText(key)` + the `%s`/`%d` argText
/// substitution ("Requires %s" + "Herbalism" → "Requires Herbalism", cursor-system.md §8.8).
/// `None` (absent or empty key) = show nothing: GlobalStrings data-suppression, faithfully
+18 -3
View File
@@ -15,10 +15,12 @@
//! 0230/0231 — a swap changes it without touching the action table). Gating those on the same flag
//! is what leaves a stale Count fontstring or a stale Attack face.
//!
//! The feed also pumps the four red-error-line queues (cast fails, mount refusals, the app's
//! by-key local refusals, and the ENGINE's own — `benilla_ui` is engine-free and cannot reach
//! The feed also pumps the UIErrorsFrame queues (cast fails, mount refusals, the app's by-key
//! local refusals, the ENGINE's own — `benilla_ui` is engine-free and cannot reach
//! [`UiErrorKeys`], so a refusal raised inside the script crate queues its GlobalStrings key here
//! instead) into `UI_ERROR_MESSAGE`, and the stance page (`GetBonusBarOffset`) — our descriptor's
//! instead — and [`UiErrorTexts`], the lines that arrive already resolved) into
//! `UI_ERROR_MESSAGE`, or `UI_INFO_MESSAGE` for the yellow arm; and the stance page
//! (`GetBonusBarOffset`) — our descriptor's
//! shapeshift-form byte indexed into `SpellShapeshiftForm.dbc`'s BonusActionBar column, wow-re
//! byte-verified, firing `UPDATE_BONUS_ACTIONBAR` on change.
@@ -38,6 +40,7 @@ use super::errors::{first_missing_totem, first_short_reagent, mount_result_key};
use super::weapon_icon::{auto_attack_icon, substitutes_weapon_icon};
use super::{
cast_fail, ui_error_text, CastErrors, MountErrors, PlayerActions, Spells, UiError, UiErrorKeys,
UiErrorTexts,
};
/// What an ITEM action shows when its icon cannot be resolved — the reference's own hardcoded
@@ -79,6 +82,7 @@ pub(super) fn feed_actions(
mut cast_errors: ResMut<CastErrors>,
mut mount_errors: ResMut<MountErrors>,
mut ui_error_keys: ResMut<UiErrorKeys>,
mut ui_error_texts: ResMut<UiErrorTexts>,
spells: Option<Res<Spells>>,
self_q: Query<&ObjectStore, With<SelfPlayer>>,
mut items: ResMut<Items>,
@@ -243,6 +247,17 @@ pub(super) fn feed_actions(
script.fire_event(event, vec![ScriptValue::Str(text)]);
}
// Already-resolved lines ([`UiErrorTexts`]) — the wire's own text, no key to look up. Same
// frame, same two events; the queued bool IS the reference's `0x4945b0` flag.
for (text, info) in ui_error_texts.0.drain(..) {
let event = if info {
"UI_INFO_MESSAGE"
} else {
"UI_ERROR_MESSAGE"
};
script.fire_event(event, vec![ScriptValue::Str(text)]);
}
// The ENGINE's own by-key refusals ride the very same line. `benilla_ui` is engine-free and
// cannot reach [`UiErrorKeys`], so a refusal raised inside the script crate (today: dropping a
// passive spell on the bar, `ERR_PASSIVE_ABILITY`) queues its key and we resolve it here —
+79 -1
View File
@@ -17,7 +17,7 @@ use benilla_ui::script::UiScript;
use bevy::prelude::*;
use super::feed::{feed_actions, MISSING_ITEM_ICON};
use super::{CastErrors, MountErrors, PlayerActions, UiErrorKeys};
use super::{CastErrors, MountErrors, PlayerActions, UiErrorKeys, UiErrorTexts};
use crate::entities::ItemDisplays;
use crate::items::{test_template, Items};
use crate::net::{ClientCommand, NetCommands};
@@ -61,6 +61,7 @@ fn app_with_food_on_the_bar() -> (App, crossbeam_channel::Receiver<ClientCommand
.init_resource::<CastErrors>()
.init_resource::<MountErrors>()
.init_resource::<UiErrorKeys>()
.init_resource::<UiErrorTexts>()
.insert_resource(ItemDisplays::icons_for_tests(
ItemDisplayCatalog::from_displays(displays),
))
@@ -308,6 +309,7 @@ fn a_macro_slot_shows_the_macros_own_icon_and_follows_an_edit() {
.init_resource::<CastErrors>()
.init_resource::<MountErrors>()
.init_resource::<UiErrorKeys>()
.init_resource::<UiErrorTexts>()
.insert_resource(NetCommands(tx));
let mut script = UiScript::new().unwrap();
script.set_macros(MacroState {
@@ -394,3 +396,79 @@ fn a_macro_slot_shows_the_macros_own_icon_and_follows_an_edit() {
app.update();
assert_eq!(events(&mut app), 1);
}
/// **The pre-resolved lines reach the frame, on the arm they asked for** — `UiErrorTexts` end to
/// end: the queue the net drain writes, through this feed, into the shipped `UIErrorsFrame`'s own
/// drawn quads.
///
/// This is the seam the GM-mode double line lived in. `SMSG_NOTIFICATION` used to be pushed into
/// the chat feed (a stand-in from before benilla had an errors frame), so vmangos answering
/// `.gm on` with both a `SendSysMessage` and a `SendNotification` printed the words twice. The
/// notice belongs here, red — and its `SMSG_AREA_TRIGGER_MESSAGE` sibling here, yellow.
#[test]
fn pre_resolved_lines_land_on_the_errors_frame_in_the_arms_colour() {
let (mut app, _rx) = app_with_food_on_the_bar();
{
let mut script = app.world_mut().non_send_resource_mut::<UiScript>();
script.set_screen_size(1024.0, 768.0);
for file in ["Fonts.xml", "ErrorsFrame.xml"] {
let text = std::fs::read_to_string(
std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
.join("assets/ui")
.join(file),
)
.unwrap();
let doc = benilla_ui::framexml::parse(&text).unwrap();
let report = benilla_ui::loader::load(&script, &doc, &|_| None);
assert!(report.errors.is_empty(), "{file}: {:?}", report.errors);
}
}
// What the net drain queues for one `.gm on` toggle, plus a refused portal.
let mut texts = app.world_mut().resource_mut::<UiErrorTexts>();
texts.error("GM mode is ON".to_string());
texts.info("You must be at least level 58 to enter.".to_string());
app.update();
assert!(
app.world().resource::<UiErrorTexts>().0.is_empty(),
"the feed drains the queue"
);
let mut script = app.world_mut().non_send_resource_mut::<UiScript>();
assert!(
script.errors().is_empty(),
"VM errors: {:?}",
script.errors()
);
script.resolve();
let mut drawn: Vec<(String, [f32; 4])> = script
.extract()
.iter()
.filter_map(|q| match &q.content {
benilla_ui::script::QuadContent::Text {
text: Some(t),
color: Some(c),
..
} if !t.is_empty() => Some((t.clone(), *c)),
_ => None,
})
.collect();
drawn.sort_by(|a, b| a.0.cmp(&b.0));
assert_eq!(
drawn,
[
// `AddMessage` byte-quantizes every channel (`ftol(v*255 + 0.5)`), so 0.1 draws as
// 26/255 — the same arithmetic `ui_script::errors_tests` pins.
(
"GM mode is ON".to_string(),
[1.0, 26.0 / 255.0, 26.0 / 255.0, 1.0]
),
(
"You must be at least level 58 to enter.".to_string(),
[1.0, 1.0, 0.0, 1.0],
),
],
"red UI_ERROR_MESSAGE for the notice, yellow UI_INFO_MESSAGE for the area trigger"
);
}
+2 -1
View File
@@ -66,7 +66,7 @@ pub(crate) struct CooldownEvents;
pub(crate) use cast_send::{CastCommit, CastLadder};
pub(crate) use errors::{
attack_actor_refusal, reagent_totem_refusal, ui_error_text, CastErrors, CastFail, MountErrors,
MsgSurface, UiError, UiErrorKeys,
MsgSurface, UiError, UiErrorKeys, UiErrorTexts,
};
// `pub(crate)`: the target chain registers the cursor pre-empt + the click commit, and the
// spellbook/stance/craft drains thread the mode through the one cast-send path (decision 0792).
@@ -290,6 +290,7 @@ impl Plugin for UiActionPlugin {
.init_resource::<CastErrors>()
.init_resource::<MountErrors>()
.init_resource::<UiErrorKeys>()
.init_resource::<UiErrorTexts>()
.init_resource::<crate::cooldowns::Cooldowns>()
.init_resource::<AutoRepeatActive>()
.init_resource::<ChainCasts>()
+48 -30
View File
@@ -294,26 +294,33 @@ fn drain_gm_ticket(
}
}
/// One drained write → its packet, or `None` for a category that cannot be filed.
/// The category a `NewGMTicket`/`UpdateGMTicket` argument may put on the wire, or `None`.
///
/// **0 is legal and means "uncategorised"** (decision 1687). Our Help window has no category picker
/// — one click goes from Home to the text box — so it sends 0 deliberately. vmangos accepts it: its
/// whole validation is `if (packet.ticketType >= GMTICKET_MAX) return;` with `GMTICKET_MAX == 11`
/// (`GMTicketHandler.cpp:112`; the enum starts at `GMTICKET_STUCK = 1`, `SharedDefines.h:1776`),
/// and `GmTicket::GetTicketCategoryName` falls through its 1..10 switch to return **"Unknown"**
/// (`GMTicketMgr.cpp:205-232`). So the GM's notification and queue read "Unknown" — the honest
/// label, and far better than picking one of the ten real headings and mislabelling every ticket
/// as an Item or a Stuck.
///
/// **Above 10 is still refused**, and that is not tidiness: the server drops those *silently*, with
/// no response packet at all, so a bad argument would be indistinguishable from a filed ticket. The
/// range test is deliberately on the **`u32`, before the narrowing** — a category of 256 truncated
/// into a `u8` lands on 0 and would sail through as "uncategorised" when the caller meant something
/// else entirely.
fn category_for_wire(category: u32) -> Option<u8> {
(category <= u32::from(GM_TICKET_CATEGORY_MAX)).then_some(category as u8)
}
/// One drained write → its packet, or `None` when [`category_for_wire`] refuses the category.
///
/// Split out so the verb choice is testable without a world: **the window's own `hasTicket` flag
/// picks the opcode**, and we must not re-derive it from whether we believe a ticket exists — our
/// belief can be stale by up to the 10-minute poll.
///
/// **The range check is not defensive tidiness — it is the only thing between a bad argument and a
/// silently misfiled ticket.** The category rides the wire as one byte, and vmangos's whole
/// validation is `if (packet.ticketType >= GMTICKET_MAX) return;` with `GMTICKET_MAX == 11`
/// (`GMTicketHandler.cpp:112`; the enum starts at `GMTICKET_STUCK = 1`, `SharedDefines.h:1776`).
/// So 11 and up are dropped by the server — but **0 is not**: it passes that check and is stored as
/// `ticket_type = 0`, a heading `GetTicketCategoryName` cannot name. Truncating a nonsense argument
/// into a `u8` lands exactly there, and a category of 256 truncates to 0 as well. No shipped UI
/// path can produce one (the buttons carry `GMTicketCategory.dbc` ids, 1..10), so this guards a
/// third-party addon's argument — and it refuses loudly rather than filing a ticket no GM can
/// categorise.
fn client_command_for(write: GmTicketWrite, map: u32, pos: [f32; 3]) -> Option<ClientCommand> {
let category = u8::try_from(write.category)
.ok()
.filter(|c| (1..=GM_TICKET_CATEGORY_MAX).contains(c))?;
let category = category_for_wire(write.category)?;
Some(if write.is_new {
ClientCommand::GmTicketCreate {
category,
@@ -585,27 +592,38 @@ mod tests {
));
}
/// A category the server cannot file is refused here rather than sent.
/// **0 rides the wire; anything above 10 does not** (decision 1687).
///
/// **0 is the one that matters**, and it is the one a naive truncation produces: vmangos only
/// rejects `>= 11`, so a zero passes its check and is stored as an unnameable
/// `ticket_type = 0` — a ticket filed under nothing, with no error anywhere. 256 truncates to
/// 0 in a `u8` and would land in the same place.
/// 0 is what our own window sends, because it has no category picker — vmangos renders it
/// "Unknown" rather than refusing it. Above 10 the server drops the packet *silently*, so
/// letting one through would be indistinguishable from a filed ticket.
///
/// The `256` case is the one worth having: it truncates to 0 in a `u8`, so a check written
/// after the narrowing would accept it as "uncategorised" when the caller meant category 256.
#[test]
fn a_category_the_server_cannot_file_is_refused_rather_than_sent() {
fn zero_is_uncategorised_and_anything_above_ten_is_refused() {
assert_eq!(category_for_wire(0), Some(0), "the uncategorised ticket");
for good in 1..=10 {
assert_eq!(category_for_wire(good), Some(good as u8));
}
for bad in [11, 255, 256, 99_999] {
assert_eq!(
category_for_wire(bad),
None,
"category {bad} must not be sent"
);
}
// And the whole path, not just the helper: a refused category produces no packet.
let write = |category| GmTicketWrite {
category,
text: "x".into(),
is_new: true,
};
for bad in [0, 11, 256, 99_999] {
assert!(
client_command_for(write(bad), 1, [0.0; 3]).is_none(),
"category {bad} must not reach the wire"
);
}
for good in 1..=10 {
assert!(client_command_for(write(good), 1, [0.0; 3]).is_some());
}
assert!(client_command_for(write(256), 1, [0.0; 3]).is_none());
assert!(matches!(
client_command_for(write(0), 1, [0.0; 3]),
Some(ClientCommand::GmTicketCreate { category: 0, .. })
));
}
}
+1 -6
View File
@@ -748,7 +748,6 @@ pub(crate) fn feed_containers(
mut equip_errors: ResMut<EquipErrors>,
// Reason 16's `%s` source (decision 0916); absent = every 16 keeps the generic line.
bag_families: Option<Res<crate::ui_items::ItemBagFamilies>>,
mut error_lines: ResMut<crate::ui_items::UiErrorLines>,
mut pending: ResMut<PendingItemOps>,
mut lock_cleared: ResMut<LockClearedByFailure>,
mut names: ResMut<crate::names::NameCache>,
@@ -798,7 +797,7 @@ pub(crate) fn feed_containers(
|| catalogs.1.as_ref().is_some_and(|r| r.is_changed());
let spells_changed = spells.as_ref().is_some_and(|r| r.is_changed());
let families_changed = bag_families.as_ref().is_some_and(|r| r.is_changed());
let errors_held = !equip_errors.0.is_empty() || !error_lines.0.is_empty();
let errors_held = !equip_errors.0.is_empty();
let locks_held = !lock_cleared.0.is_empty() || !pending.is_empty();
gate::trace(
"feed_containers",
@@ -913,10 +912,6 @@ pub(crate) fn feed_containers(
};
script.fire_event("UI_ERROR_MESSAGE", vec![ScriptValue::Str(text)]);
}
for line in error_lines.0.drain(..) {
script.fire_event("UI_ERROR_MESSAGE", vec![ScriptValue::Str(line)]);
}
// The pending-lock resolving clear (decision 0216 §4 / 0218 §3 "the field-update watcher"):
// walk the SAME guids the slot loop below is about to read and release any op whose slots have
// moved on. Folded in with the failure-driven clears `net/apply/loot.rs::inventory_failure`
-7
View File
@@ -185,12 +185,6 @@ pub(crate) struct EquipError {
#[derive(Resource, Default)]
pub(crate) struct EquipErrors(pub Vec<EquipError>);
/// Pre-formatted red error lines from the net drain (`UI_ERROR_MESSAGE` verbatim text — the
/// death durability notice today; anything whose message isn't a code map). Drained beside
/// [`EquipErrors`] by the container feed.
#[derive(bevy::prelude::Resource, Default)]
pub(crate) struct UiErrorLines(pub Vec<String>);
/// The item guid in a Lua-space bag slot, read off the player descriptor (backpack), the bag
/// object's own slot array, or — [`EQUIPMENT_BAG`], decision 0208 phase 1b — the player
/// descriptor's own `INV_SLOT` array directly (`slot0` is already the wire `EQUIPMENT_SLOT_*`
@@ -1029,7 +1023,6 @@ impl Plugin for UiItemsPlugin {
// The icon source — `ItemDisplayInfo.dbc` — is the `ItemDisplays` resource the equipment
// renderer already loads (one parse serves the world and the bags).
app.init_resource::<EquipErrors>()
.init_resource::<UiErrorLines>()
.init_resource::<PendingItemOps>()
.init_resource::<LockClearedByFailure>()
// AFTER the chain opens — a bare Startup slot raced AssetSet::Open and, when it
@@ -86,36 +86,29 @@ fn open_ticket_args(
]
}
/// **The whole point of the feature, end to end.** The category id travels
/// `GMTicketCategory.dbc` → `GetGMTicketCategories()` → a `HelpFrameButton*` → the page route →
/// `HelpFrameOpenTicket.ticketType` → `NewGMTicket`'s wire value. Break any link and every ticket
/// files under the wrong heading, silently — nothing on screen looks wrong.
/// **The whole point of decision 1687: one click from Home to the text box.**
///
/// The category list and the per-category blurb page are gone, so this walks what is left — open
/// the window, press the one button, type, submit — and asserts the ticket goes out **uncategorised
/// (0)**. That last part is the assertion that actually pins 1687: 0 is what makes a GM's queue read
/// "Unknown" instead of a heading the player never chose.
#[test]
fn clicking_a_category_files_a_ticket_under_that_categorys_dbc_id() {
fn one_click_from_home_files_an_uncategorised_ticket() {
let mut s = setup();
s.run("ShowUIPanel(HelpFrame) HelpFrame_ShowFrame(\"GMHome\")")
.unwrap();
// Opening the window is itself traffic: its OnShow calls `GetGMStatus()`. Drained here so the
// assertion below is about the CLICK and nothing else — the intents are one ordered queue, so
// setup traffic and the thing under test share it (decision 1673).
let _ = s.take_gm_ticket_intents();
s.run("ToggleHelpFrame()").unwrap();
let _ = s.take_gm_ticket_intents(); // the OnShow GetGMStatus
// Row 4 is Item, DBC id 4 — the row's LABEL and its stored id must be the same pair.
assert_eq!(
s.eval::<String>("return HelpFrameButton4Text:GetText()")
assert!(
s.eval::<bool>("return HelpFrameHome:IsVisible()").unwrap(),
"the window opens on Home"
);
s.run("HelpFrameHomeIssues:Click()").unwrap();
assert!(
s.eval::<bool>("return HelpFrameOpenTicket:IsVisible()")
.unwrap(),
"Item"
);
assert_eq!(
s.eval::<i64>("return HelpFrameButton4.ticketType").unwrap(),
4,
"the button carries the DBC id, not its row number"
"one click lands on the editor — no category page in between"
);
// Click it, take the page's action button through to the editor, and submit.
s.run("HelpFrameButton4:Click()").unwrap();
s.run("HelpFrameGeneralButton_OnClick(HelpFrameGeneralButton)")
.unwrap();
s.run("HelpFrameOpenTicketText:SetText(\"My sword vanished.\")")
.unwrap();
s.run("HelpFrameOpenTicketSubmit_OnClick()").unwrap();
@@ -123,49 +116,71 @@ fn clicking_a_category_files_a_ticket_under_that_categorys_dbc_id() {
assert_eq!(
s.take_gm_ticket_intents(),
vec![GmTicketIntent::Write(GmTicketWrite {
category: 4,
category: 0,
text: "My sword vanished.".into(),
is_new: true,
})],
"the clicked category's DBC id is what goes on the wire"
"a create files under 0 — \"Unknown\" in the GM's queue"
);
}
/// The ten shipped rows paint in DBC order, and row 2 routes to its OWN page rather than the
/// shared one — the single category in the table with a bespoke frame.
/// **Auto-Unstuck sits on Home now**, beside the ticket button — it used to be the Stuck category's
/// action button, three pages down, which is nowhere.
///
/// Clicked through the real button rather than by calling the handler, because the button's
/// existence on Home *is* the change.
#[test]
fn the_ten_categories_paint_in_dbc_order_and_harassment_has_its_own_page() {
let s = setup();
s.run("ShowUIPanel(HelpFrame) HelpFrame_ShowFrame(\"GMHome\")")
.unwrap();
assert_eq!(
s.eval::<String>("return HelpFrameButton1Text:GetText()")
.unwrap(),
"Stuck"
fn auto_unstuck_is_a_button_on_home() {
let mut s = setup();
s.run("ToggleHelpFrame()").unwrap();
s.run("BenillaHelpFrameHomeUnstick:Click()").unwrap();
assert_eq!(s.take_stuck_casts(), 1);
assert!(
!s.eval::<bool>("return HelpFrame:IsVisible()").unwrap(),
"and it closes the window"
);
assert_eq!(
s.eval::<String>("return HelpFrameButton10Text:GetText()")
.unwrap(),
"Character",
"all ten rows fit without scrolling"
}
/// **The two Home buttons do not overlap, and both fit their labels.**
///
/// The layout is ours (the reference had one button here), and its whole trick is that the left
/// button hangs by its TOPRIGHT and the right one off that fixed edge, so fitting them to their
/// text grows them *away* from the gutter. Nothing else in the suite would notice them landing on
/// top of each other: a script error inside `OnShow` is swallowed into the VM's error record, so a
/// fit that never ran still leaves every other test green.
#[test]
fn the_two_home_buttons_sit_side_by_side_without_overlapping() {
let mut s = setup();
s.run("ToggleHelpFrame()").unwrap();
s.resolve();
let edge = |frame: &str, edge: &str| {
s.eval::<f32>(&format!("return {frame}:Get{edge}()"))
.unwrap_or_else(|e| panic!("{frame}:Get{edge}(): {e}"))
};
let (ticket_l, ticket_r) = (
edge("HelpFrameHomeIssues", "Left"),
edge("HelpFrameHomeIssues", "Right"),
);
let (stuck_l, stuck_r) = (
edge("BenillaHelpFrameHomeUnstick", "Left"),
edge("BenillaHelpFrameHomeUnstick", "Right"),
);
s.run("HelpFrameButton2:Click()").unwrap();
assert!(
s.eval::<bool>("return HelpFrameHarassment:IsVisible()")
.unwrap(),
"category 2 routes to HelpFrameHarassment"
);
s.run("HelpFrameButton3:Click()").unwrap();
assert!(
s.eval::<bool>("return HelpFrameGeneral:IsVisible()")
.unwrap(),
"category 3 routes to the shared general page"
ticket_r <= stuck_l,
"the buttons overlap: ticket ends at {ticket_r}, unstick starts at {stuck_l}"
);
assert!(
!s.eval::<bool>("return HelpFrameHarassment:IsVisible()")
.unwrap(),
"and the previous page is hidden — one page at a time"
ticket_r > ticket_l && stuck_r > stuck_l,
"both buttons must have width: {ticket_l}..{ticket_r}, {stuck_l}..{stuck_r}"
);
// The fit ran: an unfitted button keeps its authored 250px, and "Auto-Unstuck" is nowhere near
// that wide. This is what catches an OnShow that silently errored out.
assert!(
stuck_r - stuck_l < 200.0,
"the unstick button was never fitted to its label (width {})",
stuck_r - stuck_l
);
}
@@ -216,35 +231,48 @@ fn an_open_ticket_turns_the_form_into_an_editor_and_a_zero_turns_it_back() {
);
}
/// The Submit button picks its verb from the window's own `hasTicket`, which is what the app must
/// not second-guess: an edit after an answer is an UPDATE, a submit before one is a CREATE.
/// The Submit button picks its verb from the window's own `hasTicket` — an edit after an answer is
/// an UPDATE, a submit before one is a CREATE — **and the two legs send different categories**.
///
/// That asymmetry is decision 1687's real content, and it is the half a suite would otherwise miss.
/// A create has nothing to go on and sends 0. An edit sends the server's own category straight
/// back, because `HandleGMTicketUpdateTextOpcode` overwrites the field with whatever arrives: send
/// 0 there and a GM who re-filed the ticket onto a real heading watches it drop to "Unknown" the
/// moment the player fixes a typo. Nothing on screen would ever show that.
#[test]
fn submit_sends_create_before_an_answer_and_update_after_one() {
fn a_create_files_under_zero_and_an_edit_gives_the_server_its_category_back() {
let mut s = setup();
s.run("ShowUIPanel(HelpFrame) HelpFrame_ShowFrame(\"OpenTicket\")")
.unwrap();
let _ = s.take_gm_ticket_intents(); // the OnShow GetGMStatus — see the category test
let _ = s.take_gm_ticket_intents(); // the OnShow GetGMStatus
// A stale `ticketType` must NOT leak into a create — the window has no picker, so a create is
// uncategorised no matter what is lying around on the frame.
s.run("HelpFrameOpenTicket.ticketType = 3 HelpFrameOpenTicketText:SetText(\"a\")")
.unwrap();
s.run("HelpFrameOpenTicketSubmit_OnClick()").unwrap();
assert!(
matches!(
s.take_gm_ticket_intents().as_slice(),
[GmTicketIntent::Write(w)] if w.is_new
),
"no ticket known yet — this is a create"
);
let intents = s.take_gm_ticket_intents();
let [GmTicketIntent::Write(create)] = intents.as_slice() else {
panic!("expected exactly one write, got {intents:?}");
};
assert!(create.is_new, "no ticket known yet — this is a create");
assert_eq!(create.category, 0, "a create is uncategorised");
s.fire_event("UPDATE_TICKET", open_ticket_args(3, "a", 0.1, 0.2, 0.01));
// The server answers: this ticket is filed under 4 (Item) — a GM moved it there.
s.fire_event("UPDATE_TICKET", open_ticket_args(4, "a", 0.1, 0.2, 0.01));
s.run("HelpFrameOpenTicketText:SetText(\"a, still\")")
.unwrap();
s.run("HelpFrameOpenTicketSubmit_OnClick()").unwrap();
let intents = s.take_gm_ticket_intents();
let [GmTicketIntent::Write(write)] = intents.as_slice() else {
let [GmTicketIntent::Write(edit)] = intents.as_slice() else {
panic!("expected exactly one write, got {intents:?}");
};
assert!(!write.is_new, "a ticket is known — this is an update");
assert_eq!(write.text, "a, still");
assert!(!edit.is_new, "a ticket is known — this is an update");
assert_eq!(edit.text, "a, still");
assert_eq!(
edit.category, 4,
"the edit must hand the GM's own category back, not overwrite it with 0"
);
}
/// **The queue gate.** `UPDATE_GM_STATUS(0)` takes the petition queue down, and asking for the
@@ -364,18 +392,6 @@ fn toggling_the_window_opens_it_and_asks_for_the_queue_status() {
assert!(!s.eval::<bool>("return HelpFrame:IsVisible()").unwrap());
}
/// The Auto-Unstuck button casts the Stuck spell and closes the window. It is the one control in
/// this window that does something other than talk about tickets.
#[test]
fn auto_unstuck_casts_and_closes() {
let mut s = setup();
s.run("ShowUIPanel(HelpFrame) HelpFrame_ShowFrame(1)")
.unwrap();
s.run("HelpFrameUnstick_OnClick()").unwrap();
assert_eq!(s.take_stuck_casts(), 1);
assert!(!s.eval::<bool>("return HelpFrame:IsVisible()").unwrap());
}
/// **The retail-only text is gone from the Home page** (director's call, 2026-08-29).
///
/// The page is entirely GlobalStrings off the player's own chain, so three of its strings still
@@ -432,9 +448,11 @@ fn the_home_page_advertises_no_dead_retail_links() {
/// **The geometry oracle** (decision 0675): every element the transcription shares with the
/// reference file carries the reference's own `<AbsDimension>` numbers.
///
/// **Verified to fail**, as 0675 requires: nudging `TicketStatusFrame`'s width 208 → 209 makes it
/// report `TicketStatusFrame: ours [(209.0, 52.0), …] != ref [(208.0, 52.0), …]` and fail. It is a
/// guard, not a comfort.
/// **Verified to fail**, as 0675 requires, and re-verified after 1687 lowered `min_compared` from
/// 60 to 28: nudging `TicketStatusFrame`'s width 208 → 209 reports
/// `TicketStatusFrame: ours [(209.0, 52.0), …] != ref [(208.0, 52.0), …]` and fails. Re-checking
/// that after shrinking the floor is the point — a guard whose floor drops below what it actually
/// compares stops guarding and never says so.
#[test]
fn the_windows_geometry_matches_the_reference_file() {
let Some(reference) = super::framexml_diff::reference("HelpFrame.xml") else {
@@ -443,11 +461,16 @@ fn the_windows_geometry_matches_the_reference_file() {
/// Deliberate deviations, by REFERENCE name. Each earns its reason here; a tolerance would
/// let a real difference hide, so this is a list and stays a list.
const EXPECTED: &[&str] = &[
// The GM category list is the house FAUX scroll kit, not the reference's ScrollFrame, and
// the kit's shared trough replaces the three loose bar textures the reference hangs beside
// it (HelpFrame.xml's header, divergence 1). Same for the ticket editor's own trough.
"HelpFrameGMScrollFrame",
// The ticket editor's scroll frame carries the house kit's shared trough in place of the
// three loose bar textures the reference hangs beside it (HelpFrame.xml's header).
"HelpFrameOpenTicketScrollFrame",
// Home's button row is ours: the reference has one button centred under the text, we have
// two, so the left one hangs by its TOPRIGHT at -6 rather than centred at 0. Its SIZE still
// matches, which is why only the offset differs.
"HelpFrameHomeIssues",
];
super::framexml_diff::assert_geometry_matches("HelpFrame.xml", &reference, EXPECTED, 60);
// 1687 deleted the category list and the per-category page, and roughly seventy named elements
// went with them — hence 28 rather than 60. The floor still has to bite: it is what stops this
// guard quietly comparing nothing after the next window-shrinking change.
super::framexml_diff::assert_geometry_matches("HelpFrame.xml", &reference, EXPECTED, 28);
}
@@ -622,6 +622,114 @@ fn every_shipped_text_attribute_answers_against_the_real_global_strings() {
assert!(keys >= 23, "only {keys} key-shaped text= values swept");
}
/// **No shipped script hands a GlobalStrings KEY to a text sink as if it were the string.**
///
/// The defect this pins, twice now: `TargetFrame_CheckDead` did `deadText:SetText("DEAD")`, and
/// `DEAD` is not the word — it is `GlobalStrings.lua` l.898's key for `"Dead"`. So the target
/// frame wore **DEAD** in caps over every corpse, in every locale, until the director looked at
/// an Onyxia and said so. The same class shipped once before with `"CREATE_MACROS"` written
/// across the macro window's title bar (0983 → 0991).
///
/// The XML half of this class is already guarded twice: `loader::resolve_text` warns at load
/// whenever a **key-shaped** `text=` attribute resolves to nothing, and the sweep directly above
/// answers every one of them against the real `GlobalStrings.lua`. The **Lua** half had no guard
/// at all — `SetText` takes a string and cannot tell a key from a word — and that is the
/// half both escapes came through. This is that missing guard, as a shape test: a literal that is
/// all `[A-Z0-9_]` with an uppercase in it is `is_global_string_key`'s own predicate, and nothing
/// we would ever want a player to read.
///
/// Swept over the whole directory rather than the file that broke, because the next one will be a
/// different window. If a genuinely uppercase word ever needs to reach a sink, it goes through the
/// house's `gs(key, fallback)` helper (AuctionFrame/HelpFrame/MailFrame) or the XML `text=`
/// attribute — both resolve the global first, and both are what "faithful" means here.
#[test]
fn no_shipped_script_sets_a_global_string_key_as_display_text() {
/// `loader::is_global_string_key`'s predicate, restated here because it is private: at least
/// two characters, uppercase/digit/underscore only, and at least one uppercase.
fn key_shaped(s: &str) -> bool {
s.len() >= 2
&& s.chars().any(|c| c.is_ascii_uppercase())
&& s.chars()
.all(|c| c.is_ascii_uppercase() || c.is_ascii_digit() || c == '_')
}
// The literal after a text sink's open paren, if the call opens on this line at all. Prose is
// the noise here — the house documents its own history in comments, this very record included
// — so XML comment blocks are tracked across lines and Lua `--` lines are dropped.
fn sink_literals(line: &str) -> Vec<&str> {
let mut out = Vec::new();
for sink in [":SetText(", ":SetFormattedText(", ":SetButtonText("] {
let mut rest = line;
while let Some(at) = rest.find(sink) {
rest = &rest[at + sink.len()..];
let arg = rest.trim_start();
if let Some(body) = arg.strip_prefix('"') {
if let Some(end) = body.find('"') {
out.push(&body[..end]);
}
}
}
}
out
}
let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("assets/ui");
let mut offenders: Vec<String> = Vec::new();
let mut swept = 0;
for entry in std::fs::read_dir(&dir).expect("assets/ui").flatten() {
let path = entry.path();
if path.extension().is_none_or(|e| e != "xml") {
continue;
}
let text = std::fs::read_to_string(&path).expect("read");
let mut in_comment = false;
for (n, raw) in text.lines().enumerate() {
// Strip whatever of this line sits inside an XML comment, carrying the state over.
let mut code = String::new();
let mut cur = raw;
loop {
if in_comment {
match cur.find("-->") {
Some(at) => {
in_comment = false;
cur = &cur[at + 3..];
}
None => break,
}
} else {
match cur.find("<!--") {
Some(at) => {
code.push_str(&cur[..at]);
in_comment = true;
cur = &cur[at + 4..];
}
None => {
code.push_str(cur);
break;
}
}
}
}
if code.trim_start().starts_with("--") {
continue; // a Lua comment line
}
for lit in sink_literals(&code) {
if key_shaped(lit) {
offenders.push(format!(
"{}:{}: SetText(\"{lit}\") — a GlobalStrings KEY, not the word",
path.file_name().unwrap().to_string_lossy(),
n + 1
));
}
}
}
swept += 1;
}
assert!(offenders.is_empty(), "{}", offenders.join("\n"));
// The sweep must never pass by finding nothing to sweep.
assert!(swept >= 40, "only {swept} xml files swept — sweep broke");
}
/// **The `$parentTextureFrame` idiom's contract, over the whole shipped UI**: a frame whose art is
/// meant to cap a unit frame must sit at a strictly HIGHER frame level than that frame's status
/// bars — because frame level is the only key term above the draw layer, and the layer would
@@ -21,6 +21,12 @@ fn load_xml(s: &UiScript, file: &str) {
/// The unit frames' production load prefix (ui_script/mod.rs order): fonts + UIParent +
/// tooltip, then the dropdown kit + unit popups the frames' DropDown children initialize into.
fn load_unit_frames(s: &UiScript) {
// The app runs the real `Interface\FrameXML\GlobalStrings.lua` off the player's chain
// BEFORE any XML (`load_global_strings`); this stands in for it, with the one key these
// frames resolve at load time — `DEAD` l.898, which both dead-text FontStrings name in
// their `text=` attribute. Without it the loader's literal fallback would leave the KEY on
// screen, which is the bug this file guards below.
s.run(r#"DEAD = "Dead""#).unwrap();
load_xml(s, "Fonts.xml");
load_xml(s, "UIParent.xml");
// The bars' numerals machinery (decision 1082), which the manifest loads immediately ahead of
@@ -134,7 +140,7 @@ fn shipped_unit_frames_drive_end_to_end() {
and not TargetFramePowerBar:IsVisible()
and TargetFrameTextureFrameNameText:GetText() == "Young Wolf"
and TargetFrameTextureFrameLevelText:GetText() == "3"
and TargetFrameTextureFrameDeadText:GetText() == "" -- living target: no DEAD text
and not TargetFrameTextureFrameDeadText:IsShown() -- living target: no dead word
"#,
)
.unwrap();
@@ -1210,6 +1216,7 @@ fn a_feigning_target_paints_empty_bars_and_the_dead_text() {
let hunter = |health: u32, power: u32, dead: bool| {
Some(UnitState {
exists: true,
is_connected: true, // CheckDead's second term — a feign is not a link-drop
name: Some("Nazriel".into()),
health,
max_health: 1500,
@@ -1230,7 +1237,7 @@ fn a_feigning_target_paints_empty_bars_and_the_dead_text() {
r#"
local hb, pb = TargetFrameHealthBar, TargetFramePowerBar
return hb:GetValue() == 1200 and pb:GetValue() == 300
and TargetFrameTextureFrameDeadText:GetText() == ""
and not TargetFrameTextureFrameDeadText:IsShown()
"#,
)
.unwrap();
@@ -1254,11 +1261,17 @@ fn a_feigning_target_paints_empty_bars_and_the_dead_text() {
.unwrap(),
"the mana bar empties, it does not disappear — UnitManaMax 0x5177e0 is ungated"
);
assert!(
s.eval::<bool>("return TargetFrameTextureFrameDeadText:IsShown() and true or false")
.unwrap(),
"TargetFrame_CheckDead's UnitHealth(unit) <= 0 test, tripped by the flag"
);
assert_eq!(
s.eval::<String>("return TargetFrameTextureFrameDeadText:GetText()")
.unwrap(),
"DEAD",
"TargetFrame_CheckDead's UnitHealth(unit) <= 0 test, tripped by the flag"
"Dead",
"the WORD is the GlobalString `DEAD` (l.898), never the key: a literal \"DEAD\" here \
is the caps bug the director caught on Onyxia"
);
assert!(
s.eval::<bool>(r#"return UnitIsDead("target")"#).unwrap(),
@@ -1273,7 +1286,7 @@ fn a_feigning_target_paints_empty_bars_and_the_dead_text() {
r#"
return TargetFrameHealthBar:GetValue() == 1200
and TargetFramePowerBar:GetValue() == 300
and TargetFrameTextureFrameDeadText:GetText() == ""
and not TargetFrameTextureFrameDeadText:IsShown()
and not UnitIsDead("target")
"#,
)
@@ -9,18 +9,24 @@
//! *dropdown*, which has no frame in 1.12 and whose `OnShow` call is commented out. The category
//! list is DBC data, and it lines up with `GENERAL_HELPFRAME`'s keys row for row:
//!
//! | id | name | the frame `HELPFRAME_FRAMES[id]` sends it to |
//! |---|---|---|
//! | 1 | Stuck | `HelpFrameGeneral`, configured by `GENERAL_HELPFRAME[1]` (`STUCK_OPTION`) |
//! | 2 | Behavior/Harassment | `HelpFrameHarassment` |
//! | 3 | Guild | `HelpFrameGeneral` (`HELPFRAME_GUILD_TITLE`) |
//! | 4 | Item | `HelpFrameGeneral` (`HELPFRAME_ITEM_TITLE`) |
//! | 5 | Environmental | `HelpFrameGeneral` (`HELPFRAME_ENVIRONMENTAL_TITLE`) |
//! | 6 | Non-Quest/Creep | `HelpFrameGeneral` (`HELPFRAME_NONQUEST_TITLE`) |
//! | 7 | Quest/Quest NPC | `HelpFrameGeneral` (`HELPFRAME_QUEST_TITLE`) |
//! | 8 | Technical | `HelpFrameGeneral` (`HELPFRAME_TECHNICAL_TITLE`) |
//! | 9 | Account/Billing | `HelpFrameGeneral` (`HELPFRAME_ACCOUNT_TITLE`) |
//! | 10 | Character | `HelpFrameGeneral` (`HELPFRAME_CHARACTER_TITLE`) |
//! | id | name |
//! |---|---|
//! | 1 | Stuck |
//! | 2 | Behavior/Harassment |
//! | 3 | Guild |
//! | 4 | Item |
//! | 5 | Environmental |
//! | 6 | Non-Quest/Creep |
//! | 7 | Quest/Quest NPC |
//! | 8 | Technical |
//! | 9 | Account/Billing |
//! | 10 | Character |
//!
//! **benilla's own Help window no longer shows this list** (decision 1687): it goes straight from
//! Home to the ticket box and files under 0, "uncategorised". The catalog still ships, because
//! `GetGMTicketCategories()` is a real Era binding a third-party addon may call and because these
//! ids are still what the *server* names a ticket by — an existing ticket's category arrives on
//! `UPDATE_TICKET` and is echoed back on an edit.
//!
//! **The id is the wire value**, not just a list index: the clicked button stores it as
//! `HelpFrameOpenTicket.ticketType`, and that is what `NewGMTicket(category, text)` puts in
+3
View File
@@ -862,6 +862,9 @@ pub enum SessionEvent {
services: Vec<TrainerSpell>,
greeting: String,
},
/// Forget a player's cached name (`SMSG_INVALIDATE_PLAYER`, decision 1689) — the name cache's
/// only explicit eviction for a player, and the safety valve a *persisted* cache needs.
InvalidatePlayer { guid: u64 },
/// A stable master's pet list (`MSG_LIST_STABLED_PETS`, decision 1676) — the current pet and
/// the stabled ones, each already carrying its rebased client slot (`0` = current), plus how
/// many stable slots the player has **bought**. Arrives unprompted when the gossip stable
@@ -364,6 +364,7 @@ pub fn decode(packet: ServerPacket) -> Vec<SessionEvent> {
services,
greeting: title,
}],
ServerPacket::InvalidatePlayer { guid } => vec![SessionEvent::InvalidatePlayer { guid }],
ServerPacket::ListStabledPets {
npc,
num_stable_slots,
@@ -133,9 +133,10 @@ pub fn gm_ticket_create(category: u8, map: u32, pos: [f32; 3], text: &str) -> Ve
/// edit re-files the ticket under whatever category the byte carries, and getting this field wrong
/// silently moves a GM's ticket to a different queue rather than merely being ignored.
///
/// The shipped Help window always passes the ticket's existing `HelpFrameOpenTicket.ticketType`
/// here, so in practice the value is unchanged by an edit — but that is the window's discipline,
/// not the server's.
/// benilla's own Help window has no category picker (decision 1687), so it passes back the
/// category the server last reported for the ticket — precisely so an edit does not overwrite a
/// re-filing a GM made. That is the window's discipline, not the server's: the opcode itself will
/// happily move a ticket to whatever byte arrives.
pub fn gm_ticket_updatetext(category: u8, text: &str) -> Vec<u8> {
let mut out = Vec::with_capacity(1 + text.len() + 1);
out.push(category);
@@ -787,6 +787,13 @@ pub const CMSG_AUTOBANK_ITEM: u16 = 0x0283; // 643
// else, so a successful one is a cue to re-ask the list. Bodies in [`super::stable`]
// (decision 1676). `CMSG_STABLE_REVIVE_PET` (0x0274, 628) is deliberately absent: vmangos's handler
// is an empty no-op and whether the 5875 client ever sends it is an open RE question.
/// Forget a cached player name (VERIFIED vmangos `Opcodes_1_12_1.h`: 796) — body `{u64 guid}`.
/// The client's name cache has **no TTL**: eviction is explicit, and this is the one packet that
/// does it for a player (wow-re `system/dbcache/dbcache.md` Contracts, remove-by-key `0x556ff0`).
/// Decision 1689. vmangos never sends it, so this is the mechanism present and correct rather
/// than a path our own server exercises.
pub const SMSG_INVALIDATE_PLAYER: u16 = 0x031C; // 796
pub const MSG_LIST_STABLED_PETS: u16 = 0x026F; // 623
pub const CMSG_STABLE_PET: u16 = 0x0270; // 624
pub const CMSG_UNSTABLE_PET: u16 = 0x0271; // 625
@@ -857,6 +857,12 @@ pub enum ServerPacket {
num_stable_slots: u8,
pets: Vec<StabledPet>,
},
/// `SMSG_INVALIDATE_PLAYER` — drop this guid from the player-name cache (decision 1689). The
/// cache ages nothing out, so this packet is the only thing that unsticks a name short of a
/// reconnect; it matters more to benilla than to the reference only because ours now persists.
InvalidatePlayer {
guid: u64,
},
/// `SMSG_STABLE_RESULT` — the whole answer to a stable/unstable/swap/buy-slot ask (vmangos
/// `StableResult::AppendBodyTo`). `result` is a [`super::stable::stable_result`] code; success
/// codes carry no updated list, so repainting the window takes a fresh
@@ -1481,6 +1487,7 @@ impl ServerPacket {
ServerPacket::TrainerBuyFailed { .. } => "SMSG_TRAINER_BUY_FAILED".into(),
ServerPacket::ListStabledPets { .. } => "MSG_LIST_STABLED_PETS".into(),
ServerPacket::StableResult { .. } => "SMSG_STABLE_RESULT".into(),
ServerPacket::InvalidatePlayer { .. } => "SMSG_INVALIDATE_PLAYER".into(),
ServerPacket::LootResponse { .. } => "SMSG_LOOT_RESPONSE".into(),
ServerPacket::LootError { .. } => "SMSG_LOOT_RESPONSE (error)".into(),
ServerPacket::LootReleaseResponse { .. } => "SMSG_LOOT_RELEASE_RESPONSE".into(),
@@ -740,6 +740,9 @@ pub fn parse_server(opcode: u16, body: &[u8]) -> io::Result<ServerPacket> {
pets,
}
}
opcode::SMSG_INVALIDATE_PLAYER => ServerPacket::InvalidatePlayer {
guid: read_u64_le(&mut r)?,
},
opcode::SMSG_STABLE_RESULT => {
let result = stable::read_stable_result(&mut r)?;
ServerPacket::StableResult { result }