diff --git a/crates/benilla-app/src/capture/probe_bg.rs b/crates/benilla-app/src/capture/probe_bg.rs index 9e6165b6..1757b83a 100644 --- a/crates/benilla-app/src/capture/probe_bg.rs +++ b/crates/benilla-app/src/capture/probe_bg.rs @@ -27,20 +27,25 @@ //! then starts a battleground as soon as *either* side has one player //! (`BattleGroundMgr.cpp:557/623/756`). It is `SEC_ADMINISTRATOR` (6), exactly what a probe account //! holds, and it is **in-memory only** — `m_testing` is initialised `false` in the manager's -//! constructor, so a server restart clears it whatever happens here. The probe turns it on at -//! Setup and off again at Report; a crashed run leaves it on until the next restart, which is -//! surprising but harmless (a solo queue would pop). +//! constructor, so a server restart clears it whatever happens here. //! -//! Note that `.debug bg` is a **toggle, not a set**, and it announces itself to the whole realm +//! `.debug bg` is a **toggle, not a set**, and it announces itself to the whole realm //! (`LANG_DEBUG_BG_ON`/`_OFF`). Both facts are reported on the probe's own lines so a run that //! started from the wrong state reads as such rather than as a broken queue. //! -//! **It is turned back off once the match is running** (see `DOORS_SAMPLE`), because leaving it on -//! makes a solo battleground unendable: vmangos decrements its premature-finish countdown inside an -//! `else if (!sBattleGroundMgr.isTesting())` branch (`BattleGround.cpp:337`), so while testing is on -//! the countdown is armed and then **frozen**. Decision 2290 said the opposite, from reading -//! `GetPrematureFinishTime()` without the branch that consumes it; 2296 corrects it, having watched -//! a run sit in Warsong Gulch for 492 s with `GetBattlefieldWinner()` nil the whole way. +//! **The lever is tracked by PARITY, not by state** ([`BgProbe::toggles_sent`]), and put back by +//! [`leave_testing_as_found`] — which every road to [`Phase::Done`] goes through, failures +//! included. "What is it now?" can only be answered by a world-text reply that may never arrive; +//! "how far from where I found it?" needs no reply at all. It is restored **as soon as the probe +//! is inside the instance**, because that is the moment the lever has finished its job: it is +//! needed for `CheckNormalMatch` to *form* the match, not to run one. +//! +//! Leaving it on also makes a solo battleground unendable: vmangos decrements its premature-finish +//! countdown inside an `else if (!sBattleGroundMgr.isTesting())` branch (`BattleGround.cpp:337`), +//! so while testing is on the countdown is armed and then **frozen**. Decision 2290 said the +//! opposite, from reading `GetPrematureFinishTime()` without the branch that consumes it; 2296 +//! corrects it, having watched a run sit in Warsong Gulch for 492 s with `GetBattlefieldWinner()` +//! nil the whole way. //! //! ## What the census reports, and why each column is there //! @@ -56,16 +61,33 @@ //! | `status` | `GetBattlefieldStatus(1..3)`, straight out of the VM | //! | `score` | `GetNumBattlefieldScores()` / `GetBattlefieldWinner()` | //! | `guide` | the nearest `SPIRITGUIDE`-flagged unit — the graveyard's resurrect wave | -//! | `lua` | script errors collected since the last sample; a non-zero count is the finding | +//! | `dropped` | every opcode the codec threw on the floor, by name | +//! | `events` | the Lua event tap's window — or an explicit "the tap is broken", never silence | +//! | `mapinfo` | (at Report) `GetMapInfo()` and the overlay/landmark counts — whether the battle +//! map has anything to **draw**, which `IsShown()` cannot tell you | +//! +//! There is deliberately **no `lua` column**; see the note at the end of [`census`] for why the one +//! that used to be here could not measure what it claimed. +//! +//! ## What this probe cannot see +//! +//! It calls [`crate::ui_chat::idle::LastInput::stamp_present`] every frame, which suppresses the +//! whole idle band — auto-sit, auto-AFK and camp alike (`ui_chat::idle::idle_action`). That is +//! deliberate and necessary (see the note at the top of [`bg_probe`]), but it means the one +//! long-running instrument that otherwise sits still for minutes can never exercise the idle +//! handler. A regression there has to be caught somewhere else. //! //! ## The run recipe //! //! ```sh //! cd && WOW_USER=probe7 WOW_PASS=pprobe7 WOW_CHAR=Probeseven \ //! WOW_UNATTENDED=1 WOW_NOSOUND=1 WOW_GM=off WOW_PROBE_BG=wsg \ -//! timeout 420 cargo run -p benilla 2>&1 | grep -E 'PROBE bg:' +//! timeout 600 cargo run -p benilla 2>&1 | grep -E 'PROBE bg:' //! ``` //! +//! Raise the `timeout` with [`census_samples`]: a default run needs ~5 min including login, and a +//! run sized to reach a match's ending needs ~10. +//! //! `WOW_GM=off` is not optional here: GM mode re-templates the body's faction to 35, and a //! battleground is the one place where every reaction, every objective and the server's own team //! assignment reads off it (0649, 0679). @@ -161,6 +183,32 @@ const MAX_REJOINS: u32 = 3; /// interaction check; this only keeps the probe from greeting a battlemaster it has not reached. const NEAR_YD: f32 = 20.0; +/// `PLAYER_FLAGS_GM` — the bit that says the body carries GM mode's faction-35 re-template +/// (`crate::probe_shield` names the same constant). +const PLAYER_FLAGS_GM: u32 = 0x0000_0008; + +/// How long the exit waits for the battleground leave to land before going anyway. The teleport +/// out is a server round trip plus a worldport; five seconds is far more than a local server needs +/// and is a backstop, not a budget. +const LEAVE_GRACE: f64 = 5.0; + +/// How long to wait for `probe_shield`'s `.gm off` to land before refusing the run. +/// +/// **Sized by that module's own sequencing, not by guesswork** — five seconds was measured as too +/// tight and refused a run that had `WOW_GM=off` set and was about to comply. `probe_shield` waits +/// up to `NAME_WAIT_SECS` (3 s) for the body's name to resolve, then spaces its commands +/// `STEP_SECS` (0.8 s) apart with the god line *before* the `.gm off` (the shield must be up first, +/// 0679), and the flag only clears once the server's field update comes back. Twenty seconds +/// clears all of that with room, and still fails fast against a run that is never going to comply. +const GM_DROP_WAIT: f64 = 20.0; + +/// How long to keep listening for `.debug bg`'s world-text reply before giving up and saying so. +/// +/// The reply crosses the network, the chat pipeline and the Lua event dispatch before it reaches +/// the tap. The old code took **one** destructive look 1.5 s after the send, which is both too +/// narrow to be reliable and, being a single sample, unable to tell "not yet" from "never". +const TOGGLE_REPLY_WINDOW: f64 = 6.0; + /// How often a census line is printed once inside. const CENSUS_EVERY: f64 = 12.0; @@ -170,11 +218,11 @@ const CENSUS_EVERY: f64 = 12.0; /// in it: vmangos holds every arrival behind closed doors for `BG_START_DELAY_2M` = **120 s** /// (`BattleGround.cpp:248`, the four `BG_STARTING_EVENT_*` steps at 2 min / 1 min / 30 s / go), /// and `.debug bg` does not shorten it. A window that ended before then would census nothing but -/// the pen and report it as the whole battleground. Sixteen × 12 s = **192 s** covers the full -/// prep, the doors dropping, and ~70 s of live match — while staying inside vmangos's -/// `BattleGround.PrematureFinishTimer` (5 min), the clock that ends an under-populated one. -/// Twelve × 12 s = **144 s**: the full prep, the doors dropping at ~116 s, and one sample past it, -/// which leaves room for the graveyard leg inside the same run. +/// the pen and report it as the whole battleground. +/// +/// Twelve × 12 s = **144 s**: the full prep, the doors dropping at ~116 s, and two samples past +/// them, which leaves room for the flag and graveyard legs inside the same run. Reaching the +/// *ending* is a different window — see [`census_samples`]. const CENSUS_SAMPLES_DEFAULT: u32 = 12; /// The census sample count, overridable with `WOW_PROBE_BG_SAMPLES`. @@ -183,14 +231,25 @@ const CENSUS_SAMPLES_DEFAULT: u32 = 12; /// of a match: with one body inside, vmangos starts its `BattleGround.PrematureFinishTimer` (5 min, /// because `CreateNewBattleGround` reads `min_players_per_team` from the template and not from the /// testing override, so a solo match is permanently under-populated) and then ends the battleground -/// with no winner. `WOW_PROBE_BG_SAMPLES=30` is a ~6-minute window, which reaches it — the -/// `MSG_PVP_LOG_DATA` "ended" byte, `GetBattlefieldWinner()` and the automatic scoreboard, all of -/// which are built and none of which anything here has watched. +/// **with no winner** — `WINNER_NONE` = 2, not a team (`BattleGroundDefines.h:204`). +/// +/// **Sizing it is arithmetic, not taste.** The countdown is armed when the battleground reaches +/// `STATUS_IN_PROGRESS` — the doors, ≈T+120 s — and runs 300 s from there, so the ending lands at +/// ≈T+420 s ≈ **35 samples**. `WOW_PROBE_BG_SAMPLES=36` reaches it; the 30 this doc used to +/// recommend stops 60 s short and reports "the match never ended", which reads as a defect rather +/// than as a window that was too small. +/// +/// **Alterac Valley can never be ended this way at all**: the whole premature-finish block is +/// guarded by `GetTypeID() != BATTLEGROUND_AV` (`BattleGround.cpp:318`). +/// +/// A value below 1 is meaningless and is clamped to 1 — the first sample is also where the testing +/// lever is put back, and a run that never censuses never restores it. fn census_samples() -> u32 { std::env::var("WOW_PROBE_BG_SAMPLES") .ok() .and_then(|v| v.parse().ok()) .unwrap_or(CENSUS_SAMPLES_DEFAULT) + .max(1) } /// How often the ghost leg samples, and for how long. The battleground resurrect wave is a 30 s @@ -203,10 +262,6 @@ const GHOST_SAMPLES: u32 = 11; /// How many 3 s samples the flag leg takes after the use. const FLAG_SAMPLES: u32 = 5; -/// The census sample by which the doors have opened (they drop ~116 s after entry, and the census -/// ticks every 12 s). The testing flag is turned back off here — see the `Inside` arm. -const DOORS_SAMPLE: u32 = 11; - /// `SPIRITGUIDE` — `UNIT_NPC_FLAGS` bit 6, the flag the reference's area-spirit-healer acquire /// scan keys on (wow-re `interact-dead-fork-and-npc-service-ladder.md` §C row 6). const NPC_FLAG_SPIRITGUIDE: u32 = 1 << 6; @@ -246,12 +301,22 @@ BenillaBgTap:RegisterEvent("ZONE_CHANGED_NEW_AREA"); "#; /// Drain the tap: everything it recorded since the last census, then empty it. +/// +/// **It reports the tap's own liveness first**, and that is not decoration. `BenillaBgLog` is set +/// on `EVENT_TAP`'s first line, so a tap that raised *after* that line — or a tap installed into a +/// VM that has since been thrown away and minted fresh — leaves a table that drains cleanly to `""` +/// forever. An empty string would then be indistinguishable from "the battleground fired nothing", +/// which is the one answer this instrument must never give by accident. The sentinel makes the +/// difference readable, and [`bg_probe`] re-installs on seeing it. +const TAP_GONE: &str = ""; + +/// See [`TAP_GONE`]: both globals are checked, because the frame is what actually receives events +/// and the table is what holds them — either one missing means the readings are not trustworthy. const EVENT_DRAIN: &str = r#" local out = ""; -if BenillaBgLog then - for i = 1, table.getn(BenillaBgLog) do out = out .. " " .. BenillaBgLog[i]; end - BenillaBgLog = {}; -end +if not BenillaBgLog or not BenillaBgTap then return "" end +for i = 1, table.getn(BenillaBgLog) do out = out .. " " .. BenillaBgLog[i]; end +BenillaBgLog = {}; return out; "#; @@ -259,17 +324,54 @@ pub(crate) struct ProbeBgPlugin; impl Plugin for ProbeBgPlugin { fn build(&self, app: &mut App) { - app.init_resource::().add_systems(Update, bg_probe); + app.init_resource::() + .add_systems(Update, (bg_probe, bg_probe_exit).chain()); } } #[derive(Resource, Default)] struct BgProbe { phase: Phase, - /// The Lua event tap is in (or failed once and said so). + /// The Lua event tap is in and believed live. **Not a latch** — the drain re-checks the tap's + /// own global every sample and clears this if the VM went away underneath it, because the + /// interface VM is minted fresh on every world entry (`ui_script::lifecycle::mint_entry_vm`) + /// and a tap installed into the boot VM is silently gone after the port. A dead tap that still + /// reads `tap_installed` prints `EVENTS (none)` forever, which is the exact inversion this + /// instrument exists to avoid. tap_installed: bool, + /// The tap raised on install and cannot be trusted. Kept apart from `tap_installed` so an + /// `EVENTS` line can say *"the tap is broken"* rather than *"the battleground fired nothing"*. + tap_broken: bool, + /// **How many `.debug bg` toggles this run has sent — parity, not state.** + /// + /// `.debug bg` is a toggle, so "what is it now?" is the wrong question and the one the old code + /// asked: it can only be answered by reading a world-text reply that may never arrive. "How far + /// from where I found it?" is always answerable, needs no reply, and is what a restore actually + /// needs. An odd count means this run left the lever flipped and owes the realm one more send; + /// an even count means it is already as found — including the normal road, where the run turns + /// testing on to queue and off again once inside. + toggles_sent: u32, + /// What the tap last said the lever reads: `Some(true)` = on ("…for debugging"), `Some(false)` = + /// off ("…normal playercount"), `None` = no reply seen yet. Consumed once so a second pass + /// cannot re-decide on a stale marker. + testing_seen: Option, + /// The lever was **read back** as on before the join, rather than merely assumed. The `Joined` + /// failure text says which, because "the flag was confirmed ON, so look elsewhere" sends the + /// reader past the actual suspect when nothing was ever confirmed. + testing_confirmed: bool, /// How many times the queue has been dropped and re-taken (see the `Joined` arm). rejoins: u32, + /// The `AppExit` has been written — `Done` is re-entered every frame until the app actually + /// stops, and a second exit write would be noise. + exited: bool, + /// When [`Phase::Done`] was first seen, so the exit can wait for the leave to land. + done_at: Option, + /// When the `Wait` arm first saw a body, so the GM-mode gate can time out rather than hang. + waiting_since: Option, + /// Events drained before the first census (during the toggle read-back and the queue road). + /// Carried rather than discarded: the old code threw them away and then attributed them to the + /// first 12 s census window, which is a reading about the wrong interval. + pending_events: String, } /// `Wait` → (levelled, hop sent) `Hopped` → (`.debug bg` sent) `Toggled` → (reply read, testing @@ -287,6 +389,13 @@ enum Phase { sent_at: f64, corrected: bool, }, + /// Looking for the battlemaster, **after** the testing lever is settled. Split out of `Toggled` + /// so the reply read happens once: the old code re-ran its destructive drain every frame of the + /// 15 s scan, which threw away every tap event that arrived during it and re-printed a + /// "TESTING unread" warning over the "TESTING on" line it had already printed. + Scanning { + since: f64, + }, Greeted { master: u64, sent_at: f64, @@ -343,6 +452,76 @@ fn gm(net: &NetCommands, text: impl Into) { }); } +/// Drain the event tap, distinguishing its three outcomes rather than flattening them to `""`. +/// +/// Returns the events, or a bracketed reason. A `TAP_GONE` sentinel also **clears +/// `tap_installed`**, so the next frame re-installs into whatever VM is live now — which is the +/// self-heal for the one-frame world-entry window where the tap can land in a boot VM that is +/// about to be discarded (`ui_script::lifecycle`). +fn drain_events(probe: &mut BgProbe, script: &mut UiScript) -> String { + match script.eval::(EVENT_DRAIN) { + Ok(s) if s.trim() == TAP_GONE => { + probe.tap_installed = false; + format!(" {TAP_GONE} — the VM was replaced under the tap; re-installing") + } + Ok(s) => s, + Err(e) => format!(" "), + } +} + +/// Put the realm's `.debug bg` lever back if **this run** is the one that moved it. +/// +/// Every road to [`Phase::Done`] goes through this, and that is the whole point. `.debug bg` is a +/// realm-global, in-memory toggle (`BattleGroundMgr::ToggleTesting`) that announces itself to every +/// player in the world, and it is a **toggle, not a set** — so a run that ends without restoring it +/// hands the next run the opposite of what it expects. That is not hypothetical: it is how runs 2, +/// 4 and 5 of this probe failed (2290 §2). The old code restored it at exactly one place, an +/// equality test on the census counter, which every failure path and every +/// `WOW_PROBE_BG_SAMPLES < DOORS_SAMPLE` run skipped silently. +fn leave_testing_as_found(probe: &mut BgProbe, net: &NetCommands) { + if probe.toggles_sent % 2 == 1 { + info!( + "PROBE bg: TESTING — {} toggle(s) sent this run; one more puts the realm's \ + `.debug bg` lever back as found", + probe.toggles_sent + ); + toggle_testing(probe, net); + } else { + info!( + "PROBE bg: TESTING — {} toggle(s) sent this run; the lever is already as found", + probe.toggles_sent + ); + } +} + +/// Send `.debug bg` and count it. **Every** send goes through here — the parity in +/// [`leave_testing_as_found`] is only as good as the counting. +fn toggle_testing(probe: &mut BgProbe, net: &NetCommands) { + probe.toggles_sent += 1; + gm(net, ".debug bg"); +} + +/// End the run, restoring anything this run changed on the server first. +fn finish(probe: &mut BgProbe, net: &NetCommands) { + leave_testing_as_found(probe, net); + probe.phase = Phase::Done; +} + +/// A world state, distinguishing **absent** from **zero**. +/// +/// [`WorldStates::get`] is reference-faithful: a key the server never sent reads `0`, because that +/// is what the real client's table does. That is right for the client and wrong for an instrument — +/// `ws2338=0` reads as a well-formed "the flag is at its base" when it actually means "no +/// `SMSG_INIT_WORLD_STATES` ever carried this key". vmangos writes only 1 or 2 into the flag rows +/// (`BattleGroundWS.cpp:509-515`, all eight call sites), so a `0` there is *always* a missing +/// reading — and the person reading the log is exactly the person who does not know that yet. +fn ws(states: &WorldStates, key: u32) -> String { + states + .pairs() + .find(|(k, _)| *k == key) + .map_or_else(|| "absent".to_string(), |(_, v)| v.to_string()) +} + // One Bevy system's full input set — the guard-poi probe's shape, plus the census reads. #[allow(clippy::too_many_arguments)] fn bg_probe( @@ -392,6 +571,7 @@ fn bg_probe( } Err(e) => { probe.tap_installed = true; // a raise will not fix itself; say so once + probe.tap_broken = true; // …and every later EVENTS line says so too error!("PROBE bg: TAP FAILURE — {e}"); } } @@ -400,11 +580,50 @@ fn bg_probe( match probe.phase { Phase::Wait => { + // **GM mode makes every objective in this probe fail silently, so refuse to start in + // it.** vmangos's `Player::CanUseBattleGroundObject` opens with + // `if (IsGameMaster()) return false;` (`Player.cpp:20576`), which means the flag click, + // the Arathi Basin banners and Alterac Valley's towers are all rejected server-side + // with no reply — while the queue, the port, the census, the world states and the + // scoreboard all keep working perfectly. That is the worst possible failure shape: a + // run that looks 90% right and is wrong exactly where the objectives are. + // + // MEASURED, by getting it wrong: a run without `WOW_GM=off` reached the Warsong Flag at + // 1.8 yd, sent `CMSG_GAMEOBJ_USE`, and reported `ws2339=1` with no banner aura and no + // `CHAT_MSG_BG_SYSTEM_ALLIANCE` — indistinguishable, from the probe's own lines, from a + // client that cannot pick up flags. `probe_shield` drops GM mode when `WOW_GM=off` is + // set (0679), so this WAITS for the flag to clear rather than refusing outright: the + // shield arms and drops GM a few hundred ms into the world, after this arm first runs. + if store.0.player_flags() & PLAYER_FLAGS_GM != 0 { + let since = *probe.waiting_since.get_or_insert(now); + if now - since < GM_DROP_WAIT { + return; + } + error!( + "PROBE bg: FAILURE — GM mode is still on after {GM_DROP_WAIT:.0}s. A \ + battleground is the one place it cannot be left on: \ + `CanUseBattleGroundObject` refuses a GM outright (vmangos \ + `Player.cpp:20576`), so every objective fails silently while the queue, the \ + port, the census and the scoreboard all keep working. Re-run with \ + `WOW_GM=off` — and if it WAS set, look for `probe-shield:` lines: the drop \ + is sequenced behind the shield and a refused command shows up there." + ); + finish(&mut probe, &net); + return; + } // Revive first, unconditionally: `CanInteractWithNPC` refuses a dead player outright, // and the refusal surfaces as an anticheat line about an "invalid creature" — which // reads like a wrong guid, not like a corpse (`probe_bg_queue`'s note). gm(&net, ".revive"); - let level = store.0.unit_level().unwrap_or(0); + // **Wait for the real level rather than assuming one.** `unit_level()` is `None` until + // `UNIT_FIELD_LEVEL` is in the store, and this arm fires on the first frame the + // `SelfPlayer` entity is queryable. Reading a missing field as `0` sends + // `.levelup 60` to a body that may already be 60 — which makes it **120**, above every + // bracket ceiling (`battleground_template.max_lvl = 60`), and the run then dies at + // `Greeted` blaming a level *under* the floor. A missing reading is not a zero. + let Some(level) = store.0.unit_level() else { + return; + }; if level < QUEUE_LEVEL { gm(&net, format!(".levelup {}", QUEUE_LEVEL - level)); } @@ -436,45 +655,76 @@ fn bg_probe( // fills — `BattleGroundMgr.cpp:1005-1028`). Flipping the flag under a queue entry that // is already sitting there schedules nothing, so the entry waits forever. Hence: get // the flag right first, then join once. - gm(&net, ".debug bg"); + toggle_testing(&mut probe, &net); probe.phase = Phase::Toggled { sent_at: now, corrected: false, }; } Phase::Toggled { sent_at, corrected } => { - if now - sent_at < 1.5 { - return; // let the world text come back and reach the tap + // **Drain every frame and keep what comes back**, rather than taking one destructive + // look 1.5 s in. The reply is a world text that crosses the network, reaches the chat + // pipeline and then the tap; a single window wide enough to be reliable is wider than + // one worth waiting on every run, and a single window narrow enough to be quick misses + // the reply often enough to matter — a missed reply is how this probe misdiagnoses + // itself. Anything drained here that is *not* the reply is kept for the first census + // instead of being thrown on the floor. + if let Some(s) = script.as_deref_mut() { + let seen = drain_events(&mut probe, s); + // The LAST marker in the chunk wins, because both can appear in one drain when the + // correcting send's reply arrives alongside the first. `contains` cannot express + // that and was the reason a corrected run could read its own first reply again. + let on = seen.rfind("debugging"); + let off = seen.rfind("normal playercount"); + match (on, off) { + (Some(a), Some(b)) => probe.testing_seen = Some(a > b), + (Some(_), None) => probe.testing_seen = Some(true), + (None, Some(_)) => probe.testing_seen = Some(false), + (None, None) => {} + } + probe.pending_events.push_str(&seen); } - let seen = script - .as_deref_mut() - .and_then(|s| s.eval::(EVENT_DRAIN).ok()) - .unwrap_or_default(); - if seen.contains("normal playercount") { - if corrected { + match probe.testing_seen.take() { + Some(true) => { + info!( + "PROBE bg: TESTING on (1v0) — confirmed from the tap after {:.1}s", + now - sent_at + ); + probe.testing_confirmed = true; + probe.phase = Phase::Scanning { since: now }; + } + Some(false) if corrected => { error!( "PROBE bg: FAILURE — `.debug bg` reads OFF after two sends; the account \ may be below SEC_ADMINISTRATOR (6) for it" ); - probe.phase = Phase::Done; - return; + finish(&mut probe, &net); } - info!("PROBE bg: TESTING was on; that send turned it OFF — sending once more"); - gm(&net, ".debug bg"); - probe.phase = Phase::Toggled { - sent_at: now, - corrected: true, - }; - return; - } - if seen.contains("debugging") { - info!("PROBE bg: TESTING on (1v0)"); - } else { - warn!( - "PROBE bg: TESTING unread — no `.debug bg` reply in the tap ({seen:?}); \ - continuing, but a queue that never pops is why" - ); + Some(false) => { + info!("PROBE bg: TESTING was on; that send turned it OFF — sending once more"); + toggle_testing(&mut probe, &net); + probe.phase = Phase::Toggled { + sent_at: now, + corrected: true, + }; + } + None if now - sent_at > TOGGLE_REPLY_WINDOW => { + // Unread is a THIRD outcome, and it is carried forward rather than rounded to + // "on": the `Joined` failure text used to assert the flag "was confirmed ON + // before the join" even on this path, sending the reader to look at Deserter + // and brackets when the lever was the actual suspect. + warn!( + "PROBE bg: TESTING unread — no `.debug bg` reply reached the tap in \ + {TOGGLE_REPLY_WINDOW:.0}s; continuing, but if the queue never pops, this \ + is the first thing to doubt" + ); + probe.phase = Phase::Scanning { since: now }; + } + // Still inside the window: stay in `Toggled` and look again next frame. + None => {} } + } + Phase::Scanning { since: sent_at } => { let here = player.pos; // By ENTRY, not by "the nearest battlemaster": all three stand in one alcove. let master = units.iter().find(|(guid, kind, store, tf)| { @@ -495,10 +745,10 @@ fn bg_probe( }; } else if now - sent_at > 15.0 { error!( - "PROBE bg: FAILURE — {} (entry {}) never streamed within 15 yd in 15 s", + "PROBE bg: FAILURE — {} (entry {}) never streamed within {NEAR_YD:.0} yd in 15 s", arena.npc_name, arena.npc_entry ); - probe.phase = Phase::Done; + finish(&mut probe, &net); } } Phase::Greeted { master, sent_at } => { @@ -518,17 +768,23 @@ fn bg_probe( "PROBE bg: FAILURE — no SMSG_BATTLEFIELD_LIST 8 s after the hello \ (a level under the bracket floor is refused here, silently)" ); - probe.phase = Phase::Done; + finish(&mut probe, &net); } } Phase::Joined { sent_at } => { // Status 2 is "your battleground is ready, confirm within the deadline". The probe // takes it through the STOCK VERB, not through a hand-built packet: the whole point // is that the reference's own dialog road works. - let ready = queue - .slots() - .iter() - .position(|s| s.as_ref().is_some_and(|(st, _)| st.status == 2)); + // **Matched on the MAP as well as the status**, because a slot is not ours just because + // it is ready. A killed run can leave this character queued for another battleground, + // and vmangos re-sends that `SMSG_BATTLEFIELD_STATUS` at login — so a status-2 slot for + // Arathi Basin would be taken here, port the body into map 529, and the `Ported` arm + // would then time out complaining that the map "is still 529, not 489". The cause would + // read as a broken worldport; it is a stale queue entry the probe confirmed on purpose. + let ready = queue.slots().iter().position(|s| { + s.as_ref() + .is_some_and(|(st, _)| st.status == 2 && st.map_id == arena.map) + }); if let Some(index) = ready { let slot = index + 1; // the verbs are 1-based info!("PROBE bg: CONFIRM — slot {slot} is ready; AcceptBattlefieldPort({slot}, 1)"); @@ -536,12 +792,12 @@ fn bg_probe( if let Err(e) = script.eval::<()>(&format!("AcceptBattlefieldPort({slot}, 1)")) { error!("PROBE bg: FAILURE — AcceptBattlefieldPort raised: {e}"); - probe.phase = Phase::Done; + finish(&mut probe, &net); return; } } else { error!("PROBE bg: FAILURE — no VM to take the port through"); - probe.phase = Phase::Done; + finish(&mut probe, &net); return; } probe.phase = Phase::Ported { sent_at: now }; @@ -581,13 +837,21 @@ fn bg_probe( }) .collect(); error!( - "PROBE bg: FAILURE — no slot reached status 2 in 30 s (slots: {}). \ - The testing flag was confirmed ON before the join, so the cause is not that: \ - look for Deserter (26013) on the body, a bracket the server refused, or a \ - battleground this body is still registered in from a killed run.", - slots.join(" ") + "PROBE bg: FAILURE — no slot reached status 2 in 30 s (slots: {}). {} \ + Also worth checking: Deserter (26013) on the body, a bracket the server \ + refused, or a battleground this body is still registered in from a killed run.", + slots.join(" "), + if probe.testing_confirmed { + "The testing flag was READ BACK as on before the join, so the cause is \ + probably not that." + } else { + "The testing flag was NEVER READ BACK (no `.debug bg` reply reached the \ + tap), so it is the first suspect: a previous run that died before its \ + own restore leaves the lever inverted, and a queue entry is only ever \ + evaluated when a join or a leave schedules it." + }, ); - probe.phase = Phase::Done; + finish(&mut probe, &net); } } Phase::Ported { sent_at } => { @@ -609,7 +873,7 @@ fn bg_probe( "PROBE bg: FAILURE — 30 s after the port the map is still {here}, not {}", arena.map ); - probe.phase = Phase::Done; + finish(&mut probe, &net); } } Phase::Inside { @@ -622,9 +886,15 @@ fn bg_probe( } let Some(mut script) = script else { error!("PROBE bg: FAILURE — the VM went away inside the battleground"); - probe.phase = Phase::Done; + finish(&mut probe, &net); return; }; + // Drained HERE, not inside `census`, so the tap's self-heal can reach `probe` — and so + // everything the tap caught on the queue road is attributed to the window it actually + // happened in rather than being folded into the first census. + let mut events = std::mem::take(&mut probe.pending_events); + events.push_str(&drain_events(&mut probe, &mut script)); + let tap_broken = probe.tap_broken; census( now - entered_at, arena, @@ -638,26 +908,29 @@ fn bg_probe( &go_templates, &queue, &mut script, + &events, + tap_broken, ); let samples = samples + 1; - // **Flip the testing flag back OFF once the match is running**, which is the only way - // a solo battleground can ever end. vmangos decrements `m_prematureCountDownTimer` - // inside an `else if (!sBattleGroundMgr.isTesting())` branch (`BattleGround.cpp:337`), - // so while testing is on the countdown is armed and **frozen** — the arm above it runs - // once, the fire below it never reaches its condition, and a one-player Warsong Gulch - // runs forever (vanilla WSG has no time limit; three captures is its only other end). - // A run that measured this stayed in for 492 s with `winner` nil the whole way. + // **Once inside, the lever has done its job — put it back immediately.** // - // Turned off at the first sample past the doors, so the flag is only on for the queue - // that needed it. The countdown then runs its full `BattleGround.PrematureFinishTimer` - // (5 min) from that moment, which is what [`census_samples`] has to cover. - if samples == DOORS_SAMPLE { + // It was an equality test on `DOORS_SAMPLE` (11), and that was wrong twice over. Any + // `WOW_PROBE_BG_SAMPLES` below 11 — a documented, user-facing dial — skipped it + // entirely and left the realm-global flag flipped for the next run, which is the exact + // breakage 2290 §2 records as having cost runs 2, 4 and 5. And there was no reason to + // wait: the flag is needed only for `CheckNormalMatch` to *form* the match, which has + // already happened by the time we are standing in it. vmangos arms the premature + // countdown when the battleground reaches `STATUS_IN_PROGRESS` (the doors) and + // decrements it inside `else if (!sBattleGroundMgr.isTesting())` (`BattleGround.cpp:337`), + // so restoring it now simply lets that clock run from the doors rather than from + // sample 11 — twelve seconds *earlier* to an ending, not later. + if samples == 1 { info!( - "PROBE bg: TESTING off — the premature-finish countdown is frozen while it is \ - on, so a solo match could never end" + "PROBE bg: TESTING — inside the instance; the lever has done its job and the \ + premature-finish countdown is frozen while it is on" ); - gm(&net, ".debug bg"); + leave_testing_as_found(&mut probe, &net); } if samples >= census_samples() { @@ -692,7 +965,7 @@ fn bg_probe( } let Some(master) = battlefield.battlemaster() else { error!("PROBE bg: FAILURE — the battlemaster list went away before the rejoin"); - probe.phase = Phase::Done; + finish(&mut probe, &net); return; }; let _ = net.0.send(ClientCommand::BattlemasterJoin { @@ -708,7 +981,7 @@ fn bg_probe( return; // the hop crosses the map; let the far base stream in } let Some((entry, _)) = arena.flag else { - probe.phase = Phase::Done; + finish(&mut probe, &net); return; }; let found = units.iter().find(|(guid, kind, _, tf)| { @@ -744,8 +1017,9 @@ fn bg_probe( if now - at < f64::from(samples + 1) * 3.0 { return; } - let Some(script) = script else { - probe.phase = Phase::Done; + let Some(mut script) = script else { + error!("PROBE bg: FAILURE — the VM went away during the flag leg"); + finish(&mut probe, &net); return; }; // **The two readings that say the flag is on the body.** World state 2339 is the @@ -767,14 +1041,14 @@ fn bg_probe( "#, ) .unwrap_or_else(|e| format!("")); - let events = script.eval::(EVENT_DRAIN).unwrap_or_default(); + let events = drain_events(&mut probe, &mut script); info!( "PROBE bg: FLAG t={:.0}s ws2338={} ws2339={} captures={}/{} buffs=[{}]{}", now - at, - states.get(2338), - states.get(2339), - states.get(1581), - states.get(1582), + ws(&states, 2338), + ws(&states, 2339), + ws(&states, 1581), + ws(&states, 1582), auras.trim(), if events.trim().is_empty() { String::new() @@ -804,7 +1078,7 @@ fn bg_probe( error!( "PROBE bg: FAILURE — still alive 15 s after `.die` (is the shield re-armed?)" ); - probe.phase = Phase::Done; + finish(&mut probe, &net); } } Phase::Ghost { @@ -816,7 +1090,7 @@ fn bg_probe( return; } let Some(mut script) = script else { - probe.phase = Phase::Done; + finish(&mut probe, &net); return; }; // The whole point of this leg, in four readings: are we a ghost, is a spirit guide in @@ -838,7 +1112,7 @@ fn bg_probe( tostring(StaticPopup_Visible and StaticPopup_Visible(\"AREA_SPIRIT_HEAL\"))", ) .unwrap_or_else(|e| format!("")); - let events = script.eval::(EVENT_DRAIN).unwrap_or_default(); + let events = drain_events(&mut probe, &mut script); info!( "PROBE bg: GHOST t={:.0}s ghost={ghost} pos=({:.0},{:.0},{:.0}) area={:?} guide={} wave={wave}{}", now - released_at, @@ -846,10 +1120,7 @@ fn bg_probe( player.pos.y, player.pos.z, area.as_deref().and_then(|a| a.0), - nearest.map_or_else( - || "none".to_string(), - |(g, d)| format!("{g:#x}@{d:.1}yd") - ), + nearest.map_or_else(|| "none".to_string(), |(g, d)| format!("{g:#x}@{d:.1}yd")), if events.trim().is_empty() { String::new() } else { @@ -865,7 +1136,7 @@ fn bg_probe( Err(e) => error!("PROBE bg: GHOST AcceptAreaSpiritHeal() raised: {e}"), } report(arena, &net, &queue, &mut script); - probe.phase = Phase::Done; + finish(&mut probe, &net); } else { probe.phase = Phase::Ghost { released_at, @@ -878,6 +1149,58 @@ fn bg_probe( } } +/// **Reaching `Done` ends the process.** This probe was the one live probe here without it +/// (`probe_auction`'s tail and `ProbeExitPlugin::fire_probe_exit` are the established shape), so +/// the client simply idled at `Done` until whatever external `timeout` wrapped the run killed it: +/// a four-minute Warsong Gulch census cost a fifteen-minute wall clock, and a sequence of runs +/// spent most of its time watching a finished probe stand still. +/// +/// It is its own system rather than an arm of [`bg_probe`] only because that system is already at +/// Bevy's parameter ceiling — there is nothing conditional about the split. +/// +/// **It waits for the leave to land before it goes**, and that wait is the whole reason this is not +/// a one-liner. [`report`] ends by teleporting the body out to map 0, and `AppExit` stops the +/// Update schedule — so exiting on the next frame can cut the connection before the worldport +/// completes and strand the body **inside a live battleground**, which is precisely the poisoned +/// state this probe warns the next run about. So: leave first, confirmed by the map actually +/// changing, then exit; with [`LEAVE_GRACE`] as the backstop for a leave that never lands, because +/// hanging forever would be worse than logging out somewhere awkward. +/// +/// The polite `AppExit` plus a hard backstop on its own OS thread is the pattern, not belt and +/// braces: `AppExit` stops the Update schedule, so an in-schedule backstop could never fire, and a +/// net/winit teardown hang would otherwise leave a zombie client holding the probe account. +fn bg_probe_exit( + time: ProbeClock, + mut probe: ResMut, + map: Option>, + mut exit: MessageWriter, +) { + if probe.phase != Phase::Done || probe.exited { + return; + } + let now = time.elapsed_secs_f64(); + let since = *probe.done_at.get_or_insert(now); + let left = map.as_ref().is_none_or(|m| m.0 != arena().map); + if !left && now - since < LEAVE_GRACE { + return; + } + if !left { + warn!( + "PROBE bg: still on map {} {LEAVE_GRACE:.0}s after the leave — exiting anyway, but \ + this body may still be registered in the battleground", + arena().map + ); + } + probe.exited = true; + info!("PROBE bg: exiting"); + exit.write(AppExit::Success); + std::thread::spawn(|| { + std::thread::sleep(std::time::Duration::from_secs(5)); + warn!("PROBE bg: still alive 5s after AppExit — hard exit"); + std::process::exit(0); + }); +} + /// One census sample — eleven greppable `PROBE bg:` lines describing everything the client can see /// from inside the battleground. Deliberately verbose: this is the first look anyone has had, and /// a column nobody needed is cheaper than a round trip for one that was left out. @@ -895,6 +1218,8 @@ fn census( go_templates: &crate::go_templates::GameObjectTemplates, queue: &BattlefieldQueue, script: &mut UiScript, + events: &str, + tap_broken: bool, ) { use benilla_protocol::EntityKind as K; @@ -909,8 +1234,11 @@ fn census( }, ); info!( + // `map=none` rather than `map=0`: 0 is Eastern Kingdoms, a real answer, and printing it + // for "there is no `CurrentMap` resource" is the same missing-reads-as-a-value mistake + // the world-state column had. "PROBE bg: CENSUS t={t:.0}s map={} area={:?} pos=({:.0},{:.0},{:.0}) terrain={terrain}", - map.map_or(0, |m| m.0), + map.map_or_else(|| "none".to_string(), |m| m.0.to_string()), area.and_then(|a| a.0), p.x, p.y, @@ -1067,28 +1395,29 @@ fn census( ); // The event tap's window (see `EVENT_TAP`): what the VM was actually told since the last - // sample. An empty window while the server is running the start countdown is a finding. - let events = script - .eval::(EVENT_DRAIN) - .unwrap_or_else(|e| format!("")); + // sample. An empty window while the server is running the start countdown is a finding — + // **provided the tap is alive**, which is why a broken tap says so instead of printing + // `(none)`. "The battleground fired nothing" and "nobody was listening" are opposite + // conclusions and used to produce identical lines. info!( "PROBE bg: EVENTS{}", - if events.trim().is_empty() { + if tap_broken { + " ".to_string() + } else if events.trim().is_empty() { " (none)".to_string() } else { - events + events.to_string() } ); - let errors = script.take_errors(); - if errors.is_empty() { - info!("PROBE bg: LUA clean"); - } else { - for e in &errors { - error!("PROBE bg: LUA ERROR {e}"); - } - } - let _ = arena; + // **No `LUA` column, deliberately.** It used to print `LUA clean` off `take_errors()`, which + // reads as "the interface raised nothing inside a battleground for the last 12 s". It cannot + // mean that: `take_errors` is a DRAIN, and two systems already drain it every frame + // (`ui_script::input` and `ui_script::extract::extract`), so this call could only ever see + // whatever our own `eval`s in this same frame had just raised. A column that reports a 12 s + // window while measuring a fraction of one frame is worse than no column, because it reads as + // evidence. Engine-side Lua raises appear as `WARN ui_script:` lines — grep for those. + let _ = (arena, script); } /// The run's last act: drive the two battleground windows the census cannot reach by watching @@ -1105,6 +1434,24 @@ fn report(arena: &Arena, net: &NetCommands, queue: &BattlefieldQueue, script: &m // // Through `getglobal`, because `BattlefieldMinimap` is LoadOnDemand: indexing it before the // toggle that loads it is a nil-index raise, and "not loaded yet" is a reading, not an error. + // **Does the battle map have anything to draw?** `IsShown()` alone cannot say: the stock + // `BattlefieldMinimap_Update` bails on its fourth line when `GetMapInfo()` is nil, so a window + // that is up and completely empty reports exactly the same `IsShown=1` as one full of terrain. + // These four readings are the difference, and they are the ones the minimap round never took. + let mapinfo = script + .eval::( + r#" + local f, sx, sy, ox, oy = GetMapInfo(); + return "file=" .. tostring(f) + .. " continent=" .. tostring(GetCurrentMapContinent()) + .. " zone=" .. tostring(GetCurrentMapZone()) + .. " overlays=" .. tostring(GetNumMapOverlays()) + .. " landmarks=" .. tostring(GetNumMapLandmarks()); + "#, + ) + .unwrap_or_else(|e| format!("")); + info!("PROBE bg: MAPINFO {mapinfo}"); + for name in ["WorldStateScoreFrame", "BattlefieldMinimap"] { let shown = script .eval::(&format!( @@ -1148,8 +1495,15 @@ fn report(arena: &Arena, net: &NetCommands, queue: &BattlefieldQueue, script: &m queue.active_map() ); let _ = script.eval::<()>("LeaveBattlefield()"); - gm(net, ".recall"); - // The lever was already put back at `DOORS_SAMPLE` (see the `Inside` arm) — a second toggle - // here would turn it back ON and leave it that way for the next run. + // **NOT `.recall`.** vmangos saves the recall point on *every* `.go` + // (`HandleGoHelper` → `SaveRecallPosition()`, `TeleportCommands.cpp:755`), so the flag leg's + // `.go xyz … 489` overwrote it with a position **inside the battleground**. `.recall` then + // resolves to the same map and `Player::TeleportTo` takes its near-teleport arm — the probe + // printed `LEAVING map 489` and went nowhere, leaving the body logged out inside a live + // instance and the queue slot uncleared. That is the poisoned state the `Joined` failure text + // warns the *next* run about, manufactured by this one. An explicit hop to the battlemaster + // alcove on map 0 is a real map change, which is what clears the slot. + let [x, y, z] = arena.at; + gm(net, format!(".go xyz {x} {y} {z} 0")); info!("PROBE bg: DONE"); } diff --git a/crates/benilla-app/src/chr_classes.rs b/crates/benilla-app/src/chr_classes.rs index 69b5c1ac..098754f6 100644 --- a/crates/benilla-app/src/chr_classes.rs +++ b/crates/benilla-app/src/chr_classes.rs @@ -1,14 +1,16 @@ //! `ChrClasses.dbc`, loaded once and read by everything that asks the client a question about a //! class. //! -//! Two consumers today and they are unrelated to each other — [`crate::ui_pet_book`] wants field -//! 4's pet name token, `UnitHasRelicSlot` wants field 16's relic flag — which is exactly why the -//! table does not live inside either of them. It used to live in the pet book, back when the pet +//! Three consumers today and they are unrelated to each other — [`crate::ui_pet_book`] wants field +//! 4's pet name token, `UnitHasRelicSlot` wants field 16's relic flag, and +//! [`crate::spell_mods`] wants field 15's class spell-family — which is exactly why the +//! table does not live inside any of them. It used to live in the pet book, back when the pet //! token was the only column anyone read. //! -//! The resource is **absent**, not empty, when the load fails. Both readers then fall to the -//! reference's own degraded answer (`"PET"` for the token, no relic slot for any class), so a -//! missing table costs a warlock the word "Demon" and a paladin their relic branches rather than +//! The resource is **absent**, not empty, when the load fails. Every reader then falls to the +//! reference's own degraded answer (`"PET"` for the token, no relic slot for any class, family 0 +//! — which the modifier gate's first conjunct refuses), so a missing table costs a warlock the +//! word "Demon", a paladin their relic branches and everyone their talent modifiers rather than //! taking the client down. use bevy::prelude::*; @@ -39,8 +41,9 @@ fn load_chr_classes(mut commands: Commands, assets: Option>) { Ok(table) => commands.insert_resource(ChrClassTable(table)), Err(e) => warn!( "chr_classes: ChrClasses.dbc failed to load — every pet book tab reads the client's \ - own \"PET\" fallback, so a warlock's says Pet rather than Demon, and no class reads \ - as having a relic slot: {e:#}" + own \"PET\" fallback, so a warlock's says Pet rather than Demon, no class reads \ + as having a relic slot, and no talent spell modifier can apply (the gate has no \ + class family to match): {e:#}" ), } } diff --git a/crates/benilla-app/src/game_plugins.rs b/crates/benilla-app/src/game_plugins.rs index 9fb0ce34..8ab64db7 100644 --- a/crates/benilla-app/src/game_plugins.rs +++ b/crates/benilla-app/src/game_plugins.rs @@ -262,6 +262,11 @@ impl PluginGroup for GamePlugins { // `_CANT_ATTACK`): the latch the packets set, and the 4 s repeat that shows it while an // attack target stands and no swing lands. .add(crate::swing_refusal::SwingRefusalPlugin) + // The talent spell-modifier tables (`SMSG_SET_FLAT_/PCT_SPELL_MODIFIER`): the wire + // fills them, world-enter clears them, and `usable::power_cost` reads op 14 out of + // them. Beside the swing refusal because both are the same shape — a small wire-fed + // store with a world-entry reset — and neither has a feed of its own. + .add(crate::spell_mods::SpellModsPlugin) // Being summoned (decision 1747): SMSG_SUMMON_REQUEST's latch, the CONFIRM_SUMMON dialog it // raises, and the CMSG_SUMMON_RESPONSE its Accept sends. The binder's twin one line up — a // server-asked question whose only wire answer is yes — and here for that reason. @@ -839,7 +844,7 @@ pub(crate) mod schedule_tests { /// **Measured 2026-09-17 (decision 2287), 662 systems:** 16,548 pairs in all, 12,849 of /// them explained — 10,703 on the VM alone, 431 on the other non-`Send` owners, 556 on /// the pure caches, 87 on the sinks, 41 on the stream, 1,031 on a mix of those — and - /// 3,705 actionable. The largest part of those is `Transform` on disjoint lanes (716 pairs + /// 3,699 actionable. The largest part of those is `Transform` on disjoint lanes (716 pairs /// alone): populations that never intersect, which the filter algebra cannot see. A lane /// is not a class — the checker cannot tell a disjoint lane from two writers of the same /// entity — so they stay here, and each new one is a claim made at the registration with @@ -867,17 +872,37 @@ pub(crate) mod schedule_tests { /// the pairs over the pure caches moved into the VM-only class, where the same systems still /// meet over the VM alone. /// - /// **3,290 (decision 2291)** — the area-spirit-healer poll's `&Transform` read against the frame's - /// movers (motion, transports, the portrait booths, the quest markers' bake): **+6**, measured - /// on the rebased tree rather than assumed additive, the same discipline 2295 used one - /// paragraph up. Its one write-write pair — against the click, which also holds - /// `AreaSpiritHealer` — is *declared*, and declared where the reference puts it: after the - /// frame's pick, because `0x4923b0` runs from the same `CGWorldFrame` OnUpdate that ran the - /// pick eighty bytes earlier at `0x48184a`. What is left is reading a unit's position one - /// mover early or late, which moves a 20 yd acquire decision by at most one frame of walking - /// — and the poll is **level**-triggered, re-deriving the whole cache every frame, so a - /// boundary case decided early is decided again next frame. The target scanner carries the - /// same class for the same reason. + /// **3,288 (decision 2291, corrected)** — the area-spirit-healer poll. Its **four** remaining + /// pairs, named one by one because the first version of this paragraph named the wrong ones: + /// `portrait::booth::face_booth_billboards`, `quest_markers::bake_seat_scale` and + /// `entities::live_display::tick_scale_ease` over `Transform`, and + /// `entities::live_display::refresh_live_display` over **`NetEntity`** — a row read, not a + /// `Transform` pair at all. All four are the poll seeing a unit one mover early or late, which + /// moves a 20 yd acquire decision by at most one frame of walking; the poll is + /// **level**-triggered, re-deriving the whole cache every frame, so a boundary case decided + /// early is decided again next frame. The target scanner carries the same class for the same + /// reason. + /// + /// **The correction is the part worth keeping.** This paragraph first claimed "+6, all + /// `&Transform`", and that the poll's "one write-write pair … is *declared*". Both were wrong, + /// and the second one hid a real defect. There were **two** undeclared write-write pairs, and + /// the system on the other side of both was `drain_latch_verbs` — the one that turns + /// `AcceptAreaSpiritHeal()` into `CMSG_AREA_SPIRIT_HEALER_QUEUE`. It met the poll *and* + /// `target::click::act_on_right_click` over `AreaSpiritHealer`, because `.after(UiInput)` + /// orders it against neither (`UiInput` precedes `WorldStage::Input`; `TargetUpdate` follows + /// it). The drain takes its presses unconditionally and only then reads the healer, so that + /// undeclared order decided whether a ghost's Accept became a packet or vanished with no + /// message. Declaring `drain_latch_verbs.before(TargetUpdate)` — the reference's own order, the + /// dialog's Lua handler running in the UI dispatch ahead of `CGWorldFrame`'s poll and pick — + /// removed exactly those two. + /// + /// **Re-measured at 3,289 on the rebased tree**, not carried across as arithmetic. This branch + /// measured 3,290 → 3,288 on its own base; a neighbour landed 3,291 first, and the resolution + /// is the number the dump prints on the merged tree — which happens to agree with 3,291 − 2 + /// this time, and is right for the reason that it was read rather than that it adds up. + /// + /// A ceiling raised with the wrong reason is a ratchet that has stopped meaning anything. The + /// lesson this one cost: **read the dump, do not reason about what the new pairs must be.** /// /// **3,291 (decision 2300)** — the glue create/main-menu scene's material lane: exactly **one** /// new pair, `ui_models::forget_dead_vm_tiles` against `portrait::glue_booth::sync_glue_scene` @@ -894,7 +919,7 @@ pub(crate) mod schedule_tests { /// Raising this ceiling is a claim that a new undeclared order is acceptable; make it with /// the reason, or declare the order instead (`.after`, a set, a `chain`). If the pair is /// about a resource that commutes by construction, the claim belongs in [`Classes`]. - const UPDATE_ACTIONABLE_CEILING: usize = 3_291; + const UPDATE_ACTIONABLE_CEILING: usize = 3_289; const UPDATE_ACTIONABLE_SLACK: usize = 40; fn ratchet(what: &str, n: usize, ceiling: usize, slack: usize) { @@ -1189,6 +1214,8 @@ pub(crate) mod schedule_tests { const EXEMPT: &[(&str, &str, Because, &str)] = &[ ("bindings.rs", "sync_dispatch", Because::MemoLatched, "`seen_generation` is a `VmMemo`: a new VM reads `None`, rebuilds and re-fires UPDATE_BINDINGS"), + ("capture/probe_bg.rs", "bg_probe", Because::SelfHealing, + "the battleground probe: dev-only (`WOW_PROBE_BG`, `cfg(feature = \"dev\")`) so it is not in a player build at all, and its `mem::take` is of its OWN pending-events string, not a queue anything else fills. Its one real VM dependency is the Lua event tap, which self-heals: `EVENT_DRAIN` returns a `` sentinel when the tap's globals are missing — the case this window causes, since a tap installed in the boot VM is discarded when `mint_entry_vm` builds the interface — and the probe re-installs on the next frame"), ("death.rs", "feed_death", Because::MemoLatched, "`feed.vm: VmMemo`: a fresh memo makes the first snapshot an edge and re-announces a held offer, confirm and corpse range"), ("screenshot.rs", "ask_for_captures", Because::FilledByVm, diff --git a/crates/benilla-app/src/lib.rs b/crates/benilla-app/src/lib.rs index ce80f3cf..d9ad2faf 100644 --- a/crates/benilla-app/src/lib.rs +++ b/crates/benilla-app/src/lib.rs @@ -119,6 +119,9 @@ mod opaque2d; mod shutdown; mod smart_rect; mod sound; +/// The two talent spell-modifier tables (`SMSG_SET_FLAT_/PCT_SPELL_MODIFIER`) and the read that +/// puts them on a number. +mod spell_mods; /// The melee swing refusal's latch + 4 s repeat (`SMSG_ATTACKSWING_*`). mod swing_refusal; mod target; diff --git a/crates/benilla-app/src/net/apply.rs b/crates/benilla-app/src/net/apply.rs index 7cb78fcc..8bb832b6 100644 --- a/crates/benilla-app/src/net/apply.rs +++ b/crates/benilla-app/src/net/apply.rs @@ -53,8 +53,8 @@ use quests::{ use spells::{ action_buttons, aura_duration, cancel_auto_repeat, cast_result, channel_start, channel_update, clear_cooldown, cooldown_cheat, cooldown_event, item_cooldown, learned_spell, removed_spell, - spell_book, spell_chain_targets, spell_cooldowns, spell_delayed, spell_failed_other, spell_go, - spell_start, superceded_spell, + set_spell_modifier, spell_book, spell_chain_targets, spell_cooldowns, spell_delayed, + spell_failed_other, spell_go, spell_start, superceded_spell, }; /// Which unit's cooldown store a wire cooldown packet addresses (decision 0982). @@ -144,6 +144,7 @@ pub(crate) fn apply_net_updates( mut ui_error_texts, mut queued_melee, mut aura_durations, + mut spell_mods, } = actions; let AnimWriters { mut server_sounds, @@ -1649,6 +1650,12 @@ pub(crate) fn apply_net_updates( &mut aura_durations, real_clock.elapsed_secs_f64(), ), + SessionEvent::SpellModifier { + flat, + mask_bit, + op, + value, + } => set_spell_modifier(flat, mask_bit, op, value, &mut spell_mods), SessionEvent::PlaySpellVisual { unit, kit_id } => { anim::play_spell_visual(unit, kit_id, &index, play_seq, &mut kit_pushes) } diff --git a/crates/benilla-app/src/net/apply/params.rs b/crates/benilla-app/src/net/apply/params.rs index fe31af7f..4adcbf4d 100644 --- a/crates/benilla-app/src/net/apply/params.rs +++ b/crates/benilla-app/src/net/apply/params.rs @@ -298,6 +298,9 @@ pub(crate) struct ActionStores<'w> { /// `SMSG_UPDATE_AURA_DURATION` lands here keyed by raw slot, timestamped on /// [`Clocks::real`] for the `ui_aura` slot-join. pub aura_durations: ResMut<'w, crate::ui_aura::AuraDurations>, + /// The two talent spell-modifier tables (`crate::spell_mods`) — one wire packet is one + /// absolute cell, and this is its only writer. + pub spell_mods: ResMut<'w, crate::spell_mods::SpellModifiers>, } /// Every animation-bearing or audible message the drain emits, in packet order, plus the diff --git a/crates/benilla-app/src/net/apply/spells.rs b/crates/benilla-app/src/net/apply/spells.rs index 5c549053..a2a6a15c 100644 --- a/crates/benilla-app/src/net/apply/spells.rs +++ b/crates/benilla-app/src/net/apply/spells.rs @@ -1046,6 +1046,23 @@ pub(super) fn aura_duration( durations.set(slot, remaining_ms, now_secs); } +/// One cell of one talent spell-modifier table (`SMSG_SET_FLAT_SPELL_MODIFIER` / +/// `SMSG_SET_PCT_SPELL_MODIFIER`) — `HandleSetSpellModifier 0x6e9950`'s whole body, which is a +/// single store: the server sends the absolute value of that `(family bit, op)` pair, never a +/// delta, so there is nothing to accumulate and nothing to invalidate. +/// +/// The out-of-range refusal lives on the store ([`crate::spell_mods::SpellModifiers::set`], which +/// documents why it is ours and not the reference's). +pub(super) fn set_spell_modifier( + flat: bool, + mask_bit: u8, + op: u8, + value: i32, + mods: &mut crate::spell_mods::SpellModifiers, +) { + mods.set(flat, mask_bit, op, value); +} + /// The caster's chain-target hop array, filled from a wire target list — the reference's /// `0x605780` (decision 0955): it **clears before it fills** and **skips any entry equal to the /// unit's own guid** (`0x6057bf`/`0x6057c9`). Targets not streamed to us drop out here: an endpoint @@ -1365,6 +1382,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::(); @@ -1657,6 +1675,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::(); let self_e = app @@ -1825,6 +1844,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::(); let self_e = app @@ -1959,6 +1979,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::(); @@ -2089,6 +2110,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::(); let self_e = app @@ -2370,6 +2392,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::(); let self_e = app @@ -2559,6 +2582,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::(); let self_e = app.world_mut().spawn((Guid(10), SelfPlayer)).id(); diff --git a/crates/benilla-app/src/spell_mods.rs b/crates/benilla-app/src/spell_mods.rs new file mode 100644 index 00000000..cfb2cede --- /dev/null +++ b/crates/benilla-app/src/spell_mods.rs @@ -0,0 +1,564 @@ +//! The **talent spell-modifier tables** — the client-side half of every talent that cheapens a +//! spell, shortens its cast or cooldown, extends its range or radius, or lengthens its duration. +//! +//! Two `i32[64][29]` tables (`SPELLMOD_FLAT 0xcead60`, `SPELLMOD_PCT 0xcecb30`), one handler that +//! fills them (`Spell_C::HandleSetSpellModifier 0x6e9950`, registered for both +//! `SMSG_SET_FLAT_SPELL_MODIFIER` and `SMSG_SET_PCT_SPELL_MODIFIER` by `Spell_C::SystemInitialize +//! 0x6e7150`), and one reader that answers them (`GetSpellModifiers 0x6e6b30`). Every law here is +//! byte-VERIFIED in wow-re `system/spell/scratch/spellmod-table-law.md`; this module is that note +//! turned into the one place benilla asks "does a talent change this number?". +//! +//! **The index is `mask_bit * 29 + op`, in both directions, and it is the thing to get right.** +//! The writer multiplies its FIRST wire byte (`6e9993: imul eax,eax,0x1d`); the reader +//! strength-reduces the same address to `4*op + 0x74*i` over a loop counter bounded by +//! `cmp eax,0x40`, which forces the first factor to be the 0..63 `SpellFamilyFlags` **bit index** +//! and the second the 0..28 SpellModOp. The transposed form addresses 47% of the array +//! (`29*28 + 63 = 875` against 1856 cells) and makes writer and reader touch systematically +//! disjoint cells — every modifier silently dropped, nothing to see in a log. wow-re's own +//! `wave-handlers.md` carried the transposed form, `verified`, for months. [`index`] is the single +//! expression both sides here go through, so they cannot drift apart. +//! +//! The server agrees, independently and in source: vmangos `Player::SendSpellMod` +//! (`Objects/Player.cpp:17815`) loops `for (int eff = 0; eff < 64; ++eff)` over the modifier's own +//! 64-bit mask and writes `uint8(eff)`, `uint8(mod->op)`, `int32(val)` for each set bit — first +//! byte the family-flag bit, second the op — with `MAX_SPELLMOD = 29` as the enum's size. +//! +//! **The value is absolute.** The store is a plain `mov`, and vmangos `Player::SendSpellMod` sends +//! one packet per set mask bit carrying that `(bit, op)` pair's total. Nothing accumulates, nothing +//! expires, and there is no talent-change clear: a respec is a fresh set of absolute cells. +//! +//! **The read is three conjunct gates and then a sum over all 64 bits** ([`SpellModifiers::apply`] +//! via [`SpellModifiers::modifiers`]): the spell's `SpellFamilyName` must be nonzero, must equal +//! the local player's own class family ([`SpellModifiers::class_family`]), and the spell must not +//! carry `AttributesEx3` bit 29. Then every set bit of the spell's 64-bit `SpellFamilyFlags` +//! contributes one cell from each table — **no early break**, and the high dword is live (322 +//! shipped spells set more than one bit; the highest index in the file is 35). +//! +//! **The trap this module's shape exists to make unrepresentable.** `0x6e6b30` has four exits. +//! Three write `(flat, pct) = (0, 100)`; the fourth — *the loop ran and nothing accumulated*, which +//! is the COMMON case — leaves `pct = 0`, because it never reaches the `add ebx,0x64`. The +//! reference gets away with it because all three appliers gate on the returned boolean. A +//! re-implementation that hands a caller a bare `(flat, pct)` pair multiplies every unmodified +//! spell by **zero**: every spell would cost 0 mana. So [`SpellModifiers::modifiers`] returns +//! `Option` — `None` IS that exit, and it is not a pair anyone can multiply by — and +//! [`SpellMod`] is private with [`SpellMod::apply`] its only verb. `apply` is the reference's own +//! integer applier `0x6e6af0`, boolean gate included. +//! +//! ## The 29 ops +//! +//! All 29 are stored: the stride is what the index arithmetic is made of, and nine of them have no +//! reader anywhere in the reference image while the handler will happily write them. The numbers +//! are VERIFIED (every one of the 35 call sites passes a literal); the names are vmangos +//! corroboration, and **op 24's name is INFERRED and suspect** — the client routes aura 65 +//! (*casting speed*) to it while vmangos calls it `SPELL_BONUS_DAMAGE`, so key on the number. +//! +//! | op | name | benilla consumer | +//! |---|---|---| +//! | 0 | DAMAGE | — | +//! | 1 | DURATION | — | +//! | 2 | THREAT | — | +//! | 3 | ATTACK_POWER | — | +//! | 4 | CHARGES | — | +//! | 5 | RANGE | — | +//! | 6 | RADIUS | — | +//! | 7 | CRITICAL_CHANCE | never read by the reference | +//! | 8 | ALL_EFFECTS | — | +//! | 9 | NOT_LOSE_CASTING_TIME | never read | +//! | 10 | CASTING_TIME | — | +//! | 11 | COOLDOWN | — | +//! | 12 | SPEED | — | +//! | 13 | *(no enumerator)* | never read | +//! | **14** | **COST** | [`OP_COST`] — `ui_action::usable::power_cost` | +//! | 15 | CRIT_DAMAGE_BONUS | never read | +//! | 16 | RESIST_MISS_CHANCE | never read | +//! | 17 | JUMP_TARGETS | — | +//! | 18 | CHANCE_OF_SUCCESS | — | +//! | 19 | ACTIVATION_TIME | — | +//! | 20 | EFFECT_PAST_FIRST | never read | +//! | 21 | GLOBAL_COOLDOWN | — | +//! | 22 | DOT | — | +//! | 23 | HASTE | — | +//! | 24 | *(aura-65 op; name INFERRED)* | — | +//! | 25 | *(no enumerator)* | never read | +//! | 26 | *(no enumerator)* | never read | +//! | 27 | MULTIPLE_VALUE | — | +//! | 28 | RESIST_DISPEL_CHANCE | never read | +//! +//! ## Where this diverges from the reference, and why +//! +//! - **Both wire bytes are range-checked.** The reference checks neither: `field1 = 64, field2 = 19` +//! writes at byte 7500 from the FLAT table's base — 76 past its end, straight onto `[0xcecaac]`, +//! the class-family global the reader itself compares against — and `field1 = 65, field2 = 23` +//! lands on element 0 of the PCT table. That is a buffer overrun, not a behaviour, and reproducing it is not +//! fidelity. [`SpellModifiers::set`] drops the packet with a `warn!` naming both values. +//! - **The lifecycle hangs off the session edge, not a process global.** The reference clears both +//! tables (and the class family) in `0x6e7150`, which runs once per **world-enter** — the Enter +//! World click, through `CGlueMgr::Update 0x46b930` — and nowhere else; its teardown +//! `Spell_C::Destroy 0x6e99e0` deliberately does not touch them. [`clear_on_world_enter`] is that +//! same edge, `OnEnter(ClientState::InWorld)`. + +use bevy::prelude::*; + +use benilla_formats::SpellDisplay; + +use crate::char_select::ClientState; +use crate::chr_classes::ChrClassTable; +use crate::net::{ObjectStore, SelfPlayer}; + +/// The number of SpellModOps — the table's column count, and therefore the index **stride** +/// (`imul eax,eax,0x1d`, and the reader's `add edx,0x74` = `29*4`). +const OPS: usize = 29; +/// The number of `SpellFamilyFlags` bits — the table's row count (`cmp eax,0x40`). +const BITS: usize = 64; +/// `0x740` dwords per table, exactly (`6e72ba: mov ecx,0x740` before each `rep stosd`), and +/// `29*63 + 28 == 1855` fills it with nothing left over — which is itself one of the three +/// discriminators that settle the index's factor roles. +const CELLS: usize = BITS * OPS; + +/// `SPELLMOD_COST` — the one op benilla reads today ([`crate::ui_action::usable::power_cost`], the +/// reference's `GetPowerCost 0x6e31b0` @ `6e32e3`). The module header's table names the other 28. +pub(crate) const OP_COST: u8 = 14; + +/// `AttributesEx3` bit 29 (`SPELL_ATTR_EX3_IGNORE_CASTER_MODIFIERS`, vmangos +/// `SpellDefines.h:940`) — the third gate, `0x6e6b52: test [SpellRec+0x24], 0x20000000`. A spell +/// carrying it takes no modifier at all, whatever the tables say. +const ATTR_EX3_IGNORE_CASTER_MODIFIERS: u32 = 0x2000_0000; + +/// A cell's offset in either table. **The one expression**, so the writer and the reader can never +/// disagree about which of the two bytes is the row — the exact failure this system is famous for. +/// +/// Callers have already range-checked both arguments ([`SpellModifiers::set`] refuses the packet, +/// [`SpellModifiers::modifiers`] refuses the op and iterates the bit itself), so this cannot leave +/// the array. +fn index(mask_bit: u8, op: u8) -> usize { + usize::from(mask_bit) * OPS + usize::from(op) +} + +/// What a talent does to one number: a flat term added first, then a percentage applied — the +/// reference's `(value + flat) * pct / 100`. +/// +/// Private, and [`Self::apply`] is the only way to spend it, because a `(flat, pct)` pair loose in +/// the codebase is exactly the shape that multiplies an unmodified spell by zero (module header). +/// [`Default`] is the identity, matching the three gate-failure exits that write `(0, 100)`. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +struct SpellMod { + flat: i32, + /// Already `max(0, 100 + Σ)` — the `+100` happens **once**, inside the reader (`6e6bb4`), and + /// never in an applier. So this is a raw percentage multiplier: 100 is "unchanged". + pct: i32, +} + +impl Default for SpellMod { + fn default() -> Self { + SpellMod { flat: 0, pct: 100 } + } +} + +impl SpellMod { + /// The reference's integer applier `0x6e6af0`: `*v = (*v + flat) * pct / 100`, with the flat + /// term added **before** the percentage and the division truncating **toward zero** (C signed + /// division — `6e6b14`'s magic-multiply/`sar 5`/`shr 31`/`add` sequence is exactly that). + /// + /// Computed in `i64` and saturated back, where the reference's 32-bit `imul` simply wraps: a + /// wrap here would be a debug panic on a packet nobody sent, and no real modifier comes within + /// several orders of magnitude of the boundary (a cost is a few hundred, a pct a few hundred). + fn apply(self, value: i32) -> i32 { + let scaled = (i64::from(value) + i64::from(self.flat)) * i64::from(self.pct) / 100; + scaled.clamp(i64::from(i32::MIN), i64::from(i32::MAX)) as i32 + } +} + +/// The two tables and the class family the gate compares against — benilla's `0xcead60` / +/// `0xcecb30` / `0xcecaac`. +/// +/// Filled only by the wire (`net::apply::spells::set_spell_modifier`), cleared only at world-enter +/// ([`clear_on_world_enter`]), and read live at every call site: like the reference, there is no +/// derived cache to invalidate. +#[derive(Resource)] +pub(crate) struct SpellModifiers { + flat: [i32; CELLS], + pct: [i32; CELLS], + /// The local player's class spell-family (`ChrClasses.dbc` field 15). `0` until the avatar + /// resolves, which is why the reader's first conjunct (`SpellFamilyName != 0`) exists at all: + /// it covers exactly that window, where a family-0 spell would otherwise match. + class_family: u32, +} + +impl Default for SpellModifiers { + fn default() -> Self { + SpellModifiers { + flat: [0; CELLS], + pct: [0; CELLS], + class_family: 0, + } + } +} + +impl SpellModifiers { + /// Store one cell, absolutely — `HandleSetSpellModifier 0x6e9950`'s whole body. + /// + /// The range check is benilla's, not the reference's (module header): a `mask_bit` of 64 is a + /// write into the neighbouring global there, and we would rather drop a packet than reproduce + /// an overrun. Nothing legitimate trips it — vmangos derives both bytes from its own enums. + pub(crate) fn set(&mut self, flat: bool, mask_bit: u8, op: u8, value: i32) { + if usize::from(mask_bit) >= BITS || usize::from(op) >= OPS { + warn!( + "spell_mods: dropping an out-of-range {} modifier — mask bit {mask_bit} \ + (0..=63), op {op} (0..=28), value {value}", + if flat { "flat" } else { "pct" } + ); + return; + } + let cell = index(mask_bit, op); + if flat { + self.flat[cell] = value; + } else { + self.pct[cell] = value; + } + } + + /// Set the gate's right-hand side — the reference's `0x6e6ca0`, whose only caller is the + /// local-player create-finalise. [`track_class_family`] is that caller here. + pub(crate) fn set_class_family(&mut self, family: u32) { + self.class_family = family; + } + + /// Drop everything — both tables and the class family, the exact set `0x6e7150` zeroes. + pub(crate) fn clear(&mut self) { + self.flat = [0; CELLS]; + self.pct = [0; CELLS]; + self.class_family = 0; + } + + /// `GetSpellModifiers 0x6e6b30` for one spell and one op: the three gates, then the sum over + /// every set family bit. + /// + /// `None` = "no modifier applies", covering **all four** of the reference's false exits — the + /// three gate failures and the both-sums-are-zero one (`6e6ba8`/`6e6bad`). The caller uses its + /// base value unchanged; see the module header for why this is an `Option` and not a pair. + /// + /// The op is range-checked here too. The reference does not check it either (`shl edx,2` on an + /// unmasked argument) and is safe only because all 35 of its call sites pass a literal ≤ 27 — + /// so do ours, and this costs one compare to make that structural rather than conventional. + fn modifiers(&self, d: &SpellDisplay, op: u8) -> Option { + if d.spell_family == 0 + || d.spell_family != self.class_family + || d.attributes_ex3 & ATTR_EX3_IGNORE_CASTER_MODIFIERS != 0 + || usize::from(op) >= OPS + { + return None; + } + // All 64 iterations, no early break: `n` set bits accumulate `n` cells from each table + // (`6e6b8f`/`6e6b97` are `add [mem],reg`, and the only branch in the body is the skip). + // `wrapping_add` because the reference's accumulators do, and because a server we do not + // control should not be able to panic a debug build. + let (mut flat, mut pct) = (0i32, 0i32); + for bit in 0..BITS as u8 { + if d.spell_family_flags >> bit & 1 == 0 { + continue; + } + let cell = index(bit, op); + flat = flat.wrapping_add(self.flat[cell]); + pct = pct.wrapping_add(self.pct[cell]); + } + if flat == 0 && pct == 0 { + return None; + } + // The `+100` and the clamp, once, here — never in an applier (`6e6bb4` … `6e6bc7`). + Some(SpellMod { + flat, + pct: pct.wrapping_add(100).max(0), + }) + } + + /// Put a spell's modifier for `op` onto `value`, or hand `value` back untouched — the + /// reference's applier plus its `test al,al` gate, in one verb so a caller cannot skip the + /// gate. + pub(crate) fn apply(&self, d: &SpellDisplay, op: u8, value: i32) -> i32 { + self.modifiers(d, op).map_or(value, |m| m.apply(value)) + } +} + +/// Entering the world drops both tables and the class family — `Spell_C::SystemInitialize +/// 0x6e7150`'s two `rep stosd`s and its `[0xcecaac] = 0`, which the §5 walked to +/// `CGlueMgr::Update 0x46b930` and pinned to **once per world entry**, not once per process. +/// +/// There is deliberately nothing else: no talent-change clear (the server re-sends absolute cells), +/// and no world-LEAVE clear (the reference's own teardown `0x6e99e0` never touches them, and the +/// next entry is what zeroes them). +fn clear_on_world_enter(mut mods: ResMut) { + mods.clear(); +} + +/// Keep the gate's class family in step with the avatar — the reference's `0x6e6ca0`, whose sole +/// caller is the local-player create-finalise `0x5debcc`: it reads `UNIT_FIELD_BYTES_0` byte 1 and +/// stores `ChrClasses[class]` field 15. +/// +/// One writer rather than a line wherever the avatar is built, for the reason +/// `char_select::publish_world_live` gives: a mirror a future transition can forget is worse than +/// the coupling it replaces. And it **never writes 0 back** — an absent avatar leaves the last +/// value standing, because a cross-map worldport drops the entity mid-session while everything the +/// server told us stays true (decision 0900's shape). Only the world-enter clear zeroes it. +fn track_class_family( + mut mods: ResMut, + classes: Option>, + self_q: Query<&ObjectStore, With>, +) { + let Some(classes) = classes else { return }; + let Some(class) = self_q.iter().next().and_then(|s| s.0.unit_class()) else { + return; + }; + let family = classes.0.spell_family(u32::from(class)); + // Guarded so the write is a real edge: an unconditional store would mark the resource changed + // every frame, and the tooltip feed rebuilds on exactly that signal. + if mods.class_family != family { + mods.set_class_family(family); + } +} + +pub(crate) struct SpellModsPlugin; + +impl Plugin for SpellModsPlugin { + fn build(&self, app: &mut App) { + app.init_resource::() + .add_systems( + Update, + // After the net stage that merges the avatar's descriptor, and before the feeds + // that read a cost through it — so the family is this frame's, never last + // frame's, on the frame the avatar first resolves. + track_class_family + .after(benilla_world::schedule::WorldStage::Net) + .before(crate::ui_unit::UnitFeed), + ) + .add_systems(OnEnter(ClientState::InWorld), clear_on_world_enter); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A mage spell (family 3) with the given bits set, against a mage player. + fn mage(bits: &[u8]) -> SpellDisplay { + SpellDisplay { + spell_family: 3, + spell_family_flags: bits.iter().fold(0u64, |m, b| m | 1 << b), + ..Default::default() + } + } + + fn mage_tables() -> SpellModifiers { + let mut mods = SpellModifiers::default(); + mods.set_class_family(3); + mods + } + + /// **The index, and the transposition that would pass a lazier test.** The cell the server + /// wrote for `(bit 5, op 14)` must be the cell the reader reads for a spell with bit 5 set — + /// and, crucially, must NOT be the one it reads for a spell with bit 14 set, which is where + /// `op * 29 + bit` lands the same packet. + /// + /// Both halves are needed: a symmetric pair like `(14, 14)` agrees under either arithmetic, so + /// this uses 5 and 14 and asserts the cross-read is empty. + #[test] + fn the_index_is_mask_bit_times_29_plus_op() { + assert_eq!(index(5, 14), 5 * 29 + 14); + assert_ne!( + index(5, 14), + 14 * 29 + 5, + "the transposed form is a different cell" + ); + + let mut mods = mage_tables(); + mods.set(true, 5, OP_COST, -30); + + // The spell the packet is about: bit 5 set, op 14 read. + assert_eq!(mods.apply(&mage(&[5]), OP_COST, 100), 70); + // The spell the TRANSPOSED index would have served: bit 14 set. Under `op*29 + bit` the + // write would have landed where this read looks, and this would be 70. + assert_eq!( + mods.apply(&mage(&[14]), OP_COST, 100), + 100, + "bit 14 is not bit 5 — a transposed index would have crossed them" + ); + } + + /// **The pct-0 trap.** The common path is "this spell has no talent on it": the reader's + /// scanned-and-nothing-matched exit leaves `pct = 0` in the reference, and a consumer that + /// ignored the boolean would multiply by it. Here the exit is `None`, so the base value + /// survives — for a spell with no bits at all, for a spell whose bits are all zero cells, and + /// for each of the three gate failures. + #[test] + fn nothing_applying_leaves_the_value_exactly_alone() { + let mut mods = mage_tables(); + // A live modifier somewhere else in the table, so "the tables are empty" is not what is + // being tested. + mods.set(false, 30, OP_COST, -100); + + assert_eq!(mods.apply(&mage(&[]), OP_COST, 100), 100, "no family bits"); + assert_eq!( + mods.apply(&mage(&[5]), OP_COST, 100), + 100, + "bit 5's cells are zero" + ); + assert_eq!( + mods.apply(&mage(&[30]), OP_COST, 100), + 0, + "and the live cell really is live — a −100% cost, not an inert table" + ); + + // Gate 1: SpellFamilyName == 0 (every creature and item spell in the file). + let mut d = mage(&[30]); + d.spell_family = 0; + assert_eq!(mods.apply(&d, OP_COST, 100), 100); + // Gate 2: another class's spell. + let mut d = mage(&[30]); + d.spell_family = 4; + assert_eq!(mods.apply(&d, OP_COST, 100), 100); + // Gate 3: IGNORE_CASTER_MODIFIERS. + let mut d = mage(&[30]); + d.attributes_ex3 = ATTR_EX3_IGNORE_CASTER_MODIFIERS; + assert_eq!(mods.apply(&d, OP_COST, 100), 100); + // An op with no cells is not a wipe either. + assert_eq!(mods.apply(&mage(&[30]), 0, 100), 100); + } + + /// The sum runs over **every** set bit of a 64-bit mask — both dwords — and the flat term is + /// added before the percentage. + #[test] + fn every_set_bit_contributes_and_flat_lands_before_pct() { + let mut mods = mage_tables(); + mods.set(true, 5, OP_COST, -10); + mods.set(true, 35, OP_COST, -15); + mods.set(false, 5, OP_COST, -20); + mods.set(false, 35, OP_COST, -30); + + // Bit 35 alone: the HIGH dword, which a 32-bit mask read would drop entirely. + assert_eq!( + mods.apply(&mage(&[35]), OP_COST, 100), + (100 - 15) * 70 / 100 + ); + // Both: flat −25 first, then 100 − 50 = 50 percent. + assert_eq!( + mods.apply(&mage(&[5, 35]), OP_COST, 100), + (100 - 25) * 50 / 100 + ); + // Order matters — `value * pct / 100 + flat` would give 25, not 37. + assert_eq!(mods.apply(&mage(&[5, 35]), OP_COST, 100), 37); + } + + /// The division truncates **toward zero**, both signs — C's `/`, not a floor and not a round. + #[test] + fn the_division_truncates_toward_zero() { + let mut mods = mage_tables(); + // 99 × 50% = 49.5 → 49, not 50. + mods.set(false, 5, OP_COST, -50); + assert_eq!(mods.apply(&mage(&[5]), OP_COST, 99), 49); + + // The negative side: −99 × 50% = −49.5 → −49 (toward zero), where a floor gives −50. + assert_eq!(SpellMod { flat: 0, pct: 50 }.apply(-99), -49); + // And the identity really is the identity. + assert_eq!(SpellMod::default().apply(-7), -7); + } + + /// The percentage sum is clamped at 0 — a −150% modifier zeroes the value, it does not invert + /// it (`6e6bbc`'s `sets`/`dec`/`and`, on the SIGNED sum). + #[test] + fn a_percentage_below_minus_one_hundred_clamps_to_zero() { + let mut mods = mage_tables(); + mods.set(false, 5, OP_COST, -150); + assert_eq!(mods.apply(&mage(&[5]), OP_COST, 200), 0); + } + + /// Out-of-range wire bytes are dropped, not stored — and dropped without disturbing the cell + /// the reference's own overrun would have hit. + #[test] + fn an_out_of_range_packet_is_dropped() { + let mut mods = mage_tables(); + // `(64, 19)` is the reference's documented overrun into the class-family global. + mods.set(true, 64, 19, 999); + // `(65, 23)` is the one that lands on PCT element 0 there. + mods.set(true, 65, 23, 999); + mods.set(true, 0, 29, 999); + mods.set(true, 255, 255, 999); + + assert_eq!(mods.class_family, 3, "the class family is untouched"); + assert!( + mods.flat.iter().chain(mods.pct.iter()).all(|&v| v == 0), + "not one cell was written" + ); + } + + /// World-enter drops everything — both tables and the family — which is the only clear there + /// is. + #[test] + fn world_enter_clears_both_tables_and_the_family() { + // A bare app: the plugin's only Update system takes both of its inputs as an `Option`/an + // empty query, so the state machine the clear hangs off is all this needs. + let mut app = App::new(); + app.add_plugins((MinimalPlugins, bevy::state::app::StatesPlugin)) + .insert_state(ClientState::Login) + .add_plugins(SpellModsPlugin); + + { + let mut mods = app.world_mut().resource_mut::(); + mods.set_class_family(3); + mods.set(true, 5, OP_COST, -30); + mods.set(false, 35, OP_COST, -50); + assert_eq!(mods.apply(&mage(&[5]), OP_COST, 100), 70); + } + + app.world_mut() + .resource_mut::>() + .set(ClientState::InWorld); + app.update(); + + let mods = app.world().resource::(); + assert_eq!(mods.class_family, 0); + assert!(mods.flat.iter().chain(mods.pct.iter()).all(|&v| v == 0)); + } + + /// The three worked multi-bit spells from the real `Spell.dbc`, driven through the whole + /// module: the same `(bit, op)` cells the server would send, read back through each spell's + /// own shipped family mask. Cleanse 4987 is the one that matters — bits 12 **and** 33 — and it + /// fails under any read that stops at 32 bits. Skips without client data. + #[test] + fn real_spells_sum_their_own_family_bits() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let cat = benilla_formats::load_spell_catalog(&mut chain).expect("load Spell.dbc"); + + // A paladin: Cleanse 4987 is family 10, bits 12 and 33. + let mut mods = SpellModifiers::default(); + mods.set_class_family(10); + mods.set(true, 12, OP_COST, -5); + mods.set(true, 33, OP_COST, -7); + mods.set(false, 33, OP_COST, -25); + let cleanse = cat.get(4987).expect("Cleanse"); + assert_eq!( + mods.apply(cleanse, OP_COST, 100), + (100 - 12) * 75 / 100, + "both dwords contribute" + ); + // Cure Poison 526 is a SHAMAN spell (family 11), and bit 35 is the only bit it sets — so + // even with that exact cell live, the paladin's family gate refuses it. Same row, wrong + // family: conjunct 2 is what keeps one class's talents off another's spells. + mods.set(true, 35, OP_COST, -50); + assert_eq!( + mods.apply(cat.get(526).expect("Cure Poison"), OP_COST, 100), + 100 + ); + + // A mage: Frostbolt 116, bits 5/19/20/30 — four cells, summed. + let mut mods = SpellModifiers::default(); + mods.set_class_family(3); + for bit in [5u8, 19, 20, 30] { + mods.set(true, bit, OP_COST, -1); + } + assert_eq!( + mods.apply(cat.get(116).expect("Frostbolt"), OP_COST, 100), + 96, + "four set bits, four cells" + ); + } +} diff --git a/crates/benilla-app/src/target/reticle.rs b/crates/benilla-app/src/target/reticle.rs index 8309722e..7a834077 100644 --- a/crates/benilla-app/src/target/reticle.rs +++ b/crates/benilla-app/src/target/reticle.rs @@ -18,8 +18,9 @@ //! per-effect `radius + casterLevel × perLevel` over **EffectRadiusIndex[0] and [1] only** //! (slot 2 is never read), max with candidate-1 winning ties/NaN. **Out of range forces the //! radius to 0.0** — the decal shrinks to the 1.3888889 default *and* turns red. `r == 0` (no -//! radius rows — a dest spell with no area) also draws at the default. Class-6 spell modifiers -//! don't exist yet (the same residual as the range gate, 0792). +//! radius rows — a dest spell with no area) also draws at the default. Spell-mod op 6 +//! (SPELLMOD_RADIUS) is not folded in: the tables are live ([`crate::spell_mods`]), this consumer +//! is not wired to them (the same residual as the range gate, 0792). //! //! **States**: in range → Acceptable at `r`; out of range → Unacceptable at the default size; //! cursor over sky / no world hit → **nothing is drawn** (the ref resets its draw state every diff --git a/crates/benilla-app/src/ui_action/cast_send.rs b/crates/benilla-app/src/ui_action/cast_send.rs index 311fa755..39bacc0f 100644 --- a/crates/benilla-app/src/ui_action/cast_send.rs +++ b/crates/benilla-app/src/ui_action/cast_send.rs @@ -102,6 +102,9 @@ pub(crate) struct CastLadder<'w, 's> { pub(crate) pending: ResMut<'w, crate::ui_cast::PendingCast>, pub(crate) queued_melee: ResMut<'w, crate::ui_cast::QueuedMeleeSpell>, pub(crate) cooldowns: ResMut<'w, crate::cooldowns::Cooldowns>, + /// The talent spell-modifier tables — rung 2's cost goes through them + /// ([`super::usable::power_cost`]). + pub(crate) spell_mods: Res<'w, crate::spell_mods::SpellModifiers>, pub(crate) cast_errors: ResMut<'w, CastErrors>, pub(crate) auto_repeat: ResMut<'w, AutoRepeatActive>, pub(crate) trade_skill_opens: ResMut<'w, crate::ui_tradeskill::TradeSkillOpens>, @@ -242,6 +245,7 @@ impl CastLadder<'_, '_> { &mut self.pending, &mut self.queued_melee, &mut self.cooldowns, + &self.spell_mods, &mut self.cast_errors, &mut self.auto_repeat, &mut self.trade_skill_opens, @@ -282,6 +286,7 @@ fn send_spell_cast( pending: &mut crate::ui_cast::PendingCast, queued_melee: &mut crate::ui_cast::QueuedMeleeSpell, cooldowns: &mut crate::cooldowns::Cooldowns, + spell_mods: &crate::spell_mods::SpellModifiers, cast_errors: &mut CastErrors, auto_repeat: &mut AutoRepeatActive, trade_skill_opens: &mut crate::ui_tradeskill::TradeSkillOpens, @@ -520,7 +525,7 @@ fn send_spell_cast( // rage-starved spam press (the 0946 campaign's live capture of the loop). if !commit.is_item() { if let (Some(d), Some(store)) = (def, ctx.rel.self_store) { - if !super::usable::can_afford(d, store) { + if !super::usable::can_afford(d, store, spell_mods) { debug!("ui_action: cast {spell_id} refused locally — not enough power (0x4d)"); cast_errors.push_local(spell_id, 0x4d); return; @@ -860,6 +865,7 @@ mod tests { world.init_resource::(); world.init_resource::(); world.init_resource::(); + world.init_resource::(); world.init_resource::(); world.init_resource::(); world.init_resource::(); diff --git a/crates/benilla-app/src/ui_action/mod.rs b/crates/benilla-app/src/ui_action/mod.rs index 7235ffba..f24a3b75 100644 --- a/crates/benilla-app/src/ui_action/mod.rs +++ b/crates/benilla-app/src/ui_action/mod.rs @@ -211,9 +211,10 @@ impl Spells { /// `wave-cooldown.md`/`moving-cast-gate.md`, byte-verified): `CastingTimeIndex` resolves the /// [`Self::cast_times`] row, `base + perLevel·(casterLevel − baseLevel)` floors to the /// row's minimum (row 1, the all-zero instant sentinel, resolves 0). The level term keys on - /// the `SpellRec+0x70` column ([`SpellDisplay::base_level`]); spellmod op `0xa` - /// (SPELLMOD_CASTING_TIME) is unmodeled — benilla has no spellmod system — a named - /// micro-divergence (a talent-shortened 0-second cast doesn't exist in the 1.12 data). + /// the `SpellRec+0x70` column ([`SpellDisplay::base_level`]). Spell-mod op `0xa` + /// (SPELLMOD_CASTING_TIME) is still unread here — the tables themselves are live + /// ([`crate::spell_mods`]), only this consumer is not wired to them, so a talent-shortened + /// cast still shows its untalented length. /// A missing row reads 0 (instant), like a failed catalog load everywhere else. pub(crate) fn cast_time_ms( &self, diff --git a/crates/benilla-app/src/ui_action/state.rs b/crates/benilla-app/src/ui_action/state.rs index ce11dcad..088f7d33 100644 --- a/crates/benilla-app/src/ui_action/state.rs +++ b/crates/benilla-app/src/ui_action/state.rs @@ -395,13 +395,15 @@ pub(super) fn feed_action_state( // One tuple param (Bevy's 16-SystemParam ceiling): our own cast tracking — the in-flight // guard, the queued on-next-swing strike, the running channel, and the awaiting-click // ground targeting — plus the macro→spell binding the MACRO arm resolves through - // (decision 0983), which rides here for the same ceiling reason. + // (decision 0983) and the talent spell-modifier tables that leg 12's cost reads through, + // both of which ride here for the same ceiling reason. cast_state: ( Res, Res, Res, Res, Res, + Res, ), self_q: Query<(&ObjectStore, &Transform, Has, Option<&Casting>), With>, selection: Res, @@ -434,7 +436,7 @@ pub(super) fn feed_action_state( memory.last_cd_trace = Some(now); } - let (pending, queued_melee, channel, targeting, bound) = &cast_state; + let (pending, queued_melee, channel, targeting, bound, spell_mods) = &cast_state; let me = self_q.iter().next(); // The bags, walked ONCE for the frame: every reagent, totem and item-count question below // reads this table. It used to be one whole walk per question — per reagent per spell slot, @@ -531,6 +533,7 @@ pub(super) fn feed_action_state( reputations: &reputations, cooldowns: &cooldowns, carried: &carried, + spell_mods, }; let (u, oom) = usable::spell_usable(button.action, d, sp, &ctx, &items, &commands); @@ -596,6 +599,7 @@ pub(super) fn feed_action_state( reputations: &reputations, cooldowns: &cooldowns, carried: &carried, + spell_mods, }; let (u, oom) = usable::item_usable( button.action, @@ -842,6 +846,7 @@ mod tests { app.insert_resource(actions) .insert_resource(bound) .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() @@ -946,6 +951,7 @@ mod tests { radii: Default::default(), }) .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() diff --git a/crates/benilla-app/src/ui_action/targeting/cursor.rs b/crates/benilla-app/src/ui_action/targeting/cursor.rs index 1309496f..24f06b1b 100644 --- a/crates/benilla-app/src/ui_action/targeting/cursor.rs +++ b/crates/benilla-app/src/ui_action/targeting/cursor.rs @@ -45,8 +45,9 @@ fn range_row(spells: Option<&Spells>, spell_id: u32) -> Option<&SpellRange> { /// radius: per-effect `radius + casterLevel × perLevel` over **EffectRadiusIndex[0] and [1] /// only** (slot 2 is never read by the client), the max with candidate 1 winning ties/NaN, /// clamped to 20.0 (`0x4820f0`'s `[0x804478]` literal — `min`, NaN → 20). `0.0` = no radius -/// rows; the reticle then draws at its literal default size. Class-6 spell modifiers are -/// unmodelled (the 0792 residual, same as the range gate). +/// rows; the reticle then draws at its literal default size. Spell-mod op 6 (SPELLMOD_RADIUS) is +/// not read here — the tables are live ([`crate::spell_mods`]), this consumer is not wired to +/// them (the 0792 residual, same as the range gate). pub(crate) fn ground_cast_radius(spells: Option<&Spells>, spell_id: u32, level: u32) -> f32 { let Some(spells) = spells else { return 0.0 }; let Some(d) = spells.catalog.get(spell_id) else { diff --git a/crates/benilla-app/src/ui_action/targeting/world.rs b/crates/benilla-app/src/ui_action/targeting/world.rs index 190941a2..32aa41df 100644 --- a/crates/benilla-app/src/ui_action/targeting/world.rs +++ b/crates/benilla-app/src/ui_action/targeting/world.rs @@ -177,6 +177,7 @@ mod tests { world.init_resource::(); world.init_resource::(); world.init_resource::(); + world.init_resource::(); world.init_resource::(); world.init_resource::(); world.init_resource::(); diff --git a/crates/benilla-app/src/ui_action/usable.rs b/crates/benilla-app/src/ui_action/usable.rs index 5598ff61..c6889b16 100644 --- a/crates/benilla-app/src/ui_action/usable.rs +++ b/crates/benilla-app/src/ui_action/usable.rs @@ -29,6 +29,7 @@ use benilla_formats::{ use crate::cooldowns::Cooldowns; use crate::items::Items; use crate::net::{NetCommands, ObjectStore, Reputations}; +use crate::spell_mods::{SpellModifiers, OP_COST}; use crate::target::{can_attack, ring_reaction, Factions}; use super::Spells; @@ -49,6 +50,8 @@ pub(crate) struct UsableCtx<'a> { pub(crate) factions: Option<&'a Factions>, pub(crate) reputations: &'a Reputations, pub(crate) cooldowns: &'a Cooldowns, + /// The talent spell-modifier tables — leg 12's cost goes through them ([`power_cost`]). + pub(crate) spell_mods: &'a SpellModifiers, /// Every carried entry's count, walked ONCE by the caller for the frame /// ([`crate::ui_items::carried_counts`]) — leg 3 reads reagents and totems off it instead /// of walking the bags per reagent per slot. @@ -450,7 +453,7 @@ pub(crate) fn spell_usable( return (false, false); } // Leg 12 (`0x6e3fba`–`0x6e3feb`): the power gate — the SOLE notEnoughMana writer (B2). - if !can_afford(d, ctx.store) { + if !can_afford(d, ctx.store, ctx.spell_mods) { return (false, true); } (true, false) @@ -461,12 +464,17 @@ pub(crate) fn spell_usable( /// `CalculatePowerCost` shape, clamped at 0 on the way out; every nonzero row is a creature /// spell, so the clamp choice is dormant for players), and `ManaCostPercentage` of the pool /// 0948 chose per type (base mana for mana spells — the vmangos basis — max pool otherwise, -/// health included for negative types). One law, every consumer: the usable walk's leg 12, the -/// press-path power gate (0948), and the tooltip's cost cell (1074) — mirroring the byte fn's -/// own caller set (`0x609657`/`0x60968d`/`0x4e5201`/`0x6e3fdb`/`0x52e8ad`/`0x507de3`). The -/// reference's per-school unit mods and spell-mods (talent cost cuts) are not modeled — 0948's -/// standing gap, now stated once here. -pub(crate) fn power_cost(d: &SpellDisplay, store: &ObjectStore) -> u32 { +/// health included for negative types) — and then the talent cost cut, `SPELLMOD_COST` (op 14), +/// which `0x6e31b0` applies inside itself at `6e32e3` through the integer applier `0x6e6af0` +/// ([`crate::spell_mods`]). One law, every consumer: the usable walk's leg 12, the press-path +/// power gate (0948), and the tooltip's cost cell (1074) — mirroring the byte fn's own caller +/// set (`0x609657`/`0x60968d`/`0x4e5201`/`0x6e3fdb`/`0x52e8ad`/`0x507de3`). The reference's +/// per-school unit mods are still not modeled — what remains of 0948's standing gap. +/// +/// **The modifier is why this one is behavioural and not only cosmetic**: the press-path gate +/// refuses a cast it judges unaffordable and sends **no packet at all** (`0x609657`'s failure +/// block), so an unmodified cost makes benilla refuse casts a talented character can afford. +pub(crate) fn power_cost(d: &SpellDisplay, store: &ObjectStore, mods: &SpellModifiers) -> u32 { let power_type = d.power_type as i32; let base = if d.mana_cost_pct == 0 { 0 @@ -484,7 +492,11 @@ pub(crate) fn power_cost(d: &SpellDisplay, store: &ObjectStore) -> u32 { let cost = i64::from(d.mana_cost) + level_delta * i64::from(d.mana_cost_per_level) + i64::from(base) * i64::from(d.mana_cost_pct) / 100; - u32::try_from(cost).unwrap_or(0) + // The talent cut, on the data terms and before the 0-clamp on the way out. The relative order + // of the two is dormant for players — every nonzero `manaCostPerlevel` row is a creature spell + // (above), so nothing reaches the modifier negative. + let cost = cost.clamp(i64::from(i32::MIN), i64::from(i32::MAX)) as i32; + u32::try_from(mods.apply(d, OP_COST, cost)).unwrap_or(0) } /// Whether the caster can afford `d`'s power cost — the shared availability-vs-cost compare @@ -492,14 +504,14 @@ pub(crate) fn power_cost(d: &SpellDisplay, store: &ObjectStore) -> u32 { /// (`0x6094f0` @ `0x60962c`, decision 0948): raw `UNIT_FIELD_POWER[type]` — ANY negative /// PowerType reads `UNIT_FIELD_HEALTH` instead (the ref's `jl` at `0x609631`; Bloodrage's −2) — /// signed-compared against [`power_cost`]'s number. -pub(crate) fn can_afford(d: &SpellDisplay, store: &ObjectStore) -> bool { +pub(crate) fn can_afford(d: &SpellDisplay, store: &ObjectStore, mods: &SpellModifiers) -> bool { let power_type = d.power_type as i32; let avail = if power_type < 0 { store.0.unit_health().unwrap_or(0) } else { store.0.unit_power(power_type as u8).unwrap_or(0) }; - avail >= power_cost(d, store) + avail >= power_cost(d, store, mods) } #[cfg(test)] @@ -716,6 +728,7 @@ mod tests { cooldowns: &'a Cooldowns, reputations: &'a Reputations, carried: &'a std::collections::HashMap, + spell_mods: &'a SpellModifiers, ) -> UsableCtx<'a> { UsableCtx { store, @@ -724,6 +737,7 @@ mod tests { reputations, cooldowns, carried, + spell_mods, } } @@ -735,16 +749,64 @@ mod tests { let (tx, _rx) = crossbeam_channel::unbounded(); let commands = NetCommands(tx); let carried = crate::ui_items::carried_counts(&store.0, &items); + let spell_mods = SpellModifiers::default(); spell_usable( 1, d, &spells, - &ctx(store, &cooldowns, &reputations, &carried), + &ctx(store, &cooldowns, &reputations, &carried, &spell_mods), &items, &commands, ) } + /// **The behavioural half of the spell-modifier system, and the reason op 14 is the first op + /// wired** (wow-re `spellmod-table-law.md` §9): the press-path gate compares the caster's + /// power against [`power_cost`] and, when short, refuses locally and sends **no** + /// `CMSG_CAST_SPELL` at all (`0x609657`'s fall-through failure block). With the tables unread, + /// a talent-discounted spell the server would happily accept is refused by the client. + /// + /// So: a mage with 80 mana and a 100-mana spell cannot cast it; one `SMSG_SET_PCT_SPELL_MODIFIER` + /// for that spell's `(family bit, op 14)` cell flips both the number and the verdict. + #[test] + fn a_talent_cost_cut_reaches_the_cost_and_the_press_path_gate() { + // Family 3 (mage), family bit 5 — the shape a real talent's `SpellFamilyMask` names. + let d = SpellDisplay { + spell_family: 3, + spell_family_flags: 1 << 5, + mana_cost: 100, + ..Default::default() + }; + let store = ObjectStore(ObjectFields::from_pairs(&[(23, 80)])); + + let mut mods = SpellModifiers::default(); + mods.set_class_family(3); + assert_eq!( + power_cost(&d, &store, &mods), + 100, + "no packet yet — the flat DBC cost" + ); + assert!( + !can_afford(&d, &store, &mods), + "80 mana does not buy a 100 mana spell" + ); + + // The server's cell for (bit 5, op 14): −30%. + mods.set(false, 5, OP_COST, -30); + assert_eq!(power_cost(&d, &store, &mods), 70); + assert!( + can_afford(&d, &store, &mods), + "the talent brings it inside the pool, so TryCast must send the cast" + ); + + // A warrior's copy of the same spell id takes nothing — the family gate, from this side. + let other_class = SpellDisplay { + spell_family: 4, + ..d + }; + assert_eq!(power_cost(&other_class, &store, &mods), 100); + } + /// Each modeled gate trips alone, and only the power leg raises notEnoughMana (B2). #[test] fn gates_trip_independently_and_only_power_sets_oom() { @@ -880,6 +942,7 @@ mod tests { reputations: &reputations, cooldowns: &cooldowns, carried: &carried, + spell_mods: &SpellModifiers::default(), }; assert_eq!( spell_usable(5308, &execute, &spells, &ctx, &items, &commands), diff --git a/crates/benilla-app/src/ui_dialog_verbs.rs b/crates/benilla-app/src/ui_dialog_verbs.rs index 73e33b9f..30b6784a 100644 --- a/crates/benilla-app/src/ui_dialog_verbs.rs +++ b/crates/benilla-app/src/ui_dialog_verbs.rs @@ -14,9 +14,11 @@ //! * **Area spirit healer** — `SMSG_AREA_SPIRIT_HEALER_TIME {guid, ms}` arms the wave clock and //! fires `AREA_SPIRIT_HEALER_IN_RANGE` when the guid is the cached healer's; Accept sends //! `0x2E3` with that guid, Cancel is the cancel-aura of spell 2584 plus `_OUT_OF_RANGE`. **The -//! cache has no writer yet**: the reference's per-frame proximity scan (20 yd to acquire, 22 to -//! retain, `0x4923b0`) is not carved for its unit filter, so no healer is ever cached here and -//! Accept stays silent — named in the record, not guessed at. +//! cache has two writers** (2291), which is what made Accept reachable at all: the reference's +//! per-frame proximity scan [`poll_area_spirit_healer`] (`0x4923b0` — ghost-gated, acquire at +//! 20 yd, retain to 22) and the SPIRITGUIDE click arm [`AreaSpiritHealer::click_guide`] +//! (`0x5df950`, whose deliberate cache-bust makes a second click re-ask). Until 2291 this said +//! "no writer yet", which was true when 1963 wrote it and false from the moment the scan landed. //! * **Battlefield queue** — `SMSG_BATTLEFIELD_STATUS` fills one of three slots and fires //! `UPDATE_BATTLEFIELD_STATUS`; `AcceptBattlefieldPort(index, accept)` sends the slot's map id //! with the answer as one byte. @@ -792,6 +794,24 @@ fn meeting_stone_leave_world(mut stone: ResMut) { stone.leave_world(); } +/// Drop the cached spirit guide when the world does. +/// +/// **A resurrect wave is the most session-bound state this module holds**, and it was the one +/// resource here with no leave-world leg. A ghost who logs out at a Warsong Gulch graveyard with a +/// guide latched and a wave armed carried `healer`, `deadline` and `in_range` into the character +/// screen and into the *next* character's session — where, on the first frame the interface came +/// up, [`feed_dialog_verbs`] pushed the stale healer into the fresh VM and could fire +/// `AREA_SPIRIT_HEALER_IN_RANGE` at a living body standing in Stormwind, and the poll's ghost gate +/// then cleared it and sent a `CMSG_CANCEL_AURA(2584)` the reference never sends there. +/// +/// The reference does not need this leg because its equivalents are process-lifetime globals whose +/// poll keeps running over the character screen's frames; ours is a resource in a world that goes +/// away. Resetting at the seam is how every other per-session cache here behaves +/// (`crate::ui_aura::end_session_aura_state` is the established shape). +fn area_spirit_healer_leave_world(mut spirit: ResMut) { + *spirit = AreaSpiritHealer::default(); +} + pub(crate) fn feed_dialog_verbs( script: Option>, mut pet: ResMut, @@ -1116,7 +1136,30 @@ impl Plugin for UiDialogVerbsPlugin { .before(feed_meeting_stone) .in_set(crate::char_select::InWorldGated), feed_meeting_stone.before(UiInput), - drain_latch_verbs.after(UiInput), + // **Before the target chain, not merely after the input pass** — and this one + // is a correctness order, not a tidiness one. + // + // `drain_latch_verbs` takes `AcceptAreaSpiritHeal`'s presses with + // `take_area_spirit_accepts()` **unconditionally**, and only then gates the + // send on `spirit.healer()`. Both systems that can *clear* that healer — + // `poll_area_spirit_healer` (which drops it past the 22 yd retain radius) and + // `act_on_right_click` (which re-points it at another guide) — live in or + // after `TargetUpdate`, and `.after(UiInput)` alone constrains this against + // neither: `UiInput` sits *before* `WorldStage::Input` and `TargetUpdate` + // after it. So with the order undeclared, a ghost who clicks **Accept** on the + // same frame they step out of range either sends + // `CMSG_AREA_SPIRIT_HEALER_QUEUE` or has the press silently eaten — decided by + // graph layout, with no message either way. + // + // `before` is the reference's own answer, not a coin toss: the dialog's Lua + // handler runs inside the UI dispatch, while `0x4923b0` (the poll) and + // `0x48184a` (the pick) both run later in the same frame off `CGWorldFrame`'s + // OnUpdate. The press is resolved against the healer the frame *began* with. + // Its neighbour `drain_meeting_stone_joins` below argued the same hazard and + // declared its way out of it; this one was missed. + drain_latch_verbs + .after(UiInput) + .before(crate::target::TargetUpdate), drain_queue_verbs.after(UiInput), // MEETINGSTONE(23)'s use slot (2283): the click resolved the object, this // runs the validator and sends. Ordered after the **target chain**, not just @@ -1132,7 +1175,7 @@ impl Plugin for UiDialogVerbsPlugin { ) .add_systems( OnExit(crate::char_select::ClientState::InWorld), - meeting_stone_leave_world, + (meeting_stone_leave_world, area_spirit_healer_leave_world), ); } } @@ -1265,14 +1308,204 @@ mod tests { assert_eq!(spirit.healer(), Some(7)); } - /// The two radii are the reference's `.rdata` f32s, not round numbers someone picked: - /// `[0x8044d0] = 20.0` to acquire and `× [0x804580] = 1.1` to retain. The hysteresis is the - /// point — equal radii would re-send a query every frame a body jittered across the boundary. + /// `UNIT_FIELD_FLAGS`, absolute field 46 — `UNIT_FLAG_PVP` (`0x1000`) is what carries a + /// non-player-controlled unit through `can_assist`'s last gate. + const UNIT_FLAGS_FIELD: u16 = 46; + /// `UNIT_NPC_FLAGS`, absolute field 147. + const NPC_FLAGS_FIELD: u16 = 147; + /// `PLAYER_FLAGS`, absolute field 190; bit `0x10` is GHOST. + const PLAYER_FLAGS_FIELD: u16 = 190; + + fn fields(pairs: &[(u16, u32)]) -> ObjectStore { + ObjectStore(benilla_protocol::ObjectFields::from_pairs(pairs)) + } + + /// A world holding exactly what [`poll_area_spirit_healer`] reads, with the body at the origin + /// and one candidate unit at `dist` yards along +X. + /// + /// `ghost` drives `PLAYER_FLAGS 0x10`; `guide` drives `UNIT_NPC_FLAGS` bit 6. The candidate + /// carries `UNIT_FLAG_PVP` and faction template **35** ("friendly to all"), against a body on + /// template **1** (PLAYER, Human) — a real friendly pair out of the shipped DBC, so + /// `can_assist` passes for the same reason it passes on the live server rather than by + /// accident. A test that wants the acquire walk to *run* must therefore hand over the real + /// catalog; `None` serves the cases that refuse **before** the walk (not a ghost, not a guide, + /// too far), which is why those need no client data and never skip. + fn poll_world( + ghost: bool, + guide: bool, + dist: f32, + factions: Option, + ) -> (World, crossbeam_channel::Receiver) { + let (tx, rx) = crossbeam_channel::unbounded::(); + let mut world = World::new(); + world.insert_resource(NetCommands(tx)); + world.init_resource::(); + world.init_resource::(); + world.init_resource::(); + if let Some(catalog) = factions { + world.insert_resource(crate::target::Factions::from_catalog(catalog)); + } + world.spawn(( + SelfPlayer, + fields(&[ + (PLAYER_FLAGS_FIELD, if ghost { 0x10 } else { 0 }), + (FACTION_TEMPLATE_FIELD, 1), + ]), + Transform::from_xyz(0.0, 0.0, 0.0), + )); + world.spawn(( + crate::net::Guid(GUIDE), + crate::net::NetEntity { + kind: benilla_protocol::EntityKind::Unit, + display_id: None, + scale: 1.0, + }, + fields(&[ + (NPC_FLAGS_FIELD, if guide { 1 << 6 } else { 0 }), + (UNIT_FLAGS_FIELD, 0x1000), + (FACTION_TEMPLATE_FIELD, 35), + ]), + Transform::from_xyz(dist, 0.0, 0.0), + )); + (world, rx) + } + + /// `UNIT_FIELD_FACTIONTEMPLATE`, absolute field 35. + const FACTION_TEMPLATE_FIELD: u16 = 35; + + /// The shipped FactionTemplate.dbc, or an early return when this machine has no client data — + /// the same `wow_data_or_skip!` shape `target::ring`'s own reaction tests use. + macro_rules! catalog_or_skip { + () => {{ + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + benilla_formats::load_faction_catalog(&mut chain).expect("FactionTemplate.dbc") + }}; + } + + const GUIDE: u64 = 0x9111; + + fn run_poll(world: &mut World) { + use bevy::ecs::system::RunSystemOnce; + world.run_system_once(poll_area_spirit_healer).unwrap(); + } + + fn queries(rx: &crossbeam_channel::Receiver) -> Vec { + rx.try_iter() + .filter_map(|c| match c { + ClientCommand::AreaSpiritHealerQuery { healer } => Some(healer), + _ => None, + }) + .collect() + } + + /// **The ghost gate is the poll's first test** (`0x4923b0`, the `0x5df74a`-shaped read of + /// `[[player+0xe68]+8] bit 4`). A living body standing on top of a spirit guide adopts nothing + /// and sends nothing — which is what keeps the resurrect dialog off the screen of everyone who + /// is merely walking through a graveyard. #[test] - fn the_acquire_and_retain_radii_are_the_reference_pair() { - assert!((SPIRIT_GUIDE_ACQUIRE_YD - 20.0).abs() < f32::EPSILON); - assert!((SPIRIT_GUIDE_RETAIN_YD - 22.0).abs() < 1e-5); - const { assert!(SPIRIT_GUIDE_RETAIN_YD > SPIRIT_GUIDE_ACQUIRE_YD) }; + fn a_living_body_never_adopts_a_spirit_guide() { + let (mut world, rx) = poll_world(false, true, 1.0, None); + run_poll(&mut world); + assert_eq!(world.resource::().healer(), None); + assert!(queries(&rx).is_empty(), "no clock is asked for"); + } + + /// The acquire leg: a ghost inside 20 yd adopts, and the adopt is what sends + /// `CMSG_AREA_SPIRIT_HEALER_QUERY` — the packet that was unreachable before 2291. + #[test] + fn a_ghost_adopts_a_guide_inside_the_acquire_radius_and_asks_for_the_clock() { + let catalog = catalog_or_skip!(); + let (mut world, rx) = poll_world(true, true, 19.0, Some(catalog)); + run_poll(&mut world); + assert_eq!(world.resource::().healer(), Some(GUIDE)); + assert_eq!( + queries(&rx), + vec![GUIDE], + "exactly one query, for that guide" + ); + } + + /// …and 20 yd is a real boundary, not decoration: one yard further and nothing is adopted. + #[test] + fn a_guide_past_the_acquire_radius_is_not_adopted() { + let (mut world, rx) = poll_world(true, true, 21.0, None); + run_poll(&mut world); + assert_eq!(world.resource::().healer(), None); + assert!(queries(&rx).is_empty()); + } + + /// **The hysteresis, which is the whole reason there are two radii.** A guide adopted at 19 yd + /// is RETAINED at 21 — past the acquire radius — and only dropped past 22 + /// (`20.0 × 1.1`). Equal radii would re-query every frame a body jittered over the boundary. + /// The drop is not silent: it cancels the wave (`CMSG_CANCEL_AURA` on 2584), which on vmangos + /// takes the player out of the resurrect queue. + #[test] + fn an_adopted_guide_is_retained_past_the_acquire_radius_and_dropped_past_the_retain_one() { + let catalog = catalog_or_skip!(); + let (mut world, rx) = poll_world(true, true, 19.0, Some(catalog)); + run_poll(&mut world); + assert_eq!(world.resource::().healer(), Some(GUIDE)); + let _ = queries(&rx); + + // 21 yd: outside acquire, inside retain — kept, and no second query. + let guide = world + .query_filtered::>() + .iter(&world) + .next() + .expect("the guide entity"); + world + .entity_mut(guide) + .insert(Transform::from_xyz(21.0, 0.0, 0.0)); + run_poll(&mut world); + assert_eq!( + world.resource::().healer(), + Some(GUIDE), + "retained between the two radii" + ); + assert!(queries(&rx).is_empty(), "a retained guide is not re-asked"); + + // 23 yd: outside both — dropped. + world + .entity_mut(guide) + .insert(Transform::from_xyz(23.0, 0.0, 0.0)); + run_poll(&mut world); + assert_eq!(world.resource::().healer(), None); + } + + /// The unit filter is the `SPIRITGUIDE` bit and nothing else. A unit standing in the same spot + /// without it — a battle master, a herald, another player's corpse-side NPC — is skipped. + #[test] + fn a_unit_without_the_spiritguide_flag_is_never_adopted() { + let (mut world, rx) = poll_world(true, false, 1.0, None); + run_poll(&mut world); + assert_eq!(world.resource::().healer(), None); + assert!(queries(&rx).is_empty()); + } + + /// Resurrecting drops the guide. The poll is **level**-triggered, so the release happens on the + /// first frame the ghost flag clears — with the guide still standing right there. + #[test] + fn losing_the_ghost_state_drops_the_guide() { + let catalog = catalog_or_skip!(); + let (mut world, _rx) = poll_world(true, true, 5.0, Some(catalog)); + run_poll(&mut world); + assert_eq!(world.resource::().healer(), Some(GUIDE)); + + let me = world + .query_filtered::>() + .iter(&world) + .next() + .expect("the body"); + world + .entity_mut(me) + .insert(fields(&[(PLAYER_FLAGS_FIELD, 0)])); + run_poll(&mut world); + assert_eq!( + world.resource::().healer(), + None, + "alive again: the cache clears even though the guide has not moved" + ); } /// The four client-side refusals of MEETINGSTONE(23)'s use slot (`0x5f69d0`, decision 2283), diff --git a/crates/benilla-app/src/ui_items/drain.rs b/crates/benilla-app/src/ui_items/drain.rs index f1feee88..9d949ae6 100644 --- a/crates/benilla-app/src/ui_items/drain.rs +++ b/crates/benilla-app/src/ui_items/drain.rs @@ -906,6 +906,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() @@ -1654,6 +1655,7 @@ mod bind_confirm_tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() diff --git a/crates/benilla-app/src/ui_shapeshift.rs b/crates/benilla-app/src/ui_shapeshift.rs index dbf8e7a7..33cd644e 100644 --- a/crates/benilla-app/src/ui_shapeshift.rs +++ b/crates/benilla-app/src/ui_shapeshift.rs @@ -240,6 +240,7 @@ fn feed_shapeshift_bar( items: Res, commands: Res, clock: Res, + spell_mods: Res, mut memory: Local>, ) { let Some(mut script) = script else { @@ -302,6 +303,7 @@ fn feed_shapeshift_bar( reputations: &reputations, cooldowns: &cooldowns, carried: &carried, + spell_mods: &spell_mods, }; usable::spell_usable(id, d, &spells, &ctx, &items, &commands).0 }); diff --git a/crates/benilla-app/src/ui_tooltip/mod.rs b/crates/benilla-app/src/ui_tooltip/mod.rs index 09e23455..a2faaa38 100644 --- a/crates/benilla-app/src/ui_tooltip/mod.rs +++ b/crates/benilla-app/src/ui_tooltip/mod.rs @@ -60,6 +60,9 @@ struct ViewCtx<'a> { items: &'a mut Items, commands: &'a NetCommands, sub_classes: Option<&'a benilla_formats::ItemSubClassCatalog>, + /// The talent spell-modifier tables — the cost cell shows the RESOLVED cost, which since + /// `SPELLMOD_COST` landed means the modified one (`crate::ui_action::usable::power_cost`). + spell_mods: &'a crate::spell_mods::SpellModifiers, /// The VM's own `GlobalStrings.lua` (decision 2045) — every cell this builder composes is a /// key, and this is where they resolve. `text` is the `%d`-filling twin the `$`-engine's /// keyed tokens take (`benilla_formats::TokenContext::text`). @@ -118,9 +121,9 @@ fn spell_tooltip_view( // replaces was unfaithful (B152). No store (a DBC-only view) degrades to the flat cost. // HAPPINESS_COST has no GlobalStrings entry and no 5875 player spell reaches powerType 4; // the unit word is a dead arm kept for the array's shape. - let resolved_cost = vctx - .store - .map_or(d.mana_cost, |s| crate::ui_action::usable::power_cost(d, s)); + let resolved_cost = vctx.store.map_or(d.mana_cost, |s| { + crate::ui_action::usable::power_cost(d, s, vctx.spell_mods) + }); let cost = { // The one `0x6e7130` table, not a local `if power_type == 1` — decision 2117 found three // hand-rolled copies of it and one of them had been applied at a single site out of four. @@ -437,10 +440,17 @@ fn feed_spell_tooltips( home_bind: Option>, area_names: Option>, mut items: ResMut, - sub_classes: Option>, + // One tuple param (Bevy's 16-SystemParam ceiling): the two lookups the view builder reads + // straight through — the item sub-class names for the required-item line, and the talent + // spell-modifier tables the cost cell resolves through. + lookups: ( + Option>, + Res, + ), commands: Res, mut memory: Local>, ) { + let (sub_classes, spell_mods) = &lookups; let Some(mut script) = script else { return; }; @@ -566,6 +576,14 @@ fn feed_spell_tooltips( memory.combat_reach = Some(reaches); wanted.extend(memory.pushed.drain()); } + // …and so does the cost cell, whose resolved number now goes through the talent + // spell-modifier tables: a respec changes what every affected spell costs, and this feed is + // the one consumer of `power_cost` that memoizes its build (the action bar recomputes every + // frame). The reference has no cache here at all — it reads the tables live at every call + // site — so this is what keeps the cell honest about the same change (`crate::spell_mods`). + if spell_mods.is_changed() { + wanted.extend(memory.pushed.drain()); + } let watched: Vec = memory.reagents.keys().copied().collect(); let reagent_state: std::collections::BTreeMap = watched .into_iter() @@ -597,6 +615,7 @@ fn feed_spell_tooltips( items: &mut items, commands: &commands, sub_classes: sub_classes.as_deref().map(|c| &c.0), + spell_mods, get: &get, text: &text, }; diff --git a/crates/benilla-app/src/ui_tooltip/tests.rs b/crates/benilla-app/src/ui_tooltip/tests.rs index 808dfe8c..d19a2e01 100644 --- a/crates/benilla-app/src/ui_tooltip/tests.rs +++ b/crates/benilla-app/src/ui_tooltip/tests.rs @@ -19,6 +19,9 @@ struct TestCtx { /// glue tests onto the loader's own assembly. get: Box, text: Box, + /// Empty tables, which is the un-talented character every cell here is graded as: the cost + /// cell's modifier hop must be the identity when nothing has been sent. + spell_mods: crate::spell_mods::SpellModifiers, } /// The two lookup shapes, named so the harness's fields read. @@ -36,6 +39,7 @@ impl TestCtx { commands: NetCommands(tx), _rx: rx, get: Box::new(move |key| benilla_ui::strings::global(for_get.lua(), key)), + spell_mods: crate::spell_mods::SpellModifiers::default(), text: Box::new(move |key, args: &[i64]| { let template = benilla_ui::strings::global(for_text.lua(), key)?; let args: Vec<_> = args @@ -85,6 +89,7 @@ impl TestCtx { items: &mut self.items, commands: &self.commands, sub_classes, + spell_mods: &self.spell_mods, get: self.get.as_ref(), text: self.text.as_ref(), } diff --git a/crates/benilla-app/src/ui_world_map.rs b/crates/benilla-app/src/ui_world_map.rs index 8015167d..9cee6983 100644 --- a/crates/benilla-app/src/ui_world_map.rs +++ b/crates/benilla-app/src/ui_world_map.rs @@ -10,14 +10,20 @@ //! `Map.dbc` localized names ("Eastern Kingdoms", not the art folder's "Azeroth"); zones //! sorted case-insensitively by AreaTable localized name (the `0x4a6390` comparator's //! `SStrCmpI`); the zone grids remapped from raw AreaTable ids by the client's one-hop parent -//! rollup + (mapId, areaId) match — here straight to 1-based zone indices. +//! rollup + (mapId, areaId) match — here straight to 1-based zone indices. Beside that it +//! builds the **orphan list** ([`DirectAreaEntry`]) — the instance maps, which belong to no +//! continent and so appear in neither list above; the reference fills the same second container +//! in the same walk (`0x4a5d00`), and it is what the `−2` direct-area selection resolves +//! against. //! - [`feed_world_map`] (every frame): reads the engine-owned selection back, projects the //! player's world position onto the displayed map via [`benilla_world::map_proj`] (world sheet: -//! WorldMapContinent constants; continent/zone: the WorldMapArea rect lerp), resolves the -//! player's `(continent, zone)` from `CurrentArea` through the AreaTable parent chain, and -//! pushes the trio + facing. (The client matches its zone-level area global directly — -//! `0x4a6650`; our MCNK `CurrentArea` is the leaf sub-area, so the parent walk lands on the -//! same zone.) It also runs the **landmark pass** — the reference's `0x4a67a0` builder +//! WorldMapContinent constants; continent/zone: the WorldMapArea rect lerp; an instance map: +//! that row's own rect), resolves where the player's own position puts the selection +//! ([`resolve_player_selection`], the reference's `0x4a6650` — continent × zone, then the +//! orphan list) from `CurrentArea` through the AreaTable parent chain, and pushes the trio + +//! facing. (The client matches its zone-level area global directly; our MCNK `CurrentArea` is +//! the leaf sub-area, so the parent walk lands on the same zone.) It also runs the **landmark +//! pass** — the reference's `0x4a67a0` builder //! ([`landmark_gates_pass`], decision 1586): the `AreaPOI.dbc` rows the displayed level admits, //! then the guard-directions marker. That pass is keyed rather than per-frame ([`LandmarkKey`]), //! because the reference rebuilds it on events, not on a clock. @@ -50,6 +56,11 @@ use benilla_world::world_map::CurrentMap; #[derive(Resource)] pub(crate) struct WorldMapUiData { continents: Vec, + /// The **orphan list** — the instance maps, which are no continent's child and so appear in + /// neither list above. `0x4a5d00`'s third array, in `WorldMapArea.dbc` file order with no + /// sort; the rows the `−2` direct-area selection resolves against (see [`DirectAreaEntry`]). + /// It also carries what the engine is pushed, so the two copies cannot drift apart. + direct: Vec, } pub(crate) struct ContinentEntry { @@ -64,6 +75,24 @@ struct ZoneEntry { rect: ZoneRect, } +/// One orphan row: an instance map (a battleground, a dungeon) selected **directly** rather than +/// through the continent/zone pair. In 5875 there are exactly three, all battlegrounds. +struct DirectAreaEntry { + /// The `WorldMapArea` row **ID** — the value the reference's third selection cell + /// `[0x845074]` holds verbatim (`0x4a6717 mov edx,[esi]`). Not a position in this list: + /// nothing indexes it, the resolver looks rows up by id. + id: u32, + /// The row's `Map.dbc` id. The resolver matches the player's own map against this + /// (`0x4a66f6`), and the projection admits a body only when its map is this one + /// (`0x4a7437`) — which is what makes a teammate elsewhere answer the (0,0) hide sentinel. + map_id: u32, + /// The row's own loc rect — the window `0x4a7360`'s direct-area leg projects through, the + /// same tail the zone case uses. + rect: ZoneRect, + /// What the engine is handed for this row (`GetMapInfo`'s art folder, the overlays). + view: WorldMapZoneView, +} + fn zone_rect(a: &WorldMapArea) -> ZoneRect { ZoneRect { left: a.loc_left, @@ -73,6 +102,55 @@ fn zone_rect(a: &WorldMapArea) -> ZoneRect { } } +/// One `WorldMapArea` row as the engine sees it — a zone, a city, or an instance map: the client +/// makes no structural distinction (the same `0x4a6cf0` id lookup answers `GetMapInfo` for all +/// three, and the same overlay gate reveals their art), so neither do we. +/// +/// `name` is passed in because the zone list sorts on it before the views are built. +fn map_row_view( + wma_id: u32, + a: &WorldMapArea, + name: String, + areas: &benilla_formats::AreaTableCatalog, + wmo: &benilla_formats::WorldMapOverlayCatalog, +) -> WorldMapZoneView { + WorldMapZoneView { + name, + area_id: a.area_id, + map_file: a.name.clone(), + loc_rect: (a.loc_left, a.loc_right, a.loc_top, a.loc_bottom), + // The row's discovery overlays: WorldMapOverlay rows joined by its WMA id, their reveal + // bits = each covered area's AreaTable exploreFlag. + overlays: wmo + .for_area(wma_id) + .iter() + .map(|o| WorldMapOverlayView { + texture: format!("Interface\\WorldMap\\{}\\{}", a.name, o.texture_name), + width: o.texture_width, + height: o.texture_height, + offset_x: o.offset_x, + offset_y: o.offset_y, + explore_bits: o + .area_id + .iter() + .filter(|&&aid| aid != 0) + .filter_map(|&aid| areas.get(aid).map(|r| r.explore_flag)) + .collect(), + hit_rect: ( + o.hit_rect_top, + o.hit_rect_left, + o.hit_rect_bottom, + o.hit_rect_right, + ), + // The zone-level hover's label inside that rect: the FIRST area slot's AreaTable + // name — `0x4a7fa0` reads `+0x8` only (wow-re 15b2a8ea §1d); a slot that resolves + // to no row makes the overlay invisible to the hover, never to the draw. + area_name: areas.name(o.area_id[0]).map(str::to_string), + }) + .collect(), + } +} + /// Build the catalog off the patch chain — the pure half of [`seed_world_map_catalog`], split out /// so a test can drive the REAL `WorldMapArea` × `AreaTable` × `WorldMapContinent` × `Map` × /// `.zmp` build rather than a hand-written stand-in. A fixture catalog can only ever prove the @@ -84,7 +162,7 @@ pub(crate) fn build_catalog( chain: &mut benilla_formats::Chain, areas: &benilla_formats::AreaTableCatalog, maps: &MapCatalogRes, -) -> Option<(Vec, Vec)> { +) -> Option<(Vec, WorldMapUiData)> { let loaded = load_world_map_area_catalog(&mut *chain).and_then(|wma| { let wmc = load_world_map_continent_catalog(&mut *chain)?; let wmo = load_world_map_overlay_catalog(&mut *chain)?; @@ -184,42 +262,7 @@ pub(crate) fn build_catalog( zone_grid, zones: zones .iter() - .map(|(wma_id, a, name)| WorldMapZoneView { - name: name.clone(), - area_id: a.area_id, - map_file: a.name.clone(), - loc_rect: (a.loc_left, a.loc_right, a.loc_top, a.loc_bottom), - // The zone's discovery overlays: WorldMapOverlay rows joined by the zone's - // WMA id, their reveal bits = each covered area's AreaTable exploreFlag. - overlays: wmo - .for_area(*wma_id) - .iter() - .map(|o| WorldMapOverlayView { - texture: format!("Interface\\WorldMap\\{}\\{}", a.name, o.texture_name), - width: o.texture_width, - height: o.texture_height, - offset_x: o.offset_x, - offset_y: o.offset_y, - explore_bits: o - .area_id - .iter() - .filter(|&&aid| aid != 0) - .filter_map(|&aid| areas.get(aid).map(|r| r.explore_flag)) - .collect(), - hit_rect: ( - o.hit_rect_top, - o.hit_rect_left, - o.hit_rect_bottom, - o.hit_rect_right, - ), - // The zone-level hover's label inside that rect: the FIRST area - // slot's AreaTable name — `0x4a7fa0` reads `+0x8` only (wow-re - // 15b2a8ea §1d); a slot that resolves to no row makes the overlay - // invisible to the hover, never to the draw. - area_name: areas.name(o.area_id[0]).map(str::to_string), - }) - .collect(), - }) + .map(|(wma_id, a, name)| map_row_view(*wma_id, a, name.clone(), areas, &wmo)) .collect(), }); entries.push(ContinentEntry { @@ -235,7 +278,43 @@ pub(crate) fn build_catalog( .collect(), }); } - Some((views, entries)) + + // ── The THIRD list: the orphans — the instance maps (wow-re + // `system/ui/scratch/worldmap-direct-area-selection.md` §2). `0x4a5d00` fills a container + // beside the continents with every row that has `areaID != 0` and whose mapID matches **no** + // continent record's mapID, in `WorldMapArea.dbc` **file order with no sort** — the passes at + // `0x4a6130` (count) and `0x4a61c9` (fill), whose predicate is the exact complement of the + // child rule above. In 5875 that is Alterac Valley (WMA 401, map 30), Warsong Gulch (443, + // 489) and Arathi Basin (461, 529), and nothing else. + // + // The `areaID != 0` gate is carried although it is **provably inert on the shipped data**: + // every `areaID == 0` row becomes a continent record and so matches its own mapID at + // `0x4a6170`. It is the reference's predicate, not a filter we inferred from the data. + let continent_maps: Vec = entries.iter().map(|c| c.map_id).collect(); + let direct: Vec = wma + .iter() + .filter(|(_, a)| a.area_id != 0 && !continent_maps.contains(&a.map_id)) + .map(|(id, a)| DirectAreaEntry { + id, + map_id: a.map_id, + rect: zone_rect(a), + view: map_row_view( + id, + a, + areas.name(a.area_id).unwrap_or(a.name.as_str()).to_string(), + areas, + &wmo, + ), + }) + .collect(); + + Some(( + views, + WorldMapUiData { + continents: entries, + direct, + }, + )) } /// The built catalog, kept for the life of the process. A static `WorldMapArea` × `AreaTable` × @@ -247,6 +326,9 @@ pub(crate) fn build_catalog( /// the test plants a catalog and drives the entry edge, which is the ordering question without /// the DBC walk (the walk itself is covered against the real chain by /// `world_map_tests::the_real_feralas_catalog_names_dire_maul_under_the_cursor`). +/// +/// The **continent** half only: the orphan half of the same build rides [`WorldMapUiData`], where +/// each instance map's row already sits beside its projection rect. #[derive(Resource)] pub(crate) struct WorldMapCatalog(pub(crate) Vec); @@ -269,32 +351,41 @@ pub(crate) struct WorldMapCatalog(pub(crate) Vec); /// load, and the getters read it whenever they are asked. pub(crate) fn seed_world_map_catalog(world: &mut World, script: &mut UiScript) { if !world.contains_resource::() { - let Some((views, entries)) = build_catalog_from_world(world) else { + let Some((views, data)) = build_catalog_from_world(world) else { return; }; info!( - "world map: catalog — {} continents, {} zones", + "world map: catalog — {} continents, {} zones, {} instance maps", views.len(), - views.iter().map(|c| c.zones.len()).sum::() + views.iter().map(|c| c.zones.len()).sum::(), + data.direct.len() ); - world.insert_resource(WorldMapUiData { - continents: entries, - }); + world.insert_resource(data); world.insert_resource(WorldMapCatalog(views)); } let Some(catalog) = world.get_resource::() else { return; }; script.set_world_map_catalog(catalog.0.clone()); + // The orphan half of the same build, pushed at the same edge and for the same reason (the + // reference fills both containers in one walk). It rides [`WorldMapUiData`] rather than + // [`WorldMapCatalog`] because that is where the rows and their projection rects already live + // together — one list, one order, nothing to drift. + if let Some(data) = world.get_resource::() { + script.set_world_map_direct_areas( + data.direct + .iter() + .map(|d| (d.id, d.view.clone())) + .collect::>(), + ); + } } /// [`build_catalog`] over the resources the app holds — `None` when the patch chain or either DBC /// catalog is missing, which in a real run cannot happen at this edge: all three are `Startup` /// systems and the initial state transition is after `PostStartup` (decision 1038). A bare test /// world takes the `None`. -fn build_catalog_from_world( - world: &World, -) -> Option<(Vec, Vec)> { +fn build_catalog_from_world(world: &World) -> Option<(Vec, WorldMapUiData)> { let assets = world.get_resource::()?; let maps = world.get_resource::()?; let areas = world.get_resource::()?; @@ -302,31 +393,37 @@ fn build_catalog_from_world( build_catalog(&mut chain, &areas.0, maps) } -/// Which of the map's three levels is displayed — the reference's `(continent, zone)` globals -/// `[0x84506c]`/`[0x845070]` reduced to the thing every landmark gate actually branches on. +/// Which of the map's levels is displayed — the reference's three selection cells +/// `[0x84506c]`/`[0x845070]`/`[0x845074]` reduced to the thing every landmark gate actually +/// branches on. /// -/// The reference has a fourth, `ORPHAN` (`continent == -2`, a map selected directly rather than -/// through the continent/zone pair). We have no way to reach it: [`UiScript::world_map_selection`] -/// is a `(u32, u32)` pair with `0` for "whole", so a direct-area selection is unrepresentable. -/// Where the reference's gates test "zone **or** orphan", ours test zone — the same answer for -/// every state our map can be in. +/// **The direct-area state answers [`MapLevel::Zone`], and that is the reference's own law rather +/// than an approximation** (wow-re `worldmap-direct-area-selection.md` §6). Both gates that read +/// this enum test "zone **or** orphan": the AreaPOI level flag falls into the zone-level +/// `test [rec+0x20],0x4` at `-2` (`0x4a79de cmp eax,-2; jne`), and `GetMapLandmarkInfo`'s +/// `textureIndex` substitution is reached when the zone cell is `-1` **and** the continent is `-2` +/// (`0x4a8856`/`0x4a885f` both fall through to `0x4a8868`). A fourth variant would be two arms +/// that copy `Zone` exactly — so the fact is recorded here instead. #[derive(Clone, Copy, PartialEq, Eq, Debug)] enum MapLevel { /// Both continents on one sheet (the reference's `continent == -1`). World, /// One continent, no zone selected (`continent >= 0, zone == -1`). Continent, - /// One zone (`continent >= 0, zone >= 0`). + /// One zone (`continent >= 0, zone >= 0`) — or an instance map (`continent == -2`). Zone, } impl MapLevel { - /// Our 1-based `(continent, zone)` selection, `0` = "whole", mapped onto the reference's - /// `-1`-for-whole pair. - fn of(continent: u32, zone: u32) -> Self { - match (continent, zone) { - (0, _) => Self::World, - (_, 0) => Self::Continent, + /// The engine's selection ([`UiScript::world_map_selection`]) — our 1-based + /// `(continent, zone)` with `0` for "whole", plus the direct-area row id — mapped onto the + /// reference's levels. The direct cell is read FIRST: it shares the `(0, 0)` pair with the + /// world sheet and overrides it. + fn of((continent, zone, direct): (u32, u32, Option)) -> Self { + match (continent, zone, direct) { + (_, _, Some(_)) => Self::Zone, + (0, _, None) => Self::World, + (_, 0, None) => Self::Continent, _ => Self::Zone, } } @@ -346,7 +443,9 @@ impl MapLevel { /// input that silently hides map icons when it is wrong. #[derive(PartialEq, Eq)] struct LandmarkKey { - selection: (u32, u32), + /// All three selection cells — the direct-area one included, or an instance map's landmark + /// list would never rebuild when the selection moved onto it. + selection: (u32, u32, Option), map: u32, states: u64, /// Owned rather than borrowed, and compared before it is ever cloned — see @@ -364,7 +463,7 @@ impl LandmarkKey { /// changed — compares without allocating. fn matches( &self, - selection: (u32, u32), + selection: (u32, u32, Option), map: u32, states: u64, explored: &[u32], @@ -467,27 +566,57 @@ fn landmark_texture_index(poi: &benilla_formats::AreaPoi, level: MapLevel) -> u3 } } -/// `SetMapToCurrentZone`'s resolver, whose two halves fail SEPARATELY — see the call site in -/// [`feed_world_map`] for the byte citation. `map_id` is the player's `Map.dbc` id; `top_zone` is -/// their current area walked up to its top-level zone, or `None` while the area feed has not -/// answered. The answer is the 1-based `(continent, zone)` the Lua selection verbs speak, with -/// **zone 0 meaning the continent map** — the reference's `SetMap(continent, −1)` as -/// `GetCurrentMapZone` reads it back. -fn resolve_player_zone( +/// `0x4a6650`'s answer — which map the player's own position selects — in the Lua-visible +/// encoding. Its two legs are mutually exclusive by construction: the resolver runs the continent +/// loop first and reaches the orphan loop only on a miss (`0x4a66c3`). +#[derive(Clone, Copy, PartialEq, Eq, Debug, Default)] +struct PlayerSelection { + /// The continent/zone leg: the 1-based pair, with **zone 0 meaning the continent map** (the + /// reference's `SetMap(continent, −1)` as `GetCurrentMapZone` reads it back). `None` = no + /// continent record carried the player's map. + zone: Option<(u32, u32)>, + /// The orphan leg: the `WorldMapArea` row **ID** whose mapID is the player's own map. + direct: Option, +} + +/// `SetMapToCurrentZone`'s resolver (`0x4a7e20` is `call 0x4a6650`, and the engine's own +/// world-enter sync `0x4947ac` calls the same function — there is no second resolver). +/// +/// Three exits, in the reference's own precedence: +/// +/// 1. **A continent whose mapID is the player's**, and then its child zones — whose two halves +/// fail SEPARATELY (see the call site in [`feed_world_map`] for the byte citation). +/// 2. **Only on a continent miss**, the orphan list, matched on `row.mapID == playerMapId`, first +/// match wins: the direct-area selection `SetMap(-2, row.id)` (`0x4a6717`–`0x4a671e`). This is +/// the leg a battleground takes — map 489 is no continent's, so nothing above can match. +/// 3. Neither: the world view (both cells `None`), which is also what an empty orphan list gives. +fn resolve_player_selection( data: &WorldMapUiData, map_id: u32, top_zone: Option, -) -> Option<(u32, u32)> { - let ci = data.continents.iter().position(|c| c.map_id == map_id)?; - let zone = top_zone - .and_then(|top| { - data.continents[ci] - .zones - .iter() - .position(|z| z.area_id == top) - }) - .map_or(0, |zi| zi as u32 + 1); - Some((ci as u32 + 1, zone)) +) -> PlayerSelection { + if let Some(ci) = data.continents.iter().position(|c| c.map_id == map_id) { + let zone = top_zone + .and_then(|top| { + data.continents[ci] + .zones + .iter() + .position(|z| z.area_id == top) + }) + .map_or(0, |zi| zi as u32 + 1); + return PlayerSelection { + zone: Some((ci as u32 + 1, zone)), + direct: None, + }; + } + PlayerSelection { + zone: None, + direct: data + .direct + .iter() + .find(|d| d.map_id == map_id) + .map(|d| d.id), + } } /// The engine-side world-enter sync's gate and answer — `0x494780`'s `old == 0` leg into the @@ -502,39 +631,56 @@ fn resolve_player_zone( /// would sync on frame 1 to the bare continent and — since the reference never re-syncs — stay /// there. /// - **Every exit of `0x4a6650` writes the selection**, including its two failure legs -/// (`0x4a667e`/`0x4a670b` ⇒ `SetMap(−1, −1)`). So an unresolvable player — an instance map, no -/// continent row — still counts as synced, at the world view, rather than re-arming the gate -/// every frame. That is `player_zone.unwrap_or((0, 0))`: our `None` IS the reference's -/// `(−1, −1)`, which `GetCurrentMapContinent` reads back to Lua as `0`. +/// (`0x4a667e`/`0x4a670b` ⇒ `SetMap(−1, −1)`). So an unresolvable player — a map in neither +/// list — still counts as synced, at the world view, rather than re-arming the gate every +/// frame. That is the all-`None` [`PlayerSelection`]: it IS the reference's `(−1, −1)`, which +/// `GetCurrentMapContinent` reads back to Lua as `0`. +/// +/// A player logging straight into a battleground takes the *direct* leg here, which is why this +/// hands the whole answer on rather than the pair: the sync happens once per session, so dropping +/// the third cell would strand such a session on the world sheet for its whole life. fn world_enter_selection( synced: bool, top_zone: Option, - player_zone: Option<(u32, u32)>, -) -> Option<(u32, u32)> { - (!synced && top_zone.is_some()).then(|| player_zone.unwrap_or((0, 0))) + player: PlayerSelection, +) -> Option { + (!synced && top_zone.is_some()).then_some(player) } /// Per frame: selection read-back → projection → feed push (see the module doc). /// One projection law for every blip on the DISPLAYED map: world-sheet mode (selection /// `(0, _)`) projects through the POSITION's own map's continent constants; continent and zone -/// mode through the selected rect, gated to that continent's map. Off-map — the wrong continent, -/// an instance map, outside the rect — is `None`, the reference's `(0, 0)` hide sentinel once it +/// mode through the selected rect, gated to that continent's map; **direct-area mode through the +/// selected row's own rect**, gated to that row's map. Off-map — the wrong continent, a body in +/// another instance, outside the rect — is `None`, the reference's `(0, 0)` hide sentinel once it /// reaches Lua. +/// +/// The direct arm is `0x4a7360`'s own (`0x4a73e0 jl 0x4a7414`): it reads `[0x845074]`, resolves +/// the row, requires `[row+0x4] == mapId` (`0x4a7437`) and then runs the **same tail the zone case +/// uses** — it never touches the continent record array. The gate is what makes it right for +/// every caller and not just the player: `0x4a7870` passes the *unit's own* map id, so a +/// teammate standing in the battleground projects and one who is elsewhere answers `(0, 0)`. pub(crate) fn project_on_displayed( data: &WorldMapUiData, - selection: (u32, u32), + selection: (u32, u32, Option), pos_map: u32, px: f32, py: f32, ) -> Option<(f32, f32)> { match selection { - (0, _) => data + // The direct cell first: it shares the `(0, 0)` pair with the world sheet. + (_, _, Some(id)) => data + .direct + .iter() + .find(|d| d.id == id && d.map_id == pos_map) + .map(|d| map_proj::zone_uv(d.rect, px, py)), + (0, _, None) => data .continents .iter() .find(|cont| cont.map_id == pos_map) .and_then(|cont| cont.proj) .map(|p| map_proj::world_uv(p, px, py)), - (c, z) => data + (c, z, None) => data .continents .get(c as usize - 1) .filter(|cont| cont.map_id == pos_map) @@ -625,11 +771,15 @@ fn feed_world_map( // every consumer that assumes a zone uv — Astrolabe, and so every addon built on it — silently // mis-scales it. // - // NOT modelled, and deliberately: the `−2` orphan/direct-area leg for a map that is no - // continent's child (the instance maps). We have no orphan list, so those still land on the - // world view rather than the reference's direct-area selection. + // And when NO continent matched, the resolver falls through to its third leg: the orphan + // list, i.e. the instance maps (`0x4a66c3`). Inside a battleground that is the only leg that + // can answer — map 489 is no continent's child — and it selects the battleground's own + // `WorldMapArea` row directly, `SetMap(-2, row.id)`. Both halves travel to the engine + // together; with only the pair, `GetMapInfo()` answers nil in there and the stock + // `Blizzard_BattlefieldMinimap.lua:83-86` returns on the fourth line of its update, which is + // a battle map and a world map that draw nothing at all. let top_zone = world.area().and_then(|aid| areas.0.top_zone(aid)); - let player_zone = resolve_player_zone(&data, map.0, top_zone); + let player_sel = resolve_player_selection(&data, map.0, top_zone); // ── First world-enter: the ENGINE selects the player's zone, with no Lua in the loop. // @@ -656,19 +806,20 @@ fn feed_world_map( // precondition. Per **VM**, not per process: a new login is a new session's first enter. { let memo = memo.get(&script); - if let Some((c, z)) = world_enter_selection(memo.map_synced, top_zone, player_zone) { + if let Some(sel) = world_enter_selection(memo.map_synced, top_zone, player_sel) { memo.map_synced = true; - script.sync_world_map_to_player_zone(c, z); + let (c, z) = sel.zone.unwrap_or((0, 0)); + script.sync_world_map_to_player_zone(c, z, sel.direct); } } - // The player's UV on the DISPLAYED map. Off-map (wrong continent, outside the rect, or an - // instance map) resolves to None/(0,0) — the reference's hide-the-blip sentinel. - let (c, z) = script.world_map_selection(); + // The player's UV on the DISPLAYED map. Off-map (wrong continent, another instance, outside + // the rect) resolves to None/(0,0) — the reference's hide-the-blip sentinel. + let selection = script.world_map_selection(); // One projection law for every blip on the displayed map (the player now, the corpse below, // the battleground teammates in `ui_battlefield_positions`): [`project_on_displayed`]. let project = - |pos_map: u32, px: f32, py: f32| project_on_displayed(&data, (c, z), pos_map, px, py); + |pos_map: u32, px: f32, py: f32| project_on_displayed(&data, selection, pos_map, px, py); let uv = project(map.0, wx, wy); // The corpse marker (decision 0308 §5): the query answer's DISPLAY position/map (a dungeon // corpse projects at its entrance — the server rewrote it). `zone_uv`'s outside-the-rect @@ -691,9 +842,9 @@ fn feed_world_map( .get(&script) .landmarks .as_ref() - .is_some_and(|k| k.matches((c, z), map.0, states_gen, &explored, marker)); + .is_some_and(|k| k.matches(selection, map.0, states_gen, &explored, marker)); if !unchanged { - let level = MapLevel::of(c, z); + let level = MapLevel::of(selection); let mut landmarks = Vec::new(); // The DBC rows first, in file order, each through the builder's gate chain — that order // IS the Lua landmark index (`0x4a6819`'s walk over `[0xc0e054]`). @@ -734,7 +885,7 @@ fn feed_world_map( } } memo.get(&script).landmarks = Some(LandmarkKey { - selection: (c, z), + selection, map: map.0, states: states_gen, explored: explored.clone(), @@ -787,13 +938,18 @@ fn feed_world_map( }; script.set_world_map_feed( - player_zone, + player_sel.zone, uv, player.facing(), corpse_uv, party_uv, raid_uv, ); + // The orphan leg of the same resolver answer, beside the pair: `SetMapToCurrentZone` reads + // both back, and the stock battlefield minimap calls it itself on `PLAYER_ENTERING_WORLD` + // while it is shown — the call that lands a player zoning into a battleground on the + // battleground's own map. + script.set_world_map_player_direct_area(player_sel.direct); } /// **Alt+click the world map to go there** — the dev jump. @@ -847,28 +1003,40 @@ fn dev_map_jump( else { return; }; - let (c, z) = script.world_map_selection(); - let Some(cont) = c - .checked_sub(1) - .and_then(|i| data.continents.get(i as usize)) - else { - info!( - "map-jump: no exact click→world law at the world level — zoom into a continent first" - ); - return; - }; - let rect = match z.checked_sub(1) { - None => cont.rect, - Some(i) => match cont.zones.get(i as usize) { - Some(zone) => zone.rect, + let (c, z, direct) = script.world_map_selection(); + // An instance map is a rect like any other — the same `0x4a7100` zone-mode inverse — so the + // jump works off a battleground map too; only the world sheet has no exact inverse. + let (rect, map_id) = match direct { + Some(id) => match data.direct.iter().find(|d| d.id == id) { + Some(d) => (d.rect, d.map_id), None => return, }, + None => { + let Some(cont) = c + .checked_sub(1) + .and_then(|i| data.continents.get(i as usize)) + else { + info!( + "map-jump: no exact click→world law at the world level — zoom into a \ + continent first" + ); + return; + }; + let rect = match z.checked_sub(1) { + None => cont.rect, + Some(i) => match cont.zones.get(i as usize) { + Some(zone) => zone.rect, + None => return, + }, + }; + (rect, cont.map_id) + } }; // `zone_world` lerps BOTH axes by its single `t` (the binary's own shape) — so it is called // once per axis, which is how the reference's own callers consume it. let (_, wy) = map_proj::zone_world(rect, u); let (wx, _) = map_proj::zone_world(rect, v); - let text = format!(".go xy {wx:.2} {wy:.2} {}", cont.map_id); + let text = format!(".go xy {wx:.2} {wy:.2} {map_id}"); info!("map-jump: {text}"); let _ = net.0.send(crate::net::ClientCommand::Chat { kind: crate::net::ChatKind::Say, @@ -1065,13 +1233,21 @@ mod tests { assert!(!is_degenerate((1e-6, 0.0))); } - /// Our `(continent, zone)` pair, `0` = whole, onto the reference's levels. + /// The engine's three selection cells — our `(continent, zone)` pair with `0` = whole, plus + /// the direct-area id — onto the reference's levels. #[test] fn the_selection_pair_maps_onto_the_reference_levels() { - assert_eq!(MapLevel::of(0, 0), MapLevel::World); - assert_eq!(MapLevel::of(0, 3), MapLevel::World, "continent 0 wins"); - assert_eq!(MapLevel::of(2, 0), MapLevel::Continent); - assert_eq!(MapLevel::of(2, 7), MapLevel::Zone); + assert_eq!(MapLevel::of((0, 0, None)), MapLevel::World); + assert_eq!( + MapLevel::of((0, 3, None)), + MapLevel::World, + "continent 0 wins" + ); + assert_eq!(MapLevel::of((2, 0, None)), MapLevel::Continent); + assert_eq!(MapLevel::of((2, 7, None)), MapLevel::Zone); + // An instance map takes the ZONE arms of both gates it feeds — `0x4a79de`'s level flag + // and `0x4a8856`'s forced textureIndex 15 — even though it shares the world sheet's pair. + assert_eq!(MapLevel::of((0, 0, Some(443))), MapLevel::Zone); } /// The REAL `AreaPOI.dbc`, run through the whole gate chain the way the feed does — the @@ -1208,7 +1384,9 @@ mod player_zone_tests { } } - /// Azeroth (map 0) with Elwynn at catalog index 9 (→ Lua zone 10), plus Kalimdor (map 1). + /// Azeroth (map 0) with Elwynn at catalog index 9 (→ Lua zone 10), plus Kalimdor (map 1) — + /// and one orphan, Warsong Gulch's real row (`WorldMapArea` 443 on map 489), which is in + /// neither continent's list because no continent record carries its map. fn data() -> WorldMapUiData { WorldMapUiData { continents: vec![ @@ -1231,6 +1409,18 @@ mod player_zone_tests { .collect(), }, ], + direct: vec![DirectAreaEntry { + id: 443, + map_id: 489, + rect: rect(), + view: WorldMapZoneView { + name: "Warsong Gulch".into(), + area_id: 3277, + map_file: "WarsongGulch".into(), + loc_rect: (1.0, -1.0, 1.0, -1.0), + overlays: Vec::new(), + }, + }], } } @@ -1243,27 +1433,92 @@ mod player_zone_tests { #[test] fn an_unresolved_zone_falls_back_to_the_continent_not_the_world() { let d = data(); + let on_continent = |c, z| PlayerSelection { + zone: Some((c, z)), + direct: None, + }; // Both halves resolve: Elwynn is continent 2, zone 10. - assert_eq!(resolve_player_zone(&d, 0, Some(12)), Some((2, 10))); + assert_eq!( + resolve_player_selection(&d, 0, Some(12)), + on_continent(2, 10) + ); // The continent resolves, the zone does not — an area the catalog has no row for. assert_eq!( - resolve_player_zone(&d, 0, Some(4242)), - Some((2, 0)), + resolve_player_selection(&d, 0, Some(4242)), + on_continent(2, 0), "SetMap(continent, -1); GetCurrentMapZone reads that back as 0" ); // The area feed has not answered yet — same leg, not the world view. assert_eq!( - resolve_player_zone(&d, 0, None), - Some((2, 0)), + resolve_player_selection(&d, 0, None), + on_continent(2, 0), "a zone we do not know YET is still this continent" ); - // No continent match at all (an instance map) — the one leg that is genuinely None, and - // whose reference counterpart is the -2 orphan list we do not model. - assert_eq!(resolve_player_zone(&d, 389, Some(12)), None); + // No continent match and no orphan either (map 389 is Ragefire Chasm — a dungeon with no + // `WorldMapArea` row at all): both cells empty, which IS the reference's `SetMap(-1, -1)`. + assert_eq!( + resolve_player_selection(&d, 389, Some(12)), + PlayerSelection::default() + ); + } + + /// **The third leg — a battleground.** No continent record carries map 489, so `0x4a6650`'s + /// continent loop misses and only then does the orphan loop run, matching on the row's own + /// mapID and selecting `SetMap(-2, 443)` — the row's **ID**, not its place in the list. + /// Everything that draws the battle map hangs off that: `GetMapInfo()` answers nil without it, + /// and `Blizzard_BattlefieldMinimap.lua:83-86` returns before it draws a single tile. + #[test] + fn a_battleground_takes_the_orphan_leg_and_projects_on_its_own_rect() { + let d = data(); + + assert_eq!( + resolve_player_selection(&d, 489, Some(3277)), + PlayerSelection { + zone: None, + direct: Some(443), + }, + "the orphan leg selects the WorldMapArea row id" + ); + // The continent loop runs FIRST: a map that IS a continent's never reaches the orphan + // list, even were a row to shadow it. + assert_eq!( + resolve_player_selection(&d, 0, Some(12)).direct, + None, + "a continent match short-circuits before the orphan loop" + ); + + // The projection: the direct state goes through the selected row's OWN rect (the same + // tail the zone case uses), gated to that row's map. The fixture rect is [-1, 1] on both + // axes, so the origin is its centre. + let sel = (0, 0, Some(443)); + assert_eq!( + project_on_displayed(&d, sel, 489, 0.0, 0.0), + Some((0.5, 0.5)), + "a body in the battleground lands on the battleground map" + ); + assert_eq!( + project_on_displayed(&d, sel, 0, 0.0, 0.0), + None, + "`0x4a7437`: a body on another map is refused, which is the (0,0) hide sentinel" + ); + assert_eq!( + project_on_displayed(&d, (0, 0, Some(9999)), 489, 0.0, 0.0), + None, + "an id no row carries resolves to nothing rather than indexing anything" + ); + // And the direct cell WINS over the pair it shares its `(0, 0)` with — read the pair + // alone and an instance map projects as the world sheet. + assert_eq!( + project_on_displayed(&d, (0, 0, None), 489, 0.0, 0.0), + None, + "the world sheet has no continent for map 489 at all" + ); + assert_eq!(MapLevel::of(sel), MapLevel::Zone); + assert_eq!(MapLevel::of((0, 0, None)), MapLevel::World); } /// **The engine selects the player's zone itself, on the first world-enter** — the side call @@ -1279,25 +1534,206 @@ mod player_zone_tests { /// perfectly non-zero pair. #[test] fn the_engine_selects_the_players_zone_on_first_world_enter() { + let on_continent = |c, z| PlayerSelection { + zone: Some((c, z)), + direct: None, + }; // Frame 1, no area yet: the reference cannot be here at all (`0x67e510` bails before // `0x494780` on a zero zone id), so neither are we — syncing now would take the // continent leg and, since the engine never re-syncs, strand us there. - assert_eq!(world_enter_selection(false, None, Some((2, 0))), None); + assert_eq!(world_enter_selection(false, None, on_continent(2, 0)), None); // The area answers: sync, once, to the player's own zone. assert_eq!( - world_enter_selection(false, Some(12), Some((2, 10))), - Some((2, 10)) + world_enter_selection(false, Some(12), on_continent(2, 10)), + Some(on_continent(2, 10)) ); // And never again. A later zone change — even cross-continent — leaves the selection // alone: the guard is `old == 0`, and only the world-session teardown `0x491180` // re-zeroes `[0xb4e314]`. - assert_eq!(world_enter_selection(true, Some(40), Some((2, 22))), None); + assert_eq!( + world_enter_selection(true, Some(40), on_continent(2, 22)), + None + ); // An unresolvable player still counts as synced rather than re-arming the gate every // frame: EVERY exit of `0x4a6650` writes the selection, and its two failure legs write // `SetMap(-1, -1)` — which `GetCurrentMapContinent` reads back to Lua as `0`. - assert_eq!(world_enter_selection(false, Some(12), None), Some((0, 0))); + assert_eq!( + world_enter_selection(false, Some(12), PlayerSelection::default()), + Some(PlayerSelection::default()) + ); + + // Logging STRAIGHT INTO a battleground: the once-per-session sync carries the direct cell + // too. Dropping it here would strand that whole session on the world sheet, because + // nothing re-arms the gate. + let bg = PlayerSelection { + zone: None, + direct: Some(443), + }; + assert_eq!(world_enter_selection(false, Some(3277), bg), Some(bg)); + } +} + +#[cfg(test)] +mod direct_area_tests { + use super::*; + + /// The real catalog off the player's own chain, or a skip. + fn real_catalog() -> Option<(Vec, WorldMapUiData)> { + let data = benilla_formats::wow_data_or_skip!(None); + let mut chain = benilla_formats::open_chain(&data).expect("chain"); + let areas = benilla_formats::load_area_table_catalog(&mut chain).expect("AreaTable"); + let maps = benilla_assets::MapCatalogRes( + benilla_formats::load_map_catalog(&mut chain).expect("Map"), + ); + Some(build_catalog(&mut chain, &areas, &maps).expect("the real catalog")) + } + + /// **The whole population of the direct-area state in 1.12.1**, off the shipped + /// `WorldMapArea.dbc` (51 rows): the three battlegrounds, in the file order `0x4a5d00`'s fill + /// pass appends in — Alterac Valley, Warsong Gulch, Arathi Basin — and nothing else. The + /// complement rule is what produces them: `areaID != 0` (so not a continent row) and a mapID + /// no continent record carries (so not a zone either). Skips without client data. + #[test] + fn the_real_dbc_carries_exactly_the_three_battleground_orphans() { + let Some((views, data)) = real_catalog() else { + return; + }; + + assert_eq!( + data.direct + .iter() + .map(|d| (d.id, d.map_id, d.view.map_file.as_str())) + .collect::>(), + vec![ + (401, 30, "AlteracValley"), + (443, 489, "WarsongGulch"), + (461, 529, "ArathiBasin"), + ], + "file order, no sort — the reference's fill pass appends as it walks" + ); + + // They are in NEITHER continent list: that is the whole reason a third state exists. + for d in &data.direct { + assert!( + !views + .iter() + .any(|c| c.zones.iter().any(|z| z.map_file == d.view.map_file)), + "{} is no continent's child", + d.view.map_file + ); + } + assert!( + data.continents + .iter() + .all(|c| c.map_id == 0 || c.map_id == 1), + "and the continents are still only Azeroth and Kalimdor" + ); + + // Alterac Valley is the only battleground with overlay art (3 rows keyed on WMA 401); + // the other two have none, so a fully explored character still sees bare detail tiles. + let overlays = |folder: &str| { + data.direct + .iter() + .find(|d| d.view.map_file == folder) + .map(|d| d.view.overlays.len()) + }; + assert_eq!(overlays("AlteracValley"), Some(3)); + assert_eq!(overlays("WarsongGulch"), Some(0)); + assert_eq!(overlays("ArathiBasin"), Some(0)); + } + + /// **The bug, end to end, on the real data**: a body standing in Warsong Gulch (map 489) + /// resolves to the direct area, the engine selects it with no Lua in the loop, and + /// `GetMapInfo()` answers `"WarsongGulch"` — the art-folder identifier the stock + /// `Blizzard_BattlefieldMinimap.lua:83-86` needs before it will draw a single tile, and the + /// name that closes `Interface\WorldMap\WarsongGulch\WarsongGulch1..12` (all twelve ship). + /// The continent reads back `-1` (`-2 + 1`) and the zone `0`. + /// + /// The player's own blip comes with it: the rect centre projects to the middle of the map. + /// Skips without client data. + #[test] + fn a_body_in_warsong_gulch_lands_on_the_warsong_gulch_map() { + let Some((views, data)) = real_catalog() else { + return; + }; + + // The resolver: map 489 is no continent's, so the orphan loop answers with the row id. + let player = resolve_player_selection(&data, 489, Some(3277)); + assert_eq!( + player, + PlayerSelection { + zone: None, + direct: Some(443), + } + ); + + let mut script = UiScript::new().expect("a bare engine"); + script.set_world_map_catalog(views); + script.set_world_map_direct_areas( + data.direct + .iter() + .map(|d| (d.id, d.view.clone())) + .collect::>(), + ); + // The engine-side first-world-enter sync — `0x4947ac` into the same resolver, no Lua. + let (c, z) = player.zone.unwrap_or((0, 0)); + script.sync_world_map_to_player_zone(c, z, player.direct); + + assert_eq!( + script + .eval::<(String, i64, i64)>( + "return GetMapInfo(), GetCurrentMapContinent(), GetCurrentMapZone()" + ) + .expect("the getters answer"), + ("WarsongGulch".into(), -1, 0) + ); + + // The blip. The WSG row's rect centre in world coords → the middle of the map art; a body + // on Azeroth is refused outright (the reference's `(0, 0)` hide sentinel). + let wsg = data + .direct + .iter() + .find(|d| d.id == 443) + .expect("the Warsong Gulch row"); + let (wx, wy) = ( + (wsg.rect.top + wsg.rect.bottom) / 2.0, + (wsg.rect.left + wsg.rect.right) / 2.0, + ); + let selection = script.world_map_selection(); + assert_eq!(selection, (0, 0, Some(443))); + let uv = project_on_displayed(&data, selection, 489, wx, wy).expect("inside the rect"); + assert!( + (uv.0 - 0.5).abs() < 1e-5 && (uv.1 - 0.5).abs() < 1e-5, + "the rect centre is the middle of the battle map, got {uv:?}" + ); + assert_eq!(project_on_displayed(&data, selection, 0, wx, wy), None); + + // THE REFRESH TRAP: a world-state push or an exploration update re-selects, and a + // re-selection built from the continent/zone pair alone would silently drop the battle + // map for the world view (the reference re-passes `direct != -1 ? direct : zone` at all + // five of its in-place refresh sites). + script.set_world_map_explored(vec![u32::MAX; 64]); + script.set_world_map_player_direct_area(player.direct); + script + .run("SetMapToCurrentZone()") + .expect("the OnShow verb"); + assert_eq!( + script + .eval::<(String, i64)>("return GetMapInfo(), GetCurrentMapContinent()") + .expect("the getters answer"), + ("WarsongGulch".into(), -1), + "the instance selection survives every refresh path" + ); + + // And a continent selection clears it — `0x4a67a0`'s other legs all store `-1` there. + script.run("SetMapZoom(1)").expect("SetMapZoom"); + assert_eq!( + script.world_map_selection(), + (1, 0, None), + "selecting a continent clears the direct cell" + ); } } diff --git a/crates/benilla-formats/src/chr_classes.rs b/crates/benilla-formats/src/chr_classes.rs index 90efa899..a9f738b2 100644 --- a/crates/benilla-formats/src/chr_classes.rs +++ b/crates/benilla-formats/src/chr_classes.rs @@ -1,8 +1,10 @@ -//! `ChrClasses.dbc` — the per-class client table, and the two columns anything here reads off it. +//! `ChrClasses.dbc` — the per-class client table, and the three columns anything here reads off +//! it. //! -//! One file, one parse. Both columns are answers the *engine* gives Lua about a class, and both -//! are read by address off the same class-indexed record table at `ds:0xc0def4`, bounded by the -//! max id at `ds:0xc0def8` (store `0xc0deec`, loader `0x542360`, filename `0x85838c`). +//! One file, one parse. Two of the three are answers the *engine* gives Lua about a class and the +//! third is the talent spell-modifier gate's right-hand side; all three are read by address off +//! the same class-indexed record table at `ds:0xc0def4`, bounded by the max id at `ds:0xc0def8` +//! (store `0xc0deec`, loader `0x542360`, filename `0x85838c`). //! //! ## Field 4 — the pet name token //! @@ -56,6 +58,22 @@ //! requires `(InventoryType == 28 RELIC) == hasRelicSlot`, so **INVSLOT 17 takes a relic for those //! three classes and a ranged weapon for everyone else** — one slot, two meanings. 1.12 has no //! *separate* relic slot, which is the one true fragment inside the old belief. +//! +//! ## Field 15 — the class's spell family +//! +//! `GetSpellModifiers 0x6e6b30`'s second gate: a talent modifier applies to a spell only when that +//! spell's own `SpellFamilyName` (`Spell.dbc` column 160) equals the LOCAL PLAYER's class family, +//! which the client caches in the global `[0xcecaac]`. That global has exactly one +//! non-zeroing writer, `0x6e6ca0` (sole caller `0x5debcc`, the local-player create-finalise): it +//! takes `UNIT_FIELD_BYTES_0` byte 1 — the class, the same walk both sections above make — and +//! stores `ChrClasses[class] + 0x3c`, i.e. **field 15** (wow-re +//! `system/spell/scratch/spellmod-table-law.md` §8). +//! +//! Anchored on the shipped 5875 file, which is also a could-have-failed control on the column: the +//! nine rows read Warrior 4, Paladin 10, Hunter 9, Rogue 8, Priest 6, Shaman 11, Mage 3, Warlock 5, +//! Druid 7 — the vmangos `SpellFamilyNames` values exactly, in an order no neighbouring column +//! could reproduce (field 14 is a name-flags word reading 8323199 on every row, field 16 is the +//! relic flag above). use std::collections::HashMap; @@ -79,6 +97,10 @@ const PET_NAME_TOKEN_FIELD: usize = 0x10 / 4; /// The relic-slot column — byte `0x40` in `UnitHasRelicSlot`'s read, i.e. field 16. const RELIC_SLOT_FIELD: usize = 0x40 / 4; +/// The class spell-family column — byte `0x3c` in `0x6e6ca0`'s read, i.e. field 15. See the module +/// header for the walk and the shipped-file anchor. +const SPELL_FAMILY_FIELD: usize = 0x3c / 4; + /// The literal the client pushes when the player object does not resolve (`0x846a40`). Also the /// value nine of the ten class rows carry, which is why an unloaded table degrades invisibly. pub const PET_NAME_TOKEN_FALLBACK: &str = "PET"; @@ -88,6 +110,7 @@ pub const PET_NAME_TOKEN_FALLBACK: &str = "PET"; struct ChrClass { pet_name_token: Option, has_relic_slot: bool, + spell_family: u32, } /// `ChrClasses.dbc` → class id ⇒ the columns we read. @@ -114,6 +137,17 @@ impl ChrClasses { self.0.get(&class).is_some_and(|c| c.has_relic_slot) } + /// The class's spell family — the value the reference caches in `[0xcecaac]` and compares + /// every spell's `SpellFamilyName` against before any talent modifier may apply. + /// + /// A class with no row answers `0`, which is the reference's own degraded state: `0x6e7150` + /// zeroes the global at world-enter (`6e7317`, `esi = 0`) and it stays 0 until the local + /// player resolves — and `GetSpellModifiers`' first conjunct (`SpellFamilyName != 0`) exists + /// precisely so nothing matches during that window. + pub fn spell_family(&self, class: u32) -> u32 { + self.0.get(&class).map_or(0, |c| c.spell_family) + } + pub fn is_empty(&self) -> bool { self.0.is_empty() } @@ -148,6 +182,7 @@ pub fn load_chr_classes(chain: &mut Chain) -> Result { ChrClass { pet_name_token: str_at(&rs, r, PET_NAME_TOKEN_FIELD), has_relic_slot: u32_at(r, RELIC_SLOT_FIELD).is_some_and(|v| v != 0), + spell_family: u32_at(r, SPELL_FAMILY_FIELD).unwrap_or(0), }, ); } @@ -205,4 +240,30 @@ mod tests { assert!(!t.has_relic_slot(6)); assert!(!t.has_relic_slot(0)); } + + /// Field 15, the spell-family gate's right-hand side. The whole column is asserted because + /// that is what makes it a column pin rather than a coincidence: nine distinct values in nine + /// rows, each matching the vmangos `SpellFamilyNames` enum, is a shape no neighbour has + /// (field 14 is 8323199 on every row, field 16 is the three-row relic flag). + #[test] + fn every_class_row_carries_its_vmangos_spell_family() { + let Some(mut chain) = chain() else { return }; + let t = load_chr_classes(&mut chain).expect("load ChrClasses.dbc"); + for (class, family, who) in [ + (1u32, 4u32, "Warrior"), + (2, 10, "Paladin"), + (3, 9, "Hunter"), + (4, 8, "Rogue"), + (5, 6, "Priest"), + (7, 11, "Shaman"), + (8, 3, "Mage"), + (9, 5, "Warlock"), + (11, 7, "Druid"), + ] { + assert_eq!(t.spell_family(class), family, "{who}"); + } + // No row: the reference's zeroed global, which conjunct 1 of the gate refuses. + assert_eq!(t.spell_family(6), 0); + assert_eq!(t.spell_family(0), 0); + } } diff --git a/crates/benilla-formats/src/spells/catalog_tests.rs b/crates/benilla-formats/src/spells/catalog_tests.rs index 2abfb3ac..3b6fed88 100644 --- a/crates/benilla-formats/src/spells/catalog_tests.rs +++ b/crates/benilla-formats/src/spells/catalog_tests.rs @@ -1550,3 +1550,77 @@ fn real_channel_bar_name_law() { "only the two Blood Siphons suppress the bar" ); } + +/// The talent spell-modifier gate's two columns — `SpellFamilyName` 160 and the `SpellFamilyFlags` +/// pair 161/162 — against the shipped 5875 `Spell.dbc`. +/// +/// Three things are asserted, and each would catch a different slip. The **family histogram** is +/// the column pin: the eleven nonzero values are exactly the vmangos `SpellFamilyNames` set, and +/// their counts are the shipped file's own (a one-column slip lands on 159/163, neither of which +/// looks like this). The **popcount census** is what makes the reader's 64-iteration walk +/// load-bearing rather than defensive — 322 rows set more than one bit, and the highest index in +/// the whole table is 35, so the HIGH dword is live. The **three worked spells** pin the join +/// direction: 4987 sets 12 and 33, which a low-dword-only read or a swapped pair both get wrong. +/// +/// Numbers from wow-re `system/spell/scratch/spellmod-table-law.md` (Provenance), re-measured here +/// off the file this catalog actually loads. Skips without client data. +#[test] +fn real_spell_family_columns_carry_the_modifier_gate() { + let data = crate::wow_data_or_skip!(); + let mut chain = crate::open_chain(&data).expect("open chain"); + let cat = load_spell_catalog(&mut chain).expect("load Spell/SpellIcon"); + + let mut families: std::collections::BTreeMap = std::collections::BTreeMap::new(); + let mut popcounts: std::collections::BTreeMap = std::collections::BTreeMap::new(); + let mut max_bit = 0u32; + for (_, d) in cat.iter() { + *families.entry(d.spell_family).or_default() += 1; + *popcounts + .entry(d.spell_family_flags.count_ones()) + .or_default() += 1; + if d.spell_family_flags != 0 { + max_bit = max_bit.max(63 - d.spell_family_flags.leading_zeros()); + } + } + assert_eq!( + families, + [ + (0, 18243), + (1, 47), + (3, 563), + (4, 350), + (5, 466), + (6, 493), + (7, 450), + (8, 316), + (9, 393), + (10, 362), + (11, 500), + (13, 174), + ] + .into_iter() + .collect::>(), + "SpellFamilyName histogram — the vmangos SpellFamilyNames set, and only it" + ); + assert_eq!( + popcounts.values().skip(1).sum::() - popcounts[&1], + 322, + "rows setting MORE than one family bit — the reader's sum is live" + ); + assert_eq!(max_bit, 35, "the highest family bit index in the file"); + + // The three worked examples. Frostbolt spans one dword, Cleanse spans BOTH, Cure Poison lives + // entirely in the high one. + for (id, family, bits) in [ + (116u32, 3u32, &[5u32, 19, 20, 30][..]), + (4987, 10, &[12, 33]), + (526, 11, &[35]), + ] { + let d = cat.get(id).unwrap_or_else(|| panic!("spell {id}")); + assert_eq!(d.spell_family, family, "{id} {:?} family", d.name); + let set: Vec = (0..64) + .filter(|b| d.spell_family_flags >> b & 1 == 1) + .collect(); + assert_eq!(set, bits, "{id} {:?} family bits", d.name); + } +} diff --git a/crates/benilla-formats/src/spells/display.rs b/crates/benilla-formats/src/spells/display.rs index f0ecc900..4e534b93 100644 --- a/crates/benilla-formats/src/spells/display.rs +++ b/crates/benilla-formats/src/spells/display.rs @@ -61,6 +61,26 @@ pub struct SpellDisplay { /// search's **hand restriction** (main-hand-only / off-hand-only), which is where /// `0x5f0c50`'s slot mask comes from (decision 1903). pub attributes_ex3: u32, + /// **`SpellFamilyName`** (column 160, `SpellRec+0x280`) — which class's talent tree may modify + /// this spell. `0` on 18243 of the 22357 shipped rows (creature and world spells); the rest + /// carry a `SpellFamilyNames` value, which for a player spell is its own class's. + /// + /// The first two of `GetSpellModifiers 0x6e6b30`'s three conjunct gates read it and nothing + /// else: `!= 0` (`6e6b38`), then `== [0xcecaac]`, the local player's own class family + /// (`6e6b46` — [`crate::ChrClasses::spell_family`]). A spell that fails either takes no + /// modifier at all, which is how a mage's talent stays off a warrior's ability and off every + /// item/creature spell in the file. wow-re `system/spell/scratch/spellmod-table-law.md` §5.1. + pub spell_family: u32, + /// **`SpellFamilyFlags`** (columns 161/162, `SpellRec+0x284`/`+0x288`) — the 64-bit bit-set + /// naming which of its family's modifier rows this spell subscribes to, low dword first. + /// + /// `GetSpellModifiers` walks **all 64 bits** with no early break and SUMS one cell per set bit + /// out of each table (`6e6b72`–`6e6ba6`), so a multi-bit spell accumulates several — and the + /// high dword is genuinely live rather than unused width: measured on the shipped 5875 file, + /// 1794 rows set exactly one bit, **322 set more than one**, and the highest bit index in the + /// whole table is **35**. Frostbolt 116 sets 5/19/20/30, Cleanse 4987 sets 12 **and 33** (it + /// needs both dwords), Cure Poison 526 sets 35 alone. + pub spell_family_flags: u64, /// **`PreventionType`** (column 165, `SpellRec+0x294`) — which crowd-control flag refuses this /// spell **locally**, before any packet: `1` = silence, `2` = pacify, `0` = neither. The /// client's CC validator `0x6094f0` (called from `TryCast 0x6e4b60`, bailing at `0x6e4f42`) @@ -337,6 +357,8 @@ impl Default for SpellDisplay { attributes_ex2: 0, modal_next_spell: 0, attributes_ex3: 0, + spell_family: 0, + spell_family_flags: 0, prevention_type: 0, passive: false, cast_ui: 0, diff --git a/crates/benilla-formats/src/spells/mod.rs b/crates/benilla-formats/src/spells/mod.rs index f18055c2..fe8055c3 100644 --- a/crates/benilla-formats/src/spells/mod.rs +++ b/crates/benilla-formats/src/spells/mod.rs @@ -210,6 +210,14 @@ const COL_START_RECOVERY_TIME: usize = 158; /// `PreventionType` (`SpellRec+0x294`, `0x294/4 == 165`) — see [`SpellDisplay::prevention_type`] /// for the two-way pin that separates it from its `DmgClass` neighbour at 164. const COL_PREVENTION_TYPE: usize = 165; +/// `SpellFamilyName` (`SpellRec+0x280`, `0x280/4 == 160`) / `SpellFamilyFlags` low+high +/// (`+0x284`/`+0x288`, 161/162) — the talent spell-modifier gate and its row selector, read by +/// `GetSpellModifiers 0x6e6b30` at `6e6b38`/`6e6b46` and `6e6b83` (wow-re +/// `system/spell/scratch/spellmod-table-law.md` §5). See [`SpellDisplay::spell_family`] and +/// [`SpellDisplay::spell_family_flags`]; the shipped-file anchors are in +/// [`catalog_tests`]. +const COL_SPELL_FAMILY_NAME: usize = 160; +const COL_SPELL_FAMILY_FLAGS_LOW: usize = 161; /// `Targets` (`SpellRec+0x34`, `0x34/4 == 13`) — the wire `TARGET_FLAG_*` seed mask the cast-arm /// loads into its targeting flag_word (`0x6e525a`, wow-re `wave-cast.md`, VERIFIED). Empirical /// pin against the binder's bit semantics: Resurrection 2006 = `0x8000` (corpse-ally bit 15), @@ -817,6 +825,11 @@ pub fn load_spell_catalog(chain: &mut Chain) -> Result { u32_at(r, COL_EFFECT_MECHANIC_1 + i).unwrap_or(0) }), prevention_type: u32_at(r, COL_PREVENTION_TYPE).unwrap_or(0), + spell_family: u32_at(r, COL_SPELL_FAMILY_NAME).unwrap_or(0), + // Low dword first: bit `i >= 32` lives in column 162, which the reference reads as + // `[edi + 4*(i>>5) + 0x284]` — the same little-endian pair this join makes. + spell_family_flags: u64::from(u32_at(r, COL_SPELL_FAMILY_FLAGS_LOW).unwrap_or(0)) + | u64::from(u32_at(r, COL_SPELL_FAMILY_FLAGS_LOW + 1).unwrap_or(0)) << 32, passive: attributes & ATTR_PASSIVE != 0, cast_ui: u32_at(r, COL_CAST_UI).unwrap_or(0), effects: [0, 1, 2].map(|i| u32_at(r, COL_EFFECT_1 + i).unwrap_or(0)), diff --git a/crates/benilla-protocol/src/events.rs b/crates/benilla-protocol/src/events.rs index c5bcb9f2..e3318d89 100644 --- a/crates/benilla-protocol/src/events.rs +++ b/crates/benilla-protocol/src/events.rs @@ -915,6 +915,19 @@ pub enum SessionEvent { slot: u32, seconds: u32, }, + /// One cell of a talent spell-modifier table, absolutely (`SMSG_SET_FLAT_SPELL_MODIFIER` / + /// `SMSG_SET_PCT_SPELL_MODIFIER`): `flat` picks the table, `mask_bit` is the spell's + /// `SpellFamilyFlags` **bit index** (the row) and `op` the SpellModOp (the column). Neither + /// byte is bounded on the wire — the consumer (`benilla::spell_mods`) refuses an out-of-range + /// pair rather than reproducing the reference's own overrun. `value` is **absolute**, not a + /// delta; the wire shape and the `bit * 29 + op` index law are on + /// [`crate::messages::ServerPacket::SpellModifier`]. + SpellModifier { + flat: bool, + mask_bit: u8, + op: u8, + value: i32, + }, /// Start an on-hold (`SPELL_ATTR_COOLDOWN_ON_EVENT`) cooldown's parked timers now /// (`SMSG_COOLDOWN_EVENT`). CooldownEvent { spell_id: u32, caster: u64 }, diff --git a/crates/benilla-protocol/src/events/decode.rs b/crates/benilla-protocol/src/events/decode.rs index 14aa6edc..df2fa473 100644 --- a/crates/benilla-protocol/src/events/decode.rs +++ b/crates/benilla-protocol/src/events/decode.rs @@ -287,6 +287,17 @@ pub fn decode(packet: ServerPacket) -> Vec { slot, seconds, }], + ServerPacket::SpellModifier { + flat, + mask_bit, + op, + value, + } => vec![SessionEvent::SpellModifier { + flat, + mask_bit, + op, + value, + }], ServerPacket::CooldownEvent { spell_id, caster } => { vec![SessionEvent::CooldownEvent { spell_id, caster }] } diff --git a/crates/benilla-protocol/src/messages/opcode.rs b/crates/benilla-protocol/src/messages/opcode.rs index 33b39636..a82486b9 100644 --- a/crates/benilla-protocol/src/messages/opcode.rs +++ b/crates/benilla-protocol/src/messages/opcode.rs @@ -362,6 +362,23 @@ pub const SMSG_SPELL_FAILED_OTHER: u16 = 0x02A6; // 678 /// chain `CharProc` consumes once and zeroes (decision 0955). Body in [`super::spells`]. pub const SMSG_SPELL_UPDATE_CHAIN_TARGETS: u16 = 0x0330; // 816 +/// The **talent spell-modifier** pair (VERIFIED vmangos `Opcodes_1_12_1.h`: 614/615) — the only +/// feed for every talent that cheapens a spell, shortens its cast or cooldown, extends its range +/// or radius, or lengthens its duration. Body in [`super::spells::read_set_spell_modifier`]; +/// decision-level law in wow-re `system/spell/scratch/spellmod-table-law.md`. +/// +/// **One handler, two tables.** Both opcodes register to `Spell_C::HandleSetSpellModifier +/// 0x6e9950` (`0x6e7245`/`0x6e7255`), which reads the same 6-byte body either way and forks on the +/// opcode alone (`6e9989: cmp edi,0x267`): `0x266` stores into the FLAT table `0xcead60`, `0x267` +/// into the PCT table `0xcecb30`, each `i32[64][29]`. The store is a plain `mov`, so the server +/// sends the **absolute** value of one cell, never a delta (vmangos `Player::SendSpellMod` sends +/// one packet per set mask bit carrying that pair's total). +/// +/// That is why the two share a variant here rather than taking one each: the wire shape is +/// identical and the opcode IS the discriminant, exactly as the reference treats it. +pub const SMSG_SET_FLAT_SPELL_MODIFIER: u16 = 0x0266; // 614 +pub const SMSG_SET_PCT_SPELL_MODIFIER: u16 = 0x0267; // 615 + // The cooldown wire (VERIFIED vmangos `Opcodes_1_12_1.h`: 308/176/309/478/481; the client // handlers are byte-verified in wow-re `wave-handlers.md` — 0x6e9460/0x6e95d0/0x6e9670/0x6e9730; // decision 0137 phase 4). Bodies in [`super::spells`]. diff --git a/crates/benilla-protocol/src/messages/packet.rs b/crates/benilla-protocol/src/messages/packet.rs index 56f35a88..fb23212e 100644 --- a/crates/benilla-protocol/src/messages/packet.rs +++ b/crates/benilla-protocol/src/messages/packet.rs @@ -764,6 +764,26 @@ pub enum ServerPacket { slot: u32, seconds: u32, }, + /// `SMSG_SET_FLAT_SPELL_MODIFIER` / `SMSG_SET_PCT_SPELL_MODIFIER` — one cell of one of the two + /// talent spell-modifier tables, absolutely (layout + the index law in + /// [`super::spells::read_set_spell_modifier`]). + /// + /// One variant for both opcodes because the reference has one handler for both + /// (`0x6e9950`), reading the identical body and forking on the opcode alone to pick the table + /// — so `flat` IS the opcode, and nothing else differs. + SpellModifier { + /// `true` = `0x266`, the FLAT table (summed and added); `false` = `0x267`, the PCT table + /// (summed, then `+100`, clamped at 0, then used as a raw multiplier). + flat: bool, + /// The spell's `SpellFamilyFlags` **bit index**, 0..=63 — the table's row. The wire does + /// not bound it (the reference's own store overruns into the neighbouring global on a + /// 64); the consumer refuses it. + mask_bit: u8, + /// The SpellModOp, 0..=28 — the table's column. Likewise unbounded on the wire. + op: u8, + /// The cell's new value, **signed** and **absolute** — never a delta. + value: i32, + }, /// `SMSG_COOLDOWN_EVENT` — start an on-hold (`SPELL_ATTR_COOLDOWN_ON_EVENT`) cooldown's /// parked timers now (layout in [`super::spellbook::read_cooldown_event`]). CooldownEvent { @@ -1752,6 +1772,14 @@ impl ServerPacket { ServerPacket::ItemCooldown { .. } => "SMSG_ITEM_COOLDOWN".into(), ServerPacket::ItemTime { .. } => "SMSG_ITEM_TIME_UPDATE".into(), ServerPacket::ItemEnchantTime { .. } => "SMSG_ITEM_ENCHANT_TIME_UPDATE".into(), + // The opcode IS the table, so the name recovers it — unlike the collapsed + // `AttackSwingError` arm above, nothing here is lost. + ServerPacket::SpellModifier { flat, .. } => if *flat { + "SMSG_SET_FLAT_SPELL_MODIFIER" + } else { + "SMSG_SET_PCT_SPELL_MODIFIER" + } + .into(), ServerPacket::CooldownEvent { .. } => "SMSG_COOLDOWN_EVENT".into(), ServerPacket::ClearCooldown { .. } => "SMSG_CLEAR_COOLDOWN".into(), ServerPacket::CooldownCheat { .. } => "SMSG_COOLDOWN_CHEAT".into(), diff --git a/crates/benilla-protocol/src/messages/parse.rs b/crates/benilla-protocol/src/messages/parse.rs index e8318852..877b27d1 100644 --- a/crates/benilla-protocol/src/messages/parse.rs +++ b/crates/benilla-protocol/src/messages/parse.rs @@ -815,6 +815,16 @@ fn parse_server_body( seconds, } } + // One body, two tables — the opcode picks, exactly as `0x6e9950`'s `cmp edi,0x267` does. + opcode::SMSG_SET_FLAT_SPELL_MODIFIER | opcode::SMSG_SET_PCT_SPELL_MODIFIER => { + let (mask_bit, op, value) = spells::read_set_spell_modifier(&mut r)?; + ServerPacket::SpellModifier { + flat: opcode == opcode::SMSG_SET_FLAT_SPELL_MODIFIER, + mask_bit, + op, + value, + } + } opcode::SMSG_COOLDOWN_EVENT => { let (spell_id, caster) = spellbook::read_cooldown_event(&mut r)?; ServerPacket::CooldownEvent { spell_id, caster } diff --git a/crates/benilla-protocol/src/messages/spells.rs b/crates/benilla-protocol/src/messages/spells.rs index 463b26f5..3f5339c6 100644 --- a/crates/benilla-protocol/src/messages/spells.rs +++ b/crates/benilla-protocol/src/messages/spells.rs @@ -24,8 +24,8 @@ use std::io::{self, Read}; use crate::wire::{ - capacity_hint, read_cstring, read_packed_guid, read_u16_le, read_u32_le, read_u64_le, read_u8, - Vector3d, + capacity_hint, read_cstring, read_i32_le, read_packed_guid, read_u16_le, read_u32_le, + read_u64_le, read_u8, Vector3d, }; /// `SMSG_CAST_RESULT`'s verdict: `u32 spellId, u8 status` — status `0` (`SPELL_RESULT_STATUS_OKAY`) @@ -315,6 +315,39 @@ pub(super) fn read_channel_update(r: &mut impl Read) -> io::Result { read_u32_le(r) } +/// Read `SMSG_SET_FLAT_SPELL_MODIFIER` / `SMSG_SET_PCT_SPELL_MODIFIER` → `(mask_bit, op, value)`. +/// +/// **Exactly 6 bytes, and the first byte is the one that is easy to get backwards.** The single +/// handler `Spell_C::HandleSetSpellModifier 0x6e9950` reads `u8` (`6e9963`), `u8` (`6e996e`), `i32` +/// (`6e9979`) — the widths are the stream accessors' own (`0x418cb0` advances the cursor by 1, +/// `0x418e30` by 4) — and then stores at `imul eax,eax,0x1d; add eax,ecx`, i.e. +/// **`index = field1 * 29 + field2`**. The reader `GetSpellModifiers 0x6e6b30` strength-reduces +/// the same index to `4*op + 0x74*i` over a loop counter `i` bounded by `cmp eax,0x40`, which +/// forces `field1 = i` (the `SpellFamilyFlags` **bit index**, 0..63) and `field2 = op` (the +/// SpellModOp, 0..28) uniquely — and only that assignment fills the 64×29 table exactly +/// (`29*63 + 28 == 1855`). The transposed reading addresses 47% of the array and would make the +/// writer and the reader touch systematically disjoint cells, dropping every modifier silently. +/// wow-re `system/spell/scratch/spellmod-table-law.md` §1/§3 — which was written to correct +/// exactly that inversion in a note that had carried it, `verified`, for months. +/// +/// **vmangos's writer says the same thing from the other end**, which is worth having because the +/// client-side roles were published inverted once already: `Player::SendSpellMod` +/// (`Objects/Player.cpp:17815`) loops `for (int eff = 0; eff < 64; ++eff)` over the modifier's +/// 64-bit `mask` and, for each set bit, writes `uint8(eff)`, `uint8(mod->op)`, `int32(val)` — one +/// packet per set bit, `val` the SUM of every matching modifier, i.e. absolute. `MAX_SPELLMOD` +/// there is **29** (`Spells/SpellDefines.h`), the stride. +/// +/// Both bytes are `movzx`-ed (unsigned); the value's signedness is fixed three independent ways by +/// the reader (`sets dl` on the pct sum, `fild dword`, and a signed magic-divide). Neither byte is +/// bounds-checked by the reference — that is the consumer's job, and +/// `benilla::spell_mods::SpellModifiers` does it; this decode owns the shape only. +pub(super) fn read_set_spell_modifier(r: &mut impl Read) -> io::Result<(u8, u8, i32)> { + let mask_bit = read_u8(r)?; + let op = read_u8(r)?; + let value = read_i32_le(r)?; + Ok((mask_bit, op, value)) +} + /// Read `SMSG_UPDATE_AURA_DURATION` → `(slot, remaining_ms)` (vmangos /// `SpellAuraHolder::UpdateAuraDuration`, `SpellAuras.cpp:7511-7523`): a `u8` `UNIT_FIELD_AURA` slot /// index and a `u32` of milliseconds left. **Self-only** — it goes to the aura's target, never to @@ -517,6 +550,37 @@ mod tests { "the body is exactly 5 bytes — slot is a byte, not a dword" ); } + + /// The spell-modifier body, byte for byte — and the field ORDER, which is the whole trap. + #[test] + fn set_spell_modifier_body_golden() { + // mask_bit 35 (0x23) · op 14 (0x0e, SPELLMOD_COST) · value -30 (0xFFFF_FFE2 LE). + let body = [0x23, 0x0E, 0xE2, 0xFF, 0xFF, 0xFF]; + let mut r = &body[..]; + assert_eq!(read_set_spell_modifier(&mut r).unwrap(), (35, 14, -30)); + assert!( + r.is_empty(), + "the body is exactly 6 bytes — two bytes and a dword, never three dwords" + ); + + // The two bytes are NOT interchangeable, and this body is chosen so that a swap is + // detectable rather than plausible: 35 is a legal mask bit (0..=63) and an ILLEGAL op + // (0..=28), so a decode that read them the other way round would hand the consumer an op + // of 35 — off the end of the table's 29-wide axis. + let (mask_bit, op, _) = read_set_spell_modifier(&mut &body[..]).unwrap(); + assert!( + mask_bit >= 29, + "field 1 would be an out-of-range op if swapped" + ); + assert!(op < 29, "field 2 is a legal op as read"); + + // Both bytes are unsigned (`movzx`) and the value is signed: 0xFF in field 1 is bit 255, + // not -1, and it is the consumer that must refuse it. + assert_eq!( + read_set_spell_modifier(&mut &[0xFF, 0xFF, 0x01, 0x00, 0x00, 0x00][..]).unwrap(), + (255, 255, 1) + ); + } } /// One decoded `SMSG_SPELL_UPDATE_CHAIN_TARGETS` — the **beam's hop list** (decision 0955). diff --git a/crates/benilla-protocol/tests/spells.rs b/crates/benilla-protocol/tests/spells.rs index 7b1b4e17..d1782449 100644 --- a/crates/benilla-protocol/tests/spells.rs +++ b/crates/benilla-protocol/tests/spells.rs @@ -1384,3 +1384,55 @@ fn combat_log_completeness_wire() { )); assert!(messages::parse_server(messages::opcode::SMSG_SPELLLOGEXECUTE, &body).is_err()); } + +/// The talent spell-modifier pair, end to end: both opcodes through `parse_server` and on into a +/// [`SessionEvent`]. One handler, one body, and the opcode as the only discriminant — so the two +/// bodies here are byte-identical and only the `flat` flag differs. +#[test] +fn spell_modifier_wire() { + // mask_bit 35 · op 14 (SPELLMOD_COST) · value -30. Bit 35 is deliberately past the low dword: + // it is a real shipped value (Cure Poison 526's only bit) and it is the half a 32-bit mask + // would quietly drop. + let body = hx("230ee2ffffff"); + for (wire_op, is_flat) in [ + (messages::opcode::SMSG_SET_FLAT_SPELL_MODIFIER, true), + (messages::opcode::SMSG_SET_PCT_SPELL_MODIFIER, false), + ] { + let packet = messages::parse_server(wire_op, &body).unwrap(); + // The name recovers the opcode, which the collapsed-arm families cannot. + assert_eq!( + packet.name(), + if is_flat { + "SMSG_SET_FLAT_SPELL_MODIFIER" + } else { + "SMSG_SET_PCT_SPELL_MODIFIER" + } + ); + match &packet { + ServerPacket::SpellModifier { + flat, + mask_bit, + op, + value, + } => assert_eq!((*flat, *mask_bit, *op, *value), (is_flat, 35, 14, -30)), + other => panic!("spell modifier, got {}", other.name()), + } + match decode(packet).pop().unwrap() { + SessionEvent::SpellModifier { + flat, + mask_bit, + op, + value, + } => assert_eq!((flat, mask_bit, op, value), (is_flat, 35, 14, -30)), + _ => panic!("spell modifier event"), + } + } + + // Five bytes is not a body: the value is a dword, not the three bytes a "u8 u8 u8 i16" misread + // would take. + assert!(messages::parse_server( + messages::opcode::SMSG_SET_FLAT_SPELL_MODIFIER, + &hx("230ee2ffff") + ) + .is_err()); +} diff --git a/crates/benilla-ui/src/script/tests/worldmap.rs b/crates/benilla-ui/src/script/tests/worldmap.rs index e0ddd05a..cb5bb869 100644 --- a/crates/benilla-ui/src/script/tests/worldmap.rs +++ b/crates/benilla-ui/src/script/tests/worldmap.rs @@ -108,6 +108,176 @@ fn push_catalog(s: &mut UiScript) { ]); } +/// The **orphan list** — `0x4a5d00`'s third array, and in 5875 its entire population: the three +/// battlegrounds, with the real row ids, map ids, area ids and art folders, in `WorldMapArea.dbc` +/// **file order** (the fill pass appends with no sort). Alterac Valley carries one of its three +/// real `WorldMapOverlay` rows over its real AreaBit 954 — the only battleground art the +/// explored-bit gate has anything to say about (WSG and AB key zero overlay rows). +fn push_direct_areas(s: &mut UiScript) { + let row = |name: &str, area_id: u32, folder: &str, overlays: Vec| { + WorldMapZoneView { + name: name.into(), + area_id, + map_file: folder.into(), + // A synthetic rect: nothing engine-side projects through it (the app owns every + // projection), it is carried because the row is the row. + loc_rect: (1500.0, 500.0, 1600.0, 600.0), + overlays, + } + }; + s.set_world_map_direct_areas(vec![ + ( + 401, + row( + "Alterac Valley", + 2597, + "AlteracValley", + vec![WorldMapOverlayView { + texture: "Interface\\WorldMap\\AlteracValley\\DUNBALDAR".into(), + width: 128, + height: 128, + offset_x: 300, + offset_y: 200, + explore_bits: vec![954], + hit_rect: (200, 300, 328, 428), + area_name: Some("Dun Baldar".into()), + }], + ), + ), + (443, row("Warsong Gulch", 3277, "WarsongGulch", Vec::new())), + (461, row("Arathi Basin", 3358, "ArathiBasin", Vec::new())), + ]); +} + +/// **The direct-area selection** — the third state, and the whole of what a battleground map is +/// (wow-re `system/ui/scratch/worldmap-direct-area-selection.md`). Inside Warsong Gulch the +/// reference selects `(continent = -2, direct = WorldMapArea 443)`, and `GetMapInfo()` answers the +/// art folder that `Blizzard_BattlefieldMinimap.lua:83-86` needs before it will draw anything at +/// all; a client that models the selection as `(continent, zone)` answers nil there and draws an +/// empty battle map. +#[test] +fn a_direct_area_is_the_third_selection_state() { + let mut s = script(); + push_catalog(&mut s); + push_direct_areas(&mut s); + + // The player is in Warsong Gulch: `0x4a6650`'s continent loop misses (map 489 is no + // continent's) and only then does the orphan loop match, selecting the row's own id. + s.set_world_map_feed(None, Some((0.5, 0.5)), 0.0, None, Vec::new(), Vec::new()); + s.set_world_map_player_direct_area(Some(443)); + s.run("SetMapToCurrentZone()").unwrap(); + + assert_eq!( + s.eval::<(String, i64, i64)>( + "return GetMapInfo(), GetCurrentMapContinent(), GetCurrentMapZone()" + ) + .unwrap(), + ("WarsongGulch".into(), -1, 0), + "the ART FOLDER (not the localized \"Warsong Gulch\"), continent -2 + 1, zone -1 + 1" + ); + assert_eq!( + s.eval::<(i64, i64)>("local n, h, hpot = GetMapInfo() return h, hpot") + .unwrap(), + (0, 0), + "slots 2/3 are 0 on every instance map: WorldMapContinent.dbc ships only mapIDs 0 and 1" + ); + assert!( + s.eval::("local t = { GetMapZones(GetCurrentMapContinent()) } return t[1] == nil") + .unwrap(), + "GetMapZones(-1) takes the unsigned-bound exit — the dropdown that could write a bogus \ + row id has no entries to fire from" + ); + + // Hover and click are refused. Both matter: there is no `.zmp` bitmap for a battleground + // (`0x4a6ec0`/`0x4a7620`/`0x4a7540` each test `-2` explicitly), and the world sheet's + // continent walk must not run just because the direct state shares its `continent == 0` cell. + assert!(s + .eval::>("return UpdateMapHighlight(0.5, 0.5)") + .unwrap() + .is_none()); + s.run("ProcessMapClick(0.72, 0.63)").unwrap(); // EK land, were this the world sheet + assert_eq!( + s.eval::<(String, i64)>("return GetMapInfo(), GetCurrentMapContinent()") + .unwrap(), + ("WarsongGulch".into(), -1), + "a click cannot drill off an instance map" + ); + + // ── THE REFRESH TRAP. All five of the reference's in-place refresh sites re-pass + // `direct != -1 ? direct : zone` (`0x48f9f6`, `0x4a6460`, `0x6d93d8`, `0x6d9a17`, `0x6dac72`); + // one that re-passed the zone alone would drop the instance map back to the world view on the + // next world-state push or exploration update — silently, and only inside a battleground. + s.set_world_map_explored(vec![u32::MAX; 64]); + s.set_world_map_landmarks(Vec::new()); + s.run("SetMapToCurrentZone()").unwrap(); + s.sync_world_map_to_player_zone(0, 0, Some(443)); + assert_eq!( + s.eval::<(String, i64)>("return GetMapInfo(), GetCurrentMapContinent()") + .unwrap(), + ("WarsongGulch".into(), -1), + "a refresh re-selects the DIRECT cell, never the (0, 0) pair beside it" + ); + + // Selecting a continent or a zone CLEARS the direct cell: `0x4a67a0`'s other legs all fall + // into `0x4a67ea mov [0x845074],-1`, and only `ecx == -2` jumps past it. + s.run("SetMapZoom(2, 1)").unwrap(); + assert_eq!( + s.eval::<(String, i64, i64)>( + "return GetMapInfo(), GetCurrentMapContinent(), GetCurrentMapZone()" + ) + .unwrap(), + ("Elwynn".into(), 2, 1), + "the instance map does not shadow later navigation" + ); + + // And the stock zoom-out button's own path off a battleground map: its `else` arm fires + // because `GetCurrentMapZone()` is 0 there, and `SetMapZoom(0)` is the world view. + s.run("SetMapToCurrentZone()").unwrap(); + s.run("SetMapZoom(0)").unwrap(); + assert_eq!( + s.eval::<(Option, i64)>("return GetMapInfo(), GetCurrentMapContinent()") + .unwrap(), + (None, 0), + "zooming out of an instance map lands on the world sheet" + ); +} + +/// An instance map's overlays are **admitted**, keyed by the direct area, under exactly the zone's +/// explored-bit gate (`0x4a67a0`'s overlay half, `0x4a6b10 jl 0x4a6b3d`; wow-re +/// `worldmap-overlay-reveal-gate.md` §2) — no free reveal for a battleground. On 5875 data that +/// means Alterac Valley's three rows and nothing at all for the other two. +#[test] +fn an_instance_map_reveals_its_overlays_under_the_zone_gate() { + let mut s = script(); + push_catalog(&mut s); + push_direct_areas(&mut s); + + s.set_world_map_player_direct_area(Some(401)); + s.run("SetMapToCurrentZone()").unwrap(); + assert_eq!( + s.eval::("return GetNumMapOverlays()").unwrap(), + 0, + "nothing explored — the gate applies exactly as on a zone map" + ); + + // AreaBit 954 = word 29, bit 26. + let mut explored = vec![0u32; 64]; + explored[29] = 1 << 26; + s.set_world_map_explored(explored); + assert_eq!(s.eval::("return GetNumMapOverlays()").unwrap(), 1); + assert_eq!( + s.eval::("return GetMapOverlayInfo(1)").unwrap(), + "Interface\\WorldMap\\AlteracValley\\DUNBALDAR" + ); + + // Warsong Gulch keys zero `WorldMapOverlay` rows — a fully explored character still sees bare + // detail tiles there. + s.set_world_map_player_direct_area(Some(443)); + s.run("SetMapToCurrentZone()").unwrap(); + s.set_world_map_explored(vec![u32::MAX; 64]); + assert_eq!(s.eval::("return GetNumMapOverlays()").unwrap(), 0); +} + /// The navigation surface: lists come from the catalog, SetMapZoom's selection reads back /// synchronously in the same Lua breath (the reference's dropdown-click contract), zone/continent /// arguments clamp, and GetMapInfo names the displayed art folder (nil at the world level — the @@ -240,7 +410,7 @@ fn the_engine_can_select_a_zone_with_no_lua_call() { (0, 0) ); - s.sync_world_map_to_player_zone(1, 2); + s.sync_world_map_to_player_zone(1, 2, None); assert_eq!( s.eval::<(i64, i64)>("return GetCurrentMapContinent(), GetCurrentMapZone()") @@ -259,7 +429,7 @@ fn the_engine_can_select_a_zone_with_no_lua_call() { // It clamps through the same tail as the Lua verbs — an out-of-range pair cannot corrupt // the selection (`0x4a67a0` range-checks the zone against the continent's child count). - s.sync_world_map_to_player_zone(99, 99); + s.sync_world_map_to_player_zone(99, 99, None); let (c, _) = s .eval::<(i64, i64)>("return GetCurrentMapContinent(), GetCurrentMapZone()") .unwrap(); diff --git a/crates/benilla-ui/src/script/worldmap.rs b/crates/benilla-ui/src/script/worldmap.rs index e3631ee7..36a0542a 100644 --- a/crates/benilla-ui/src/script/worldmap.rs +++ b/crates/benilla-ui/src/script/worldmap.rs @@ -17,6 +17,15 @@ //! verified rules (WorldMapArea file order for continents, case-insensitive display-name sort //! for zones; wow-re Q1(d)/Q3(b) verdicts, 2026-07-07). //! +//! …and a **third** selection state beside that pair: the **direct area**, the map of an +//! instance — a battleground, a dungeon — which is no continent's child and so falls out of both +//! lists (wow-re `system/ui/scratch/worldmap-direct-area-selection.md`). The reference does not +//! encode it inside `(continent, zone)`: it carries THREE `.data` cells, and `continent == -2` +//! with a `WorldMapArea` row **ID** in the third one IS that state ([`WorldMapState::direct_area`]). +//! Inside Warsong Gulch a client without it answers `GetMapInfo() == nil`, and the stock +//! `Blizzard_BattlefieldMinimap.lua:83-86` returns on the fourth line of its update — an empty +//! battle map and an empty world map, which is the bug this models away. +//! //! Continent-level hover/click resolve through the pushed **zone grid** — the 128×128 area //! bitmap (`Interface\WorldMap\.zmp`, remapped app-side to 1-based zone indices; the //! source + cell law are wow-re-verified, `0x4a6ec0` / the Q1 §5 verdict 2026-07-07). The cell @@ -178,6 +187,14 @@ fn area_grid_cell(loc: (f32, f32, f32, f32), u: f32, v: f32) -> Option { /// bitmap, a zone/city is just a different rect window (wow-re 15b2a8ea, FUN_004a6ec0 §1c). This /// is what lets a click on a city's footprint from its neighbouring zone map drill into the city. fn grid_area(state: &WorldMapState, u: f32, v: f32) -> Option { + // The direct-area state has no bitmap and is refused before the cell law runs: `0x4a6ec0` + // opens `cmp esi,-2; je 0x4a70ed` (wow-re `worldmap-direct-area-selection.md` §6) — the chain + // ships exactly three `.zmp` files and none of them is a battleground's. Explicit rather than + // left to `c == 0` below, because "the world sheet" and "an instance map" are different + // states that happen to share that cell. + if state.direct_area.is_some() { + return None; + } let (c, z) = state.selection; if c == 0 { return None; @@ -215,12 +232,37 @@ pub struct WorldMapState { /// the world level, where `GetMapInfo` returns nil and the reference Lua falls back to /// `"World"`). pub continents: Vec, - /// The displayed map: `(continent, zone)`, `(0, 0)` = the world sheet. + /// The **orphan list** — the third array the reference's catalog builder fills (`0x4a5d00`'s + /// count/fill passes `0x4a6130`/`0x4a61c9`): every `WorldMapArea` row with `areaID != 0` + /// whose mapID matches **no** continent record, as `(row ID, the row)`, in DBC **file order, + /// unsorted** — the three battlegrounds in 5875. Keyed by id, never indexed: nothing outside + /// [`direct_row`]'s lookup reads a position here, because the selection stores the row's own + /// ID rather than a place in this list. + pub direct_areas: Vec<(u32, WorldMapZoneView)>, + /// The displayed map: `(continent, zone)`, `(0, 0)` = the world sheet. **Not the whole + /// selection on its own** — [`Self::direct_area`] overrides it. pub selection: (u32, u32), + /// The **direct-area selection**: the `WorldMapArea` row **ID** of an instance map, or `None` + /// at every continent/zone/world selection. + /// + /// The reference's selection is THREE `.data` cells, not two (wow-re + /// `worldmap-direct-area-selection.md` §1): `[0x84506c]` continent, `[0x845070]` zone, + /// `[0x845074]` direct. `continent == -2` IS this state, and the third cell then holds the + /// row's own id — **not** an index into [`Self::direct_areas`], which is only ever a lookup + /// table. The setter `0x4a67a0` keeps the two exclusive (`0x4a67c1` stores the id only on the + /// `ecx == -2` leg and jumps past `0x4a67ea`'s `= -1`; every other leg reaches it), so + /// `direct_area.is_some()` ⇒ `selection == (0, 0)` — enforced in [`store_selection`], the one + /// writer. + pub direct_area: Option, /// Where `SetMapToCurrentZone` lands — the player's current `(continent, zone)` resolved by - /// the app (AreaTable parent walk × the catalog). `None` = unresolvable (instance, loading) → - /// the world sheet. + /// the app (AreaTable parent walk × the catalog). `None` = no continent matched → either the + /// orphan leg below, or the world sheet. pub player_zone: Option<(u32, u32)>, + /// The orphan half of that same answer: the `WorldMapArea` row ID whose mapID is the player's + /// own map, or `None`. `0x4a6650` runs its continent loop FIRST and reaches the orphan loop + /// only on a miss (`0x4a66c3`), so at most one of the two is ever set — and inside a + /// battleground it is this one. + pub player_direct_area: Option, /// `GetPlayerMapPosition("player")` for the CURRENT selection — projected app-side each frame /// (`map_proj`), `None`/(0,0) = off this map (the reference hides the blip). pub player_uv: Option<(f32, f32)>, @@ -355,6 +397,11 @@ fn continent_hover(cont: &WorldMapContinentView, zone: &WorldMapZoneView) -> Opt /// name-only tail is reachable from there (`0x4a812e`). World level: the continent highlight is /// not built, so the tail. fn hover(wm: &WorldMapState, x: f32, y: f32) -> Hover { + // An instance map resolves no area under the cursor: the lookup INSIDE `UpdateMapHighlight` + // (`0x4a7620`) opens `cmp esi,-2; je 0x4a76d7` → `xor eax,eax`, so no highlight and no name. + if wm.direct_area.is_some() { + return Hover::Miss; + } let (c, z) = wm.selection; let Some(cont) = c.checked_sub(1).and_then(|i| wm.continents.get(i as usize)) else { return Hover::Miss; @@ -377,8 +424,28 @@ fn hover(wm: &WorldMapState, x: f32, y: f32) -> Hover { .unwrap_or(Hover::Miss) } -/// The current selection's zone view, if a zone map is displayed. +/// The selected direct area's row, resolved by **id** through the orphan list — the reference's +/// `[0xc0d5bc]` id-index lookup (`0x4a6cf0`'s `0x4a6d24` leg), which bound-checks on every read +/// rather than trusting the stored cell. An id no row carries reads back as `None`, which is the +/// binary's own NULL → `lua_pushnil`. +fn direct_row(state: &WorldMapState) -> Option<&WorldMapZoneView> { + let id = state.direct_area?; + state + .direct_areas + .iter() + .find(|(row_id, _)| *row_id == id) + .map(|(_, row)| row) +} + +/// The displayed map's `WorldMapArea` row, if one is displayed: the selected zone, or — in the +/// direct-area state — the instance map's own row. `0x4a6cf0` resolves both through the same id +/// lookup, and `0x4a67a0`'s overlay half admits the `-2` state with the displayed key +/// `= [0x845074]` (wow-re `worldmap-overlay-reveal-gate.md` §2), so a battleground's overlays +/// reveal under the explored-bit gate exactly as a zone's do — no free reveal. fn current_zone(state: &WorldMapState) -> Option<&WorldMapZoneView> { + if state.direct_area.is_some() { + return direct_row(state); + } let (c, z) = state.selection; if c == 0 || z == 0 { return None; @@ -417,6 +484,25 @@ impl super::UiScript { .push(("WORLD_MAP_UPDATE".to_string(), Vec::new())); } + /// Push the **orphan list** — the instance maps, keyed by `WorldMapArea` row id (see + /// [`WorldMapState::direct_areas`]). Pushed beside the continent catalog at the same edge and + /// for the same reason; the reference fills both containers in one walk (`0x4a5d00`). + pub fn set_world_map_direct_areas(&mut self, direct_areas: Vec<(u32, WorldMapZoneView)>) { + let mut model = self.model_mut(); + model.worldmap.direct_areas = direct_areas; + model + .pending_events + .push(("WORLD_MAP_UPDATE".to_string(), Vec::new())); + } + + /// Push the resolver's **orphan leg** for the player — the `WorldMapArea` row id whose map is + /// the player's own, or `None` (see [`WorldMapState::player_direct_area`]). The continent/zone + /// leg rides [`Self::set_world_map_feed`]'s `player_zone`; `0x4a6650` computes both in one + /// pass and reaches this one only when that one missed. + pub fn set_world_map_player_direct_area(&mut self, direct_area: Option) { + self.model_mut().worldmap.player_direct_area = direct_area; + } + /// Push the discovery bitset (the app calls this when `PLAYER_EXPLORED_ZONES` changes — /// including the first stream-in). Queues `WORLD_MAP_UPDATE` so an open zone map fills in /// newly explored art immediately. @@ -463,10 +549,15 @@ impl super::UiScript { model.worldmap.raid_uv = raid_uv; } - /// The engine-owned selection `(continent, zone)` — the app reads it each frame to project - /// the feed for the displayed map. - pub fn world_map_selection(&self) -> (u32, u32) { - self.model_ref().worldmap.selection + /// The engine-owned selection — the app reads it each frame to project the feed for the + /// displayed map. **All three of the reference's cells**, in its own order: the continent and + /// zone pair (`0` = whole), then the direct-area row id + /// ([`WorldMapState::direct_area`]) which overrides both when it is `Some`. A consumer that + /// reads only the pair sees an instance map as the world sheet — which is precisely the bug + /// that left the battle map empty — so the third cell travels with them. + pub fn world_map_selection(&self) -> (u32, u32, Option) { + let wm = &self.model_ref().worldmap; + (wm.selection.0, wm.selection.1, wm.direct_area) } /// Move the selection **from the engine**, with no Lua in the loop — the reference's @@ -479,9 +570,19 @@ impl super::UiScript { /// level `GetPlayerMapPosition` answers a world-SHEET uv (`0x4a7360` step 2), which every /// addon that assumes a zone uv silently mis-scales. See [`crate::script::worldmap`]'s caller /// in `ui_world_map::feed_world_map` for the gate. - pub fn sync_world_map_to_player_zone(&mut self, continent: u32, zone: u32) { + /// + /// It takes the resolver's whole answer — including the orphan leg — because `0x4947ac` calls + /// `0x4a6650`, the same resolver `SetMapToCurrentZone` does: a player logging straight into a + /// battleground is on the battleground's own map before any Lua runs, exactly as one logging + /// into Elwynn is on Elwynn's. + pub fn sync_world_map_to_player_zone( + &mut self, + continent: u32, + zone: u32, + direct_area: Option, + ) { let mut model = self.model_mut(); - select(&mut model, i64::from(continent), i64::from(zone)); + select_resolved(&mut model, continent, zone, direct_area); } /// The **normalized position within the map art** at UI-space `(x, y)`, or `None` when the @@ -515,8 +616,22 @@ impl super::UiScript { } } -/// Clamp + store a selection and queue the repaint event. The shared tail of -/// `SetMapZoom`/`SetMapToCurrentZone`/`ProcessMapClick`. +/// **The one writer of the three selection cells** — `0x4a67a0`, whose whole job is that the +/// continent/zone pair and the direct-area id can never both be live: the setter's valid-continent +/// leg and its `-1` leg both fall into `0x4a67ea mov [0x845074],eax` (`= -1`), and only `ecx == -2` +/// jumps past it after storing the id (`0x4a67c1`), having already forced the zone cell to `-1`. +/// Every selection verb in this module goes through here, so neither half can be moved alone. +fn store_selection(model: &mut Model, selection: (u32, u32), direct_area: Option) { + model.worldmap.selection = selection; + model.worldmap.direct_area = direct_area; + model + .pending_events + .push(("WORLD_MAP_UPDATE".to_string(), Vec::new())); +} + +/// Clamp + store a continent/zone selection and queue the repaint event. The shared tail of +/// `SetMapZoom`/`SetMapToCurrentZone`/`ProcessMapClick` — and it **clears the direct area**, which +/// is the setter invariant above. fn select(model: &mut Model, continent: i64, zone: i64) { let c = continent.clamp(0, model.worldmap.continents.len() as i64) as u32; let z = if c == 0 { @@ -525,10 +640,30 @@ fn select(model: &mut Model, continent: i64, zone: i64) { let n = model.worldmap.continents[c as usize - 1].zones.len() as i64; zone.clamp(0, n) as u32 }; - model.worldmap.selection = (c, z); - model - .pending_events - .push(("WORLD_MAP_UPDATE".to_string(), Vec::new())); + store_selection(model, (c, z), None); +} + +/// Select an instance map directly — `0x4a67a0` reached with `ecx == -2`: the zone cell goes to +/// `-1` (our `0`) and the raw `WorldMapArea` row id lands in `[0x845074]` **verbatim and +/// unchecked**. We keep it unchecked for the same reason the reference can: every consumer +/// bound-checks on read ([`direct_row`]), so an id no row carries degrades to the nil map name +/// instead of corrupting the selection. +fn select_direct(model: &mut Model, wma_id: u32) { + store_selection(model, (0, 0), Some(wma_id)); +} + +/// Apply the resolver `0x4a6650`'s answer — the shared tail of `SetMapToCurrentZone` (`0x4a7e20` +/// is literally `call 0x4a6650`) and of the engine's own first-world-enter sync (`0x4947ac`, the +/// same function: there is no second resolver). +/// +/// The orphan leg wins when it is set, and it is set only when no continent matched — the +/// reference reaches `0x4a66c3` exclusively through the continent loop's miss, so the two answers +/// are mutually exclusive by construction upstream and the order here is belt-and-braces. +fn select_resolved(model: &mut Model, continent: u32, zone: u32, direct_area: Option) { + match direct_area { + Some(id) => select_direct(model, id), + None => select(model, i64::from(continent), i64::from(zone)), + } } /// Register the world-map globals. @@ -551,6 +686,11 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { )?; // GetMapZones(continent) → name1, name2, … (that continent's zone list; index = zone number). + // A NEGATIVE continent answers empty — `0x4a7d10`'s `dec edi; cmp edi,[0xb6e664]; jae` is an + // unsigned bound, so `GetCurrentMapContinent()`'s `-1` on an instance map takes the empty + // exit. That emptiness is load-bearing: it is what leaves `WorldMapZoneButton_OnClick` with + // no button to fire, and so what stops a `SetMapZoom(-1, id)` from writing a bogus row id + // into the direct cell (wow-re §5.4). g.set( "GetMapZones", lua.create_function(|lua, c: i64| { @@ -570,12 +710,20 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { )?; // GetCurrentMapContinent() / GetCurrentMapZone() — the selection halves (0 = world / whole - // continent). The reference reads them right after SetMapZoom in the same click. + // continent). The reference reads them right after SetMapZoom in the same click. Each is its + // cell `+ 1` and nothing else (`0x4a7ed0`, `0x4a7f00`: a straight-line fild/pushnumber), so + // the direct-area state answers **-1** (from the `-2` sentinel) and **0** (the zone cell it + // forces to `-1`). The `-1` is a value no stock path tests for, and benignly: it leaves + // `WorldMapFrame.lua:59`'s zoom-out button enabled, and the zone dropdown it would feed is + // empty because `GetMapZones(-1)` takes the unsigned-bound exit below. g.set( "GetCurrentMapContinent", lua.create_function(|lua, ()| { let model = lua.app_data_ref::().expect("model app_data"); - Ok(i64::from(model.worldmap.selection.0)) + Ok(match model.worldmap.direct_area { + Some(_) => -1i64, + None => i64::from(model.worldmap.selection.0), + }) })?, )?; g.set( @@ -596,28 +744,41 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; - // SetMapToCurrentZone() — the OnShow verb: jump to the player's zone (app-resolved feed). + // SetMapToCurrentZone() — the OnShow verb: jump to the player's own map (app-resolved feed). + // `0x4a7e20` is `call 0x4a6650; xor eax,eax; ret`, so this is the resolver's whole answer, + // orphan leg included — which is what puts a player standing in Warsong Gulch on the Warsong + // Gulch map. The stock battlefield minimap calls it itself on PLAYER_ENTERING_WORLD. g.set( "SetMapToCurrentZone", lua.create_function(|lua, ()| { let mut model = lua.app_data_mut::().expect("model app_data"); let (c, z) = model.worldmap.player_zone.unwrap_or((0, 0)); - select(&mut model, i64::from(c), i64::from(z)); + let direct = model.worldmap.player_direct_area; + select_resolved(&mut model, c, z, direct); Ok(()) })?, )?; // GetMapInfo() → mapFileName (the Interface\WorldMap\\ folder). World level → nil — // the reference Lua's own `"World"` fallback exists because the client returned nil there - // (INFERRED, wow-re Q3 pending). + // (wow-re `worldmap-direct-area-selection.md` §5.1/§5.3). + // + // The name is `WorldMapArea` field[3], the **art-folder identifier** — "WarsongGulch", never + // the localized "Warsong Gulch": `0x4a6cf0` reads `[row+0xc]` with no locale multiplier, in + // deliberate contrast to `0x4a8740`'s `[rec + 4*[0xc0e080] + 0x2c]` AreaTable read. That is + // why FrameXML interpolates it straight into `Interface\WorldMap\\1..12`. g.set( "GetMapInfo", lua.create_function(|lua, ()| { let model = lua.app_data_ref::().expect("model app_data"); let wm = &model.worldmap; - let file = match wm.selection { - (0, _) => None, - (c, z) => wm.continents.get(c as usize - 1).map(|cont| match z { + let file = match (wm.direct_area, wm.selection) { + // The direct-area state — an instance map, resolved by id through the orphan + // list. `0x4a6cf0`'s `jl 0x4a6d24` sends BOTH negative continents here and the + // `-1` world level answers nil only because its third cell is `-1` too. + (Some(_), _) => direct_row(wm).map(|row| row.map_file.clone()), + (None, (0, _)) => None, + (None, (c, z)) => wm.continents.get(c as usize - 1).map(|cont| match z { 0 => cont.map_file.clone(), z => cont .zones @@ -626,11 +787,18 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { .unwrap_or_else(|| cont.map_file.clone()), }), }; - // THREE values, always. `0x4a7e30` has one `ret` and not a single conditional jump — - // there is no miss branch at all, the emptiness lives in two helpers — so the - // world-level answer is `nil, 0, 0` rather than one nil. (`arity_conf = exact`; - // decision 1845.) The two zeros are the map's texture dimensions, which no caller - // reads on the world level. + // THREE values, always: `0x4a7e30` pushes all three unconditionally and returns + // `mov eax,3` from its single `ret`, so the world-level answer is `nil, 0, 0` rather + // than one nil. (Its four conditional jumps all sit inside slot 3's power-of-two + // round and none gates a push — wow-re §10 corrects the "no conditional jump at all" + // absolute this comment used to carry, without touching the conclusion. + // `arity_conf = exact`; decision 1845.) + // + // The two zeros are the map art's texture dimensions, which no caller reads: both + // consumers bind slot 2 to a `textureHeight` local they never read again. They are + // **0 on every instance map** in the reference too — `0x4a6d50` scans + // `WorldMapContinent.dbc` for the row's mapID and that table ships only mapIDs + // {0, 1}, so the scan exhausts (§5.2). let file = match file { Some(f) => Value::String(lua.create_string(&f)?), None => Value::Nil, @@ -696,6 +864,13 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { "ProcessMapClick", lua.create_function(|lua, (x, y): (f32, f32)| { let mut model = lua.app_data_mut::().expect("model app_data"); + // An instance map swallows the click: the drill INSIDE `ProcessMapClick` + // (`0x4a7540`) opens `cmp esi,-2; je 0x4a760d`, a bare epilogue. Ahead of the world + // arm below, because the direct state shares its `continent == 0` cell and must not + // fall into the world sheet's continent walk. + if model.worldmap.direct_area.is_some() { + return Ok(()); + } if model.worldmap.selection.0 == 0 { let hit = model.worldmap.continents.iter().position(|cont| { let (u0, v0, u1, v1) = cont.world_rect; @@ -793,9 +968,11 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; - // GetNumMapOverlays() — how many of the displayed zone's overlays are REVEALED by the + // GetNumMapOverlays() — how many of the displayed map's overlays are REVEALED by the // explored bitset (the client's C side filters the same way; the reference Lua draws every - // returned overlay). 0 at world/continent level. + // returned overlay). 0 at world/continent level; an instance map is admitted under the same + // gate as a zone (see [`current_zone`]) — which on 5875 data means Alterac Valley's three + // rows and nothing for the other two battlegrounds. g.set( "GetNumMapOverlays", lua.create_function(|lua, ()| {