From 8f0bece3be9b281a7bbfb8bcb5a2c502de7f3714 Mon Sep 17 00:00:00 2001 From: Sam Date: Sun, 6 Sep 2026 19:39:11 +0700 Subject: [PATCH] loot and pet refusals become message ids, and waterfalls flow again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - a loot refusal is a message id, not a sentence: `loot_error_text` composed eight sentences for `SMSG_LOOT_RESPONSE`'s refusals and six of them said something 1.12 never says — "too far away to loot that." for a string that ends "that corpse.", "You can't loot that from there." for "You must be facing the corpse to loot it.", and an invented "Those pockets are already empty." for a code the client has no string for; it and the `LootErrors` queue are deleted, and the refusal is raised by GlobalStrings key into `UiErrorKeys` at the net bridge, the way `fish_verdict` two functions above already did, off the reference's own eleven-entry jump table at `0x5ebc64`; a composed string had no record to read a surface or a voice off, so all eleven — `MsgKind::Error`, the red `UIErrorsFrame` line — had been going into the chat window as yellow SYSTEM lines, and the four spoken in the player's own race and gender (`DIDNT_KILL`, `BAD_FACING`, `LOCKED`, `TOO_FAR`) were silent; a second bug fell out of reading the leg: codes 4–9, 11 and the default fall through a release tail at `0x5ebac2` that zeroes `[player+0x1d28]` — our `LootLatch` — while 10, 12, 13 and 14 jump past it, so a master looter refused a hand-off no longer stands up out of the kneel at a still-open corpse; the default arm's `ja 0x5ebabd` lands on code 7's arm, so every unnamed code prints `ERR_LOOT_DIDNT_KILL` and releases (2035); and the pet has its own two message tables, not the player's with a flag: `SMSG_PET_ACTION_FEEDBACK`'s handler `0x4bdb70` decrements the byte, bounds it at 3 and jump-tables it through `0x4bdbe8` to `ERR_PET_SPELL_DEAD`, `ERR_NO_ATTACK_TARGET`, `ERR_INVALID_ATTACK_TARGET`, `ERR_PET_SPELL_NOPATH` — one per vmangos `PetFeedback` value, where our two-entry map had printed "No path available for your pet" for a dead pet, "Out of range" for no target and nothing for 3 and 4; `SMSG_PET_CAST_FAILED` is `0x6e8eb0`, a separate 142-byte index table that overrides ten reasons, six of them to `ERR_PET_SPELL_*` rows with no other raise site in the client, so `CastFail` grows a `Caster` and picks its table from it while layers 1 and 3 stay shared, as the binary shares them; `0x6e8eb0` calls neither the combat-log formatter `0x62c360` nor the error sound `0x458a50`, so a pet's refused Growl no longer prints "You fail to cast Growl: …" as the player's own failure; and the source walk that exists to catch a non-catalog key was blind to both wrong keys because all 465 catalog keys begin `ERR_`, so the tripwire moves to the funnel — `Shown::keyed` warns on a key `by_key` cannot resolve (2033) - the hunter-pet feedback cluster, five opcodes carved off the binary where four had sat name-table-only: `SMSG_PET_TAME_FAILURE` (`0x173`) is two GlobalStrings lookups in order — `0x6e6a20` maps `reason − 1` through the eleven-arm jump table at `0x6e6ac0` to a `PETTAME_*` key and hands the resolved string to `DisplayError(ERR_TAME_FAILED)`, anything outside `1..=11` taking `PETTAME_UNKNOWNERROR`, and Call Pet and Revive Pet raise it too; `SMSG_PET_NAME_INVALID` (`0x178`) raises `ERR_INVALID_PETNAME` from the shared dispatcher `0x5e38c0`'s arm `0x5e3e33` — 1066 had called a refused rename silent on a carve that gave `0x178` the handler `0x4bdc00`, which is `0x2AF SMSG_PET_BROKEN`'s and raises `ERR_PET_BROKEN`, body unread; `SMSG_PET_ACTION_SOUND` (`0x324`) is a bark, not a sound id — its two talk values reach the bark dispatcher `0x623a40` at states 1 and 2, i.e. `CreatureSoundData` columns 28 and 27, and the shipped DBC names them from the other side: exactly four of its 406 rows carry them, the Imp, Succubus, Doomguard and Voidwalker voices, as `A__ORDER`/`A__KILL`; `Latch::Voice` now carries the bark state as a number and every one-shot creature vocal goes through one `play_bark` that is `0x623a40` — aggro at 0, the pet barks at 1 and 2, the death cry at 4, which had been on a bare `play_kit` off the slot so a pet bark could talk over it — with the reference's `jle` priority rule, because under the old boolean slot a demon's voice would have lost every race against its own aggro flares and been inaudible; it is a warlock-demon surface end to end — vmangos only sends it for a summoned pet, at a 10% roll per order, and a hunter's wolf has neither the columns nor the packet; and the fifth opcode surfaced because the cross-check went looking for column 29's consumer: `SMSG_PET_DISMISS_SOUND` (`0x325`), handler `0x604140`, reads column 29 twice (`0x6041c4`/`0x6041dc`) off a row resolved fresh by `CreatureModelData` id rather than the cached `[unit+0xb40]` — which is why a census over the cache had called the column dead — and plays `A__DISMISS` at the packet's point with `z + 1.0`, no unit and no latch; column 29 is loaded, the column nothing reads is 22 (zero on all 406 rows), and the dismiss handler is built although vmangos never sends the packet (2039) - every UV scroll and animated tint in the world had been frozen for three days: 1940's deferred realization parks a freshly built `WowModelMaterial` behind a reserved handle until something view-visible binds it, and `doodad_anim::register_uv`/`register_tint` still reached for the store alone — `Assets::get_mut` answered `None` for every registration, the slot was freed, the registry stayed empty and the shader read the pinned-zero identity row for every batch, silently, because the `else` branch was a bare `return`; the fix is the address, not the timing: `Parked::value_mut`/`value`/`holds` reach a value wherever it currently lives and `lazy::with_material_mut`/`with_material`/`holds` publish that, so the one-shot build stamp lands in the parked value and rides realization in — realizing it to stamp it would have thrown away exactly what deferral buys; the tick's eviction on `Assets::contains` read a parked material as dead and asks `lazy::holds` now, the empty-registry early-out says so out loud when marked parts exist, and two instruments ship — `WOW_MATANIM_PROBE=` prints per animated part whether it is registered, whether its loop moves, what its table row holds and whether it is drawn, and `WOW_WORLDVIEW_AT=x,y[,z]` opens the server-less viewer anywhere; measured at the reported waterfall: 0 uv / 0 tint entries before, 87 uv / 6 tint around the Elwynn valley after, its two sheets on slots 1 and 2 with live rows −0.2122 and −0.4246 — the authored 1:2 ratio (2038); and the paper doll and the inspect pane are lit from the model's right: `CGLight+0x24` is the direction the light PROPAGATES — the `CharacterModelBase` ctor `0x505680` writes `(0, 1, 0)` and `0x71bce0` negates all three components before building the SH basis — so `model_pane_light()` hands the packer the ctor's vector as written where 0638 had negated it on the way in and lit both panes from the left; the ambient `(0.7, 0.7, 0.7)`, the diffuse `(0.8, 0.8, 0.64)` and the side-light analysis all hold, which is why the A/B that approved 0638 could not tell the two readings apart (2034) - a unit tooltip whose name query has not answered titles `UNKNOWNOBJECT`, not a blank gold row: the reference's builder `0x529fe0` reads its NAME line through `CGUnit_C::GetUnitName 0x609210` — the same function `UnitName 0x517020` delegates to — and every miss on its three legs, the null creature record included, ends at the one `FrameScript_GetText("UNKNOWNOBJECT")` tail at `0x609324`, so `unknownobject` moves into `unit` proper and the plate and the verb share it, read out of the VM's `_G` with the binary's "Unknown Being" behind a missing global; the faction line stops waiting for the creature record — its entry gate `0x612610` returns 1 when there is none, so it resolves off `UNIT_FIELD_FACTIONTEMPLATE` and keeps only `HIDE_FACTION_TOOLTIP` for when the record lands; the reference's placeholder sticks for the whole hover (stock `GameTooltip.xml` registers `UPDATE_MOUSEOVER_UNIT` alone) and our line-change repaint, which replaces it when the answer lands, is kept (2040); a unit predicate answers the NUMBER 1 or nil, never a Lua boolean: `UnitExists`, `UnitIsDead`, `UnitIsGhost`, `UnitIsDeadOrGhost` and `UnitOnTaxi` pushed a boolean and twenty-three more open-coded the right shape in eighteen places, where the reference pushes through `lua_pushnumber 0x6f3810` or `lua_pushnil 0x6f37f0`; all twenty-eight now reach 1830's one push site `binding_abi::predicate` — via `unit::unit_predicate` for a `UnitState` field, directly otherwise — `UnitIsCharmed`'s `Number(1.0)` and `ClearTarget`'s hand-branched nil included, and `UnitOnTaxi` gains the reference's `Usage:` raise (gate `0x517a48`) where a nil token had raised mlua's type error; truthiness is why it survived (`if UnitIsDead(u)` reads the same either way, and `== nil` inverts), nine `eval::` tests were vacuous under mlua's coercion and `taxi.rs` had pinned `== false`/`== true` — the one shape the reference cannot emit — so a table-driven test covers all twenty-eight on both legs, and `UnitIsAFK`/`UnitIsDND` now say they are ours (zero occurrences image-wide) (2043); and the quest log's stock title width cap — `275 − 15 − tagWidth`, live off the chain since 1944 — ellipsizes on one line rather than wrapping into the 16-unit row: 1873 declined it reading an explicit width as a wrap width, but the title FontString declares `y="10"`, and a declared height arms both overflow regimes, so a 258 px title in a 206 px box paints as one line ending in three ASCII dots — the reference's own result, its ellipsis gate at `0x771ec0` being `boxW > 0 && (maxLines != 0 || boxH > 0)`, which only an auto-height FontString escapes; two tests pin the render law on the real FRIZQT face and the geometry the stock file hands it, including the cap's per-row-button stickiness, which is the reference's too (2036) Co-Authored-By: Claude Fable 5.1 --- crates/benilla-app/src/net.rs | 22 + crates/benilla-app/src/net/apply.rs | 107 ++-- crates/benilla-app/src/net/apply/loot.rs | 289 ++++++++++- crates/benilla-app/src/net/apply/pet.rs | 205 +++++++- crates/benilla-app/src/net/apply/spells.rs | 2 + crates/benilla-app/src/portrait/light.rs | 52 +- crates/benilla-app/src/sound/creature.rs | 387 +++++++++++++-- crates/benilla-app/src/sound/kit.rs | 69 ++- crates/benilla-app/src/ui_action/cast_fail.rs | 457 +++++++++++++++--- crates/benilla-app/src/ui_action/errors.rs | 70 ++- crates/benilla-app/src/ui_action/feed.rs | 44 +- .../benilla-app/src/ui_action/feed_tests.rs | 144 +++++- crates/benilla-app/src/ui_action/mod.rs | 5 +- crates/benilla-app/src/ui_loot.rs | 69 +-- .../benilla-app/src/ui_script/death_tests.rs | 12 +- .../src/ui_script/questlog_tests.rs | 109 +++++ .../src/ui_script/tooltip_anchor_tests.rs | 47 ++ .../src/ui_script/unit_frame_tests.rs | 7 +- crates/benilla-app/src/ui_text/layout/mod.rs | 74 +++ crates/benilla-app/src/ui_unit.rs | 144 +++++- crates/benilla-app/tests/message_keys.rs | 58 ++- crates/benilla-formats/src/creature_sound.rs | 135 +++++- crates/benilla-protocol/src/events.rs | 16 + crates/benilla-protocol/src/events/decode.rs | 12 + crates/benilla-protocol/src/messages/mod.rs | 13 +- .../benilla-protocol/src/messages/opcode.rs | 68 ++- .../benilla-protocol/src/messages/packet.rs | 26 + crates/benilla-protocol/src/messages/parse.rs | 15 + crates/benilla-protocol/src/messages/pet.rs | 68 ++- .../benilla-protocol/tests/simple_packets.rs | 105 +++- crates/benilla-ui/src/script/taxi.rs | 24 +- crates/benilla-ui/src/script/tests/taxi.rs | 4 +- .../src/script/tests/tooltip_unit.rs | 127 +++++ crates/benilla-ui/src/script/tooltip_unit.rs | 30 +- crates/benilla-ui/src/script/unit/bindings.rs | 191 +++----- crates/benilla-ui/src/script/unit/mod.rs | 65 ++- crates/benilla-ui/src/script/unit/tests.rs | 405 +++++++++++++++- .../benilla-world/src/doodad_anim/mat_anim.rs | 211 +++++++- crates/benilla-world/src/doodad_anim/mod.rs | 3 + crates/benilla-world/src/model_render/lazy.rs | 113 +++++ crates/benilla-world/src/worldview.rs | 31 +- 41 files changed, 3488 insertions(+), 547 deletions(-) diff --git a/crates/benilla-app/src/net.rs b/crates/benilla-app/src/net.rs index c98a6fac..5f10ca00 100644 --- a/crates/benilla-app/src/net.rs +++ b/crates/benilla-app/src/net.rs @@ -117,6 +117,8 @@ impl Plugin for NetPlugin { .add_message::() .add_message::() .add_message::() + .add_message::() + .add_message::() .add_message::() .add_message::() .add_message::() @@ -2632,6 +2634,26 @@ pub(crate) struct AiReactionMessage { pub(crate) hostile: bool, } +/// The pet spoke (`SMSG_PET_ACTION_SOUND`, bridged from the Net drain; decision 2039): `talk` is +/// the wire's own selector (`PET_TALK_ORDER` / `PET_TALK_ATTACK`), not a `SoundEntries` id — the +/// kit comes off the pet's own `CreatureSoundData` row. Pure audio in the client (`0x6040c0` +/// resolves the guid and calls the bark dispatcher, nothing else); consumer: `sound::creature`. +#[derive(Message, Clone, Copy)] +pub(crate) struct PetTalkMessage { + pub(crate) unit: Entity, + pub(crate) talk: u32, +} + +/// A dismissed pet's parting sound (`SMSG_PET_DISMISS_SOUND`, decision 2039): a +/// `CreatureModelData` id and the point, already in Bevy space. **No entity** — the packet names +/// no guid and the pet is gone by the time it arrives, which is the whole reason it carries a +/// position at all. Consumer: `sound::creature`. +#[derive(Message, Clone, Copy)] +pub(crate) struct PetDismissSoundMessage { + pub(crate) model_id: u32, + pub(crate) pos: Vec3, +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/benilla-app/src/net/apply.rs b/crates/benilla-app/src/net/apply.rs index 562f88f7..ffc6ec5a 100644 --- a/crates/benilla-app/src/net/apply.rs +++ b/crates/benilla-app/src/net/apply.rs @@ -11,8 +11,9 @@ use bevy::prelude::*; use super::{ AiReactionMessage, CharActionResultMessage, CharListMessage, EmoteMessage, EnteredWorldMessage, Guid, GuidIndex, LoggedOutMessage, NetCommands, NetEvents, NetStatus, ObjectStore, - PendingTransfer, RemoteMotion, Reputations, SelfGuid, SelfPlayer, ServerSoundMessage, - ServerTime, ServerWallClock, TeleportMessage, UnitMoveModes, WorldportMessage, + PendingTransfer, PetDismissSoundMessage, PetTalkMessage, RemoteMotion, Reputations, SelfGuid, + SelfPlayer, ServerSoundMessage, ServerTime, ServerWallClock, TeleportMessage, UnitMoveModes, + WorldportMessage, }; use benilla_world::weather::WeatherMessage; @@ -327,10 +328,13 @@ pub(crate) fn apply_net_updates( // drain sends it through the one cast path. Rides here for the same reason the // catalog does: this is where the ceiling left room. ResMut, + // The taming-refusal queue (decision 2039) — `SMSG_PET_TAME_FAILURE`'s reason byte. + // Its own queue rather than `UiErrorKeys` because its message needs TWO GlobalStrings + // lookups, and the inner one is only reachable at the drain (see the resource's doc). + ResMut, ), ResMut, ResMut, - ResMut, ResMut, ResMut, ResMut, @@ -398,6 +402,12 @@ pub(crate) fn apply_net_updates( // auto-attack start (`0x6e83e7`, decision 1593). Filter-only, so it conflicts with // nothing else in this drain. Query<(), (With, With)>, + // The pet's bark (`SMSG_PET_ACTION_SOUND`, decision 2039) — `AiReactionMessage`'s + // sibling: both are pure audio that resolves a guid and hands the sound layer a + // state for the SAME `0x623a40` dispatcher. Nested for the same reason as its + // neighbours: the outer tuple is at the 16-param ceiling. + MessageWriter, + MessageWriter, ), ), // The aura feed's duration side-table + the clock to stamp arrivals (decisions 0255/0257): the @@ -550,7 +560,7 @@ pub(crate) fn apply_net_updates( index: &index, factions: ui_actions.1 .2.as_deref(), reputations: &reputations, - spells: ui_actions.11.as_deref(), + spells: ui_actions.10.as_deref(), } }; } @@ -1089,7 +1099,7 @@ pub(crate) fn apply_net_updates( SessionEvent::SpellBook { spell_ids, cooldowns, - } => spell_book(spell_ids, cooldowns, &mut ui_actions.0, &mut ui_actions.10), + } => spell_book(spell_ids, cooldowns, &mut ui_actions.0, &mut ui_actions.9), SessionEvent::ActionButtons { buttons } => action_buttons(buttons, &mut ui_actions.0), SessionEvent::SpellLearned { spell_id } => learned_spell(spell_id, &mut ui_actions.0), SessionEvent::SpellRemoved { spell_id } => removed_spell(spell_id, &mut ui_actions.0), @@ -1113,12 +1123,12 @@ pub(crate) fn apply_net_updates( &mut ui_actions.1 .0, &audio.4, &mut audio.5, - &mut ui_actions.5, + &mut ui_actions.4, + &mut ui_actions.8, + &mut ui_actions.14, &mut ui_actions.9, - &mut ui_actions.15, - &mut ui_actions.10, - &mut ui_actions.12, - ui_actions.11.as_deref(), + &mut ui_actions.11, + ui_actions.10.as_deref(), &net_commands, &mut ui_actions.1 .3, play_seq.next(), @@ -1134,8 +1144,8 @@ pub(crate) fn apply_net_updates( item_guid, bag_slot, &mut ui_actions.2, + &mut ui_actions.5, &mut ui_actions.6, - &mut ui_actions.7, &mut loot_latch, ), SessionEvent::Chat(m) => { @@ -1170,9 +1180,9 @@ pub(crate) fn apply_net_updates( SessionEvent::ChatRestricted => { chat::broadcast(crate::ui_chat::Broadcast::ChatRestricted, &mut chat_log) } - SessionEvent::Notification { text } => chat::notification(text, &mut ui_actions.14), + SessionEvent::Notification { text } => chat::notification(text, &mut ui_actions.13), SessionEvent::AreaTriggerMessage { text } => { - chat::area_trigger_message(text, &mut ui_actions.14) + chat::area_trigger_message(text, &mut ui_actions.13) } SessionEvent::PlayedTime { total, level } => { // BOTH halves, and they are not redundant. The chat breakdown is our stand-in for @@ -1419,7 +1429,7 @@ pub(crate) fn apply_net_updates( &net_commands, ), SessionEvent::LootError { guid, error } => { - loot_error(guid, error, &mut ui_actions.4, &mut loot_latch) + loot_error(guid, error, &mut ui_error_keys, &mut loot_latch) } SessionEvent::LootRemoved { slot } => loot_removed(slot, &mut loot), SessionEvent::LootMoneyNotify { amount } => loot_money_notify(amount), @@ -1520,7 +1530,7 @@ pub(crate) fn apply_net_updates( &index, &self_guid, &stores, - ui_actions.11.as_deref(), + ui_actions.10.as_deref(), &mut audio.7, &mut audio.15 .0, &mut audio.15 .1, @@ -1539,7 +1549,7 @@ pub(crate) fn apply_net_updates( &index, &self_guid, &stores, - ui_actions.11.as_deref(), + ui_actions.10.as_deref(), &mut audio.7, &mut audio.15 .0, &mut audio.15 .1, @@ -1669,9 +1679,9 @@ pub(crate) fn apply_net_updates( &index, &mut audio.5, &self_guid, - &mut ui_actions.5, - &mut ui_actions.9, - ui_actions.11.as_deref(), + &mut ui_actions.4, + &mut ui_actions.8, + ui_actions.10.as_deref(), play_seq.next(), ), SessionEvent::SpellGo { @@ -1703,21 +1713,21 @@ pub(crate) fn apply_net_updates( &mut audio.6, &self_guid, &stores, - &mut ui_actions.5, - &mut ui_actions.9, - &mut ui_actions.15, + &mut ui_actions.4, + &mut ui_actions.8, + &mut ui_actions.14, &mut audio.7, &mut audio.10, &mut loot_latch, ( - &mut ui_actions.10, - ui_actions.11.as_deref(), + &mut ui_actions.9, + ui_actions.10.as_deref(), &mut items, &net_commands, &mut pet_bar, ), ( - &mut ui_actions.12, + &mut ui_actions.11, &mut audio.15 .2, !audio.15 .4.is_empty(), ), @@ -1736,20 +1746,20 @@ pub(crate) fn apply_net_updates( &audio.4, &mut audio.5, &self_guid, - &mut ui_actions.5, - &mut ui_actions.9, - &mut ui_actions.15, + &mut ui_actions.4, + &mut ui_actions.8, + &mut ui_actions.14, play_seq.next(), ), SessionEvent::SpellDelayed { caster, delay_ms } => spell_delayed( caster, delay_ms, &self_guid, - &mut ui_actions.5, - &mut ui_actions.9, + &mut ui_actions.4, + &mut ui_actions.8, ), SessionEvent::CancelAutoRepeat => cancel_auto_repeat( - &mut ui_actions.12, + &mut ui_actions.11, &self_guid, &index, &mut commands, @@ -1757,15 +1767,15 @@ pub(crate) fn apply_net_updates( ), SessionEvent::SpellCooldowns { caster, cooldowns } => { if let Some(store) = - addressed_store(caster, &self_guid, &mut ui_actions.10, &mut pet_bar) + addressed_store(caster, &self_guid, &mut ui_actions.9, &mut pet_bar) { - spell_cooldowns(caster, cooldowns, ui_actions.11.as_deref(), store); + spell_cooldowns(caster, cooldowns, ui_actions.10.as_deref(), store); } } SessionEvent::ItemCooldown { item_guid, spell_id, - } => item_cooldown(item_guid, spell_id, &items, &mut ui_actions.10), + } => item_cooldown(item_guid, spell_id, &items, &mut ui_actions.9), // The item-lifetime countdown's ONLY feed (decision 1933): park the deadline on the // item store, exactly as the enchant timer below does — vmangos's own writer says the // `ITEM_FIELD_DURATION` field the client also holds is not what it displays from. @@ -1781,21 +1791,21 @@ pub(crate) fn apply_net_updates( } => items.set_enchant_deadline(item_guid, slot, seconds), SessionEvent::CooldownEvent { spell_id, caster } => { if let Some(store) = - addressed_store(caster, &self_guid, &mut ui_actions.10, &mut pet_bar) + addressed_store(caster, &self_guid, &mut ui_actions.9, &mut pet_bar) { cooldown_event(spell_id, caster, store); } } SessionEvent::ClearCooldown { spell_id, caster } => { if let Some(store) = - addressed_store(caster, &self_guid, &mut ui_actions.10, &mut pet_bar) + addressed_store(caster, &self_guid, &mut ui_actions.9, &mut pet_bar) { clear_cooldown(spell_id, caster, store); } } SessionEvent::CooldownCheat { caster } => { if let Some(store) = - addressed_store(caster, &self_guid, &mut ui_actions.10, &mut pet_bar) + addressed_store(caster, &self_guid, &mut ui_actions.9, &mut pet_bar) { cooldown_cheat(caster, store); } @@ -1803,7 +1813,7 @@ pub(crate) fn apply_net_updates( // The pet action bar (decision 0982) — server-authoritative, so PET_SPELLS is a // wholesale replace and its zero-guid form is the teardown. SessionEvent::PetSpells(spells) => { - pet::pet_spells(*spells, ui_actions.11.as_deref(), &mut pet_bar) + pet::pet_spells(*spells, ui_actions.10.as_deref(), &mut pet_bar) } SessionEvent::PetMode(mode) => pet::pet_mode(mode, &mut pet_bar), SessionEvent::PetActionFeedback { reason } => { @@ -1812,12 +1822,25 @@ pub(crate) fn apply_net_updates( SessionEvent::PetCastFailed { spell_id, reason } => { pet::pet_cast_failed(spell_id, reason, &mut ui_actions.1 .0) } + // The three pet-feedback arms and the pet's voice (decision 2039). Each was + // name-table-only until then; each is something the reference visibly does. + SessionEvent::PetTameFailure { reason } => { + pet::pet_tame_failure(reason, &mut ui_actions.1 .4) + } + SessionEvent::PetNameInvalid => pet::pet_name_invalid(&mut ui_error_keys), + SessionEvent::PetBroken => pet::pet_broken(&mut ui_error_keys), + SessionEvent::PetActionSound { pet_guid, talk } => { + pet::pet_action_sound(pet_guid, talk, &index, &mut audio.15 .5) + } + SessionEvent::PetDismissSound { model_id, position } => { + pet::pet_dismiss_sound(model_id, position, &mut audio.15 .6) + } SessionEvent::ChannelStart { spell_id, duration_ms, - } => channel_start(spell_id, duration_ms, &mut ui_actions.13, &mut ui_actions.5), + } => channel_start(spell_id, duration_ms, &mut ui_actions.12, &mut ui_actions.4), SessionEvent::ChannelUpdate { remaining_ms } => { - channel_update(remaining_ms, &mut ui_actions.13, &mut ui_actions.5) + channel_update(remaining_ms, &mut ui_actions.12, &mut ui_actions.4) } SessionEvent::AuraDuration { slot, remaining_ms } => { aura_duration(slot, remaining_ms, &mut aura.0, aura.1.elapsed_secs_f64()) @@ -1936,7 +1959,7 @@ pub(crate) fn apply_net_updates( npc::trainer_buy_succeeded(trainer, spell_id, &mut trainer_open, &net_commands) } SessionEvent::TrainerBuyFailed { error, .. } => { - npc::trainer_buy_failed(error, &mut ui_actions.8 .0) + npc::trainer_buy_failed(error, &mut ui_actions.7 .0) } SessionEvent::InvalidatePlayer { guid } => names::invalidate_player(guid, &mut names), SessionEvent::ListStabledPets { @@ -1947,7 +1970,7 @@ pub(crate) fn apply_net_updates( SessionEvent::StableResult { result } => npc::stable_result( result, &mut stable_open, - &mut ui_actions.8 .1, + &mut ui_actions.7 .1, &net_commands, ), SessionEvent::TaxiNodesShown { diff --git a/crates/benilla-app/src/net/apply/loot.rs b/crates/benilla-app/src/net/apply/loot.rs index 9f514b37..b3e0790b 100644 --- a/crates/benilla-app/src/net/apply/loot.rs +++ b/crates/benilla-app/src/net/apply/loot.rs @@ -16,7 +16,7 @@ use crate::net::ClientCommand; use crate::pending_item_ops::{LockTransitions, PendingItemOps}; use crate::ui_action::{UiError, UiErrorKeys}; use crate::ui_items::{EquipError, EquipErrors}; -use crate::ui_loot::{LootErrors, LootLatch, LootState}; +use crate::ui_loot::{LootLatch, LootState}; use crate::ui_loot_roll::LootRolls; /// The wire `loot_type` values `SMSG_LOOT_RESPONSE`'s **cold-latch** admission accepts @@ -106,20 +106,125 @@ pub(super) fn fish_verdict(escaped: bool, errors: &mut UiErrorKeys) { errors.0.push(UiError::key(key)); } +/// The **default** arm releases — VERIFIED at the bytes (wow-re §5, +/// `system/object-layer/scratch/loot-anim-leg.md` §7.1). +/// +/// The out-of-range branch is not merely *adjacent* to code 7's arm, it **is** code 7's arm: +/// +/// ```text +/// 5eba11: 0f 87 a6 00 00 00 ja 0x5ebabd ; 0x5eba17 + 0xa6 = 0x5ebabd +/// 5ebabd: 68 83 00 00 00 push 0x83 ; 5 bytes, ending exactly at 0x5ebac2 +/// 5ebac2: e8 59 ac ea ff call 0x496720 ; the release tail, reached by FALL-THROUGH +/// ``` +/// +/// Jump-table slot 3 (`0x5ebc70`) holds the same `bd ba 5e 00`, a dword that occurs exactly once +/// in the image. So every code the table does not name shows `ERR_LOOT_DIDNT_KILL` **and** ends +/// the kneel — and the default's reach is wider than the enum suggests: `0x5eba07` is a `movzx` +/// and the `ja` is **unsigned**, so 0..3 and 15..255 all land here. +const DEFAULT_ARM_RELEASES: bool = true; + +/// One arm of the loot-refusal jump table: the message it displays, and whether it falls through +/// the **release tail**. +/// +/// Both halves are the reference's, read off the same eleven-entry table — each arm is a +/// `push ` that either falls into the tail at `0x5ebac2` or jumps past it, so "what does +/// it say" and "does the kneel end" are one decision in the bytes and one decision here. +struct LootRefusal { + /// The `GlobalStrings` key of the id the arm hands `CGGameUI::DisplayError` (`0x496720`). + key: &'static str, + /// Whether the arm falls into the release tail at `0x5ebac2` — zero the pending-loot guid + /// (`[player+0x1d28/+0x1d2c]`), `UnlockItem 0x495420`, `RecomputeBaseAnim 0x5fd9e0(-1)`. + /// That guid **is** benilla's [`LootLatch`], so this is exactly what ends the + /// client-predicted loot kneel. + releases: bool, +} + +/// The refusal a `SMSG_LOOT_RESPONSE` error code produces — **the reference's own jump table**, +/// transcribed (wow-re `system/ui/scratch/loot-slot-record.md` §1, byte-verified). +/// +/// The error leg is entered on `lootType == 0` (`0x5eb9f4`), reads the code (`0x5eba02`) and +/// dispatches `0x5eba0b add eax,-4; cmp eax,0xa; ja ; jmp [4*eax + 0x5ebc64]`. The +/// eleven-entry table read out of the PE gives: +/// +/// | wire code | stringId | key | releases | +/// |---|---|---|---| +/// | 4 `TOO_FAR` | `0x82` (130) | `ERR_LOOT_TOO_FAR` | yes | +/// | 5 `BAD_FACING` | `0x84` (132) | `ERR_LOOT_BAD_FACING` | yes | +/// | 6 `LOCKED` | `0x81` (129) | `ERR_LOOT_LOCKED` | yes | +/// | 7 (unnamed server-side) | `0x83` (131) | `ERR_LOOT_DIDNT_KILL` | yes | +/// | 8 `NOTSTANDING` | `0x85` (133) | `ERR_LOOT_NOTSTANDING` | yes | +/// | 9 `STUNNED` | `0x86` (134) | `ERR_LOOT_STUNNED` | yes | +/// | 10 `PLAYER_NOT_FOUND` | `0x19b` (411) | `ERR_LOOT_PLAYER_NOT_FOUND` | **no** | +/// | 11 `PLAY_TIME_EXCEEDED` | `0x1bf` (447) | `ERR_PLAY_TIME_EXCEEDED` | yes | +/// | 12 `MASTER_INV_FULL` | `0x1cd` (461) | `ERR_LOOT_MASTER_INV_FULL` | **no** | +/// | 13 `MASTER_UNIQUE_ITEM` | `0x1ce` (462) | `ERR_LOOT_MASTER_UNIQUE_ITEM` | **no** | +/// | 14 `MASTER_OTHER` | `0x1cf` (463) | `ERR_LOOT_MASTER_OTHER` | **no** | +/// +/// **The four that do not release are the four that arrive at an already-open window** — the +/// master looter's three `CMSG_LOOT_MASTER_GIVE` refusals (decision 1675) and the dead/absent +/// recipient. The corpse stays open and the character stays kneeling, which is why the reference +/// jumps past the tail for exactly these. +/// +/// **Everything outside 4..=14 takes the default arm**, `ERR_LOOT_DIDNT_KILL` — including code +/// `0` (`DIDNT_KILL` itself, which reaches its own sentence only through the default) and vmangos's +/// `ALREADY_PICKPOCKETED` (15) / `NOT_WHILE_SHAPESHIFTED` (16). 1.12 ships **no** string for an +/// already-picked pocket or a shapeshifted refusal; the client says "You don't have permission to +/// loot that corpse." for both, and inventing a better sentence is not fidelity. +fn loot_refusal(reason: u8) -> LootRefusal { + use benilla_protocol::messages::loot_error as e; + // `7` has no vmangos name — it is a gap in the server enum — but it is a real arm of the + // client's table, and a *releasing* one, which is why it cannot simply fall to the default. + const UNNAMED_DIDNT_KILL: u8 = 7; + let (key, releases) = match reason { + e::TOO_FAR => ("ERR_LOOT_TOO_FAR", true), + e::BAD_FACING => ("ERR_LOOT_BAD_FACING", true), + e::LOCKED => ("ERR_LOOT_LOCKED", true), + UNNAMED_DIDNT_KILL => ("ERR_LOOT_DIDNT_KILL", true), + e::NOTSTANDING => ("ERR_LOOT_NOTSTANDING", true), + e::STUNNED => ("ERR_LOOT_STUNNED", true), + e::PLAYER_NOT_FOUND => ("ERR_LOOT_PLAYER_NOT_FOUND", false), + e::PLAY_TIME_EXCEEDED => ("ERR_PLAY_TIME_EXCEEDED", true), + e::MASTER_INV_FULL => ("ERR_LOOT_MASTER_INV_FULL", false), + e::MASTER_UNIQUE_ITEM => ("ERR_LOOT_MASTER_UNIQUE_ITEM", false), + e::MASTER_OTHER => ("ERR_LOOT_MASTER_OTHER", false), + _ => ("ERR_LOOT_DIDNT_KILL", DEFAULT_ARM_RELEASES), + }; + LootRefusal { key, releases } +} + /// The server refused to open the loot window (`SMSG_LOOT_RESPONSE`'s error shape — didn't kill -/// it, too far, not standing, …). The refusal also ends the client-predicted kneel: the latch -/// armed at the `CMSG_LOOT` send drops (guid-matched), or the character would kneel forever at a -/// corpse whose window never opened (decision 0515). -pub(super) fn loot_error( - guid: u64, - error: u8, - loot_errors: &mut LootErrors, - latch: &mut LootLatch, -) { - // The server refused to open the window → the red UI error line (equip-error path). - debug!("net: loot error {error} on {guid:#x}"); - loot_errors.0.push(error); - latch.clear_for(guid); +/// it, too far, not standing, …): the message by **GlobalStrings key**, never a sentence of ours, +/// and the kneel dropped only on the arms that release ([`loot_refusal`]). +/// +/// The key route is not a style preference. The reference displays every one of these through +/// `DisplayError(stringId)`, so the *surface* (red `UIErrorsFrame`, from the catalog row's `+0x04`) +/// and the *voice line* (`+0x0c` — four of these speak in the player's own race and gender) are +/// properties of the message, and a hardcoded English string throws all three away along with +/// localization. Before this, benilla composed its own eight sentences here and six of them said +/// something the client never says. +/// +/// **Two gaps this deliberately does not close** (surfaced by the wow-re §5, named here rather +/// than left to a later bug report): +/// +/// 1. **A guid-MISMATCHED error takes a different arm in the reference.** The error leg is +/// reachable only past the admission gate, so the reference only ever *displays* a refusal for +/// the session it asked about; a mismatched one falls to `0x5eb963`, which clears the latch +/// unconditionally, shows **nothing**, and (because `lootType == 0`) sends nothing. benilla has +/// no admission gate on the error shape — it displays the line and keeps the latch. Reachable +/// only under the corpse-switch race, and it belongs with the gate (1477), not with the text. +/// 2. **`UnlockItem` fires `ITEM_LOCK_CHANGED` (188)** on the way through the tail, clearing +/// `[item+0x314]` bit 0. That matters only when the loot source is an ITEM guid — a lockbox — +/// where a refused open should drop the item's pending lock; for a corpse or chest guid there +/// is no item to unlock. benilla does not touch [`LockTransitions`] here. +pub(super) fn loot_error(guid: u64, error: u8, errors: &mut UiErrorKeys, latch: &mut LootLatch) { + let LootRefusal { key, releases } = loot_refusal(error); + debug!("net: loot error {error} on {guid:#x} → {key} (releases: {releases})"); + errors.0.push(UiError::key(key)); + if releases { + // The latch armed at the `CMSG_LOOT` send drops (guid-matched — see [`LootLatch`]), or the + // character would kneel forever at a corpse whose window never opened (decision 0515). + latch.clear_for(guid); + } } /// One loot-window row was taken, by anyone (`SMSG_LOOT_REMOVED`) — the UI clears that row. @@ -319,6 +424,162 @@ mod tests { assert_eq!(g("ERR_FISH_ESCAPED"), "Your fish got away!"); } + /// **The refusal table is the reference's, by message id.** Each key must be the catalog row + /// whose id the binary's own jump table pushes to `DisplayError` (wow-re + /// `loot-slot-record.md` §1: `4→0x82 · 5→0x84 · 6→0x81 · 7→0x83 · 8→0x85 · 9→0x86 · + /// 10→0x19b · 11→0x1bf · 12→0x1cd · 13→0x1ce · 14→0x1cf`, default `0x83`). + /// + /// Asserting the **id** rather than the spelling is the point: the key text is only a name for + /// a number the client pushes, and the catalog is generated from that same table (decision + /// 1770). A key that reads plausibly but names the wrong row cannot survive this. + #[test] + fn every_refusal_names_the_message_id_the_reference_pushes() { + const TABLE: &[(u8, u16)] = &[ + (4, 0x82), + (5, 0x84), + (6, 0x81), + (7, 0x83), + (8, 0x85), + (9, 0x86), + (10, 0x19b), + (11, 0x1bf), + (12, 0x1cd), + (13, 0x1ce), + (14, 0x1cf), + // The default arm: code 0 is `DIDNT_KILL` itself, 15 `ALREADY_PICKPOCKETED` and 16 + // `NOT_WHILE_SHAPESHIFTED` have no string of their own, and 1/2/3 are server-enum gaps. + (0, 0x83), + (1, 0x83), + (15, 0x83), + (16, 0x83), + (200, 0x83), + ]; + for &(code, id) in TABLE { + let key = loot_refusal(code).key; + let record = benilla_ui::messages::by_key(key) + .unwrap_or_else(|| panic!("code {code} named {key}, which is not a catalog row")); + assert_eq!( + record.id, id, + "code {code} → {key} (id {}), but the reference pushes {id:#x}", + record.id + ); + // Every one of these is a red `UIErrorsFrame` line, never a chat line — the record's + // `+0x04`, which is what benilla threw away by composing its own sentence. + assert_eq!( + record.kind, + benilla_ui::messages::MsgKind::Error, + "{key} is not a red line" + ); + } + + // **The four commonest refusals are SPOKEN**, in the player's own race and gender — their + // `type_tag` (`+0x0c`) is a `VocalUIEnum` line id rather than the `0x44` "play the cue" + // sentinel. A hardcoded sentence has no record, so it had no voice either; this is the + // third thing the key route gets back, after the wording and the surface. + let voiced = |key: &str| { + usize::from(benilla_ui::messages::by_key(key).expect(key).type_tag) + < benilla_formats::VOCAL_UI_LINES + }; + for key in [ + "ERR_LOOT_DIDNT_KILL", + "ERR_LOOT_BAD_FACING", + "ERR_LOOT_LOCKED", + "ERR_LOOT_TOO_FAR", + ] { + assert!(voiced(key), "{key} lost its voice line"); + } + // …and the rest are cue-tagged, so the split is asserted from both sides. + for key in ["ERR_LOOT_NOTSTANDING", "ERR_LOOT_MASTER_OTHER"] { + assert!(!voiced(key), "{key} unexpectedly carries a voice line"); + } + } + + /// **The regression for the invented English.** Every refusal key must resolve in the shipped + /// 1.12 `GlobalStrings.lua` to the sentence the real client shows. benilla used to compose + /// these itself and six of the eight said something 1.12 never says — "You are too far away to + /// loot that." for a string that ends "that corpse.", "You can't loot that from there." for + /// "You must be facing the corpse to loot it.", and a wholly invented "Those pockets are + /// already empty." for a code that has no string at all. Skips without client data. + #[test] + fn refusal_keys_resolve_to_the_real_1_12_sentences() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let src = chain + .read_file("Interface\\FrameXML\\GlobalStrings.lua") + .expect("GlobalStrings.lua in the chain"); + let s = benilla_ui::script::UiScript::new().expect("VM"); + s.run(&String::from_utf8_lossy(&src)).expect("runs clean"); + let g = |key: &str| s.lua().globals().get::(key).expect(key); + + for (code, want) in [ + (4u8, "You are too far away to loot that corpse."), + (5, "You must be facing the corpse to loot it."), + (6, "Someone is already looting that corpse."), + (7, "You don't have permission to loot that corpse."), + (8, "You need to be standing up to loot something!"), + (9, "You can't loot anything while stunned!"), + (10, "Player not found"), + (11, "Maximum play time exceeded"), + (12, "That player's inventory is full"), + (13, "Player has too many of that item already"), + (14, "Can't assign item to that player"), + // The default arm, including the two vmangos codes 1.12 has no sentence for. + (0, "You don't have permission to loot that corpse."), + (15, "You don't have permission to loot that corpse."), + (16, "You don't have permission to loot that corpse."), + ] { + assert_eq!(g(loot_refusal(code).key), want, "wire code {code}"); + } + } + + /// **The four refusals that arrive at an already-open window do not end the kneel.** The + /// reference's arms for 10 and the master trio jump *past* the release tail at `0x5ebac2`; the + /// rest fall into it. A master looter refused a hand-off keeps kneeling at the open corpse. + #[test] + fn only_the_window_less_refusals_drop_the_kneel() { + // The named releasing arms, plus the default — `ja 0x5ebabd` lands *on* code 7's arm and + // falls through the tail, and the compare is unsigned, so 0..3 and 15..255 all release. + for code in [4u8, 5, 6, 7, 8, 9, 11, 0, 1, 2, 3, 15, 16, 200, 255] { + let mut errors = UiErrorKeys::default(); + let mut latch = LootLatch(Some(CORPSE)); + loot_error(CORPSE, code, &mut errors, &mut latch); + assert_eq!(latch.0, None, "code {code} should release"); + assert_eq!(errors.0.len(), 1, "code {code} shows exactly one line"); + } + for code in [10u8, 12, 13, 14] { + let mut errors = UiErrorKeys::default(); + let mut latch = LootLatch(Some(CORPSE)); + loot_error(CORPSE, code, &mut errors, &mut latch); + assert_eq!( + latch.0, + Some(CORPSE), + "code {code} answers an OPEN window — the kneel stays" + ); + } + } + + /// The **error leg's** clear stays guid-matched, and the wow-re §5 showed this is not the + /// deviation 0515 took it for: it is behaviourally *identical* to the reference. + /// + /// The tail's own store is unconditional, but the tail cannot be reached on a mismatch. + /// `0x5eb9f1` has exactly one predecessor image-wide — `0x5eb93e je`, which already required + /// latch ≠ 0 **and** both GUID dwords equal — and it has no fall-through predecessor + /// (`0x5eb9ee` is `ret 8`); the other predecessors of `0x5eb9f4` all require the loot type in + /// `{2,3,4}` and so leave at `0x5eb9f8 jne`. A mismatched error never gets here at all. + /// + /// Read this beside [`a_refusal_drops_whatever_latch_was_live_not_just_a_matching_one`], which + /// asserts the opposite for a **different arm**: the admission gate `0x5eb963`/`0x5eb9d2`, + /// which *is* reached on a mismatch and does clear unconditionally. Not a contradiction — a + /// distinction the handler draws, and the reason the mismatch case is a separate open gap + /// (see [`loot_error`]). + #[test] + fn the_error_legs_clear_is_guid_matched() { + let mut errors = UiErrorKeys::default(); + let mut latch = LootLatch(Some(CHEST)); + loot_error(CORPSE, 8, &mut errors, &mut latch); + assert_eq!(latch.0, Some(CHEST)); + } + /// A GameObject guid (HIGHGUID_GAMEOBJECT in the high dword) — a chest. const CHEST: u64 = 0xF110_0000_0000_1234; /// A creature guid — a corpse. diff --git a/crates/benilla-app/src/net/apply/pet.rs b/crates/benilla-app/src/net/apply/pet.rs index 5ebf5d66..2de8dcb1 100644 --- a/crates/benilla-app/src/net/apply/pet.rs +++ b/crates/benilla-app/src/net/apply/pet.rs @@ -6,11 +6,13 @@ //! [`crate::ui_pet`]'s, latched on the press because the server never answers one (that file's //! module doc has the why). The only state this file writes is what actually arrives on the wire. //! -//! The two refusal arms deliberately reuse the *player's* red-line queues rather than growing pet -//! copies: both resolve their text through the VM's own `GlobalStrings.lua` the same way, and the -//! cast-fail resolver keys its power family on the failing **spell's** record ([`cast_fail`]'s -//! `power_keys(spell.power_type)`), so a pet's focus ability already reads "Not enough focus" -//! with no pet-awareness anywhere in the display layer. +//! The two refusal arms share the player's red-line **queues** — one drain, one sink, one +//! GlobalStrings lookup — but they do **not** share his message table, and believing they did is +//! what decision 2033 corrects. The reference gives each of these packets its own handler with its +//! own reason -> errorId map (`0x4bdb70` for the feedback byte, `0x6e8eb0` for the cast refusal), +//! and both maps exist precisely to say "your **pet** is dead / rooted / out of range" where the +//! player's says "you are". So the caster rides the queue as far as [`cast_fail`], which picks the +//! table; everything downstream of that stays pet-unaware, as it should be. //! //! [`cast_fail`]: crate::ui_action @@ -20,7 +22,10 @@ use bevy::prelude::*; use benilla_protocol::messages::{PetMode, PetSpells}; -use crate::ui_action::{CastErrors, Spells, UiError, UiErrorKeys}; +use benilla_assets::coords::wow_to_bevy; + +use super::super::{GuidIndex, PetDismissSoundMessage, PetTalkMessage}; +use crate::ui_action::{CastErrors, PetTameFailures, Spells, UiError, UiErrorKeys}; use crate::ui_pet::PetBar; /// `SMSG_PET_SPELLS` — replace the whole bar, and reseed the pet's own cooldown store from the @@ -136,31 +141,151 @@ pub(super) fn pet_action_feedback(reason: u8, errors: &mut UiErrorKeys) { } } -/// The `SMSG_PET_ACTION_FEEDBACK` reason → its `GlobalStrings.lua` key. +/// The `SMSG_PET_ACTION_FEEDBACK` reason → the message-catalog row the reference raises for it. /// -/// vmangos sends exactly two (`Unit::SendPetActionFeedback`'s call sites): `1` when the pet cannot -/// path to the ordered spot, `2` when the ordered target is out of its reach. Both keys ship in -/// 1.12's `GlobalStrings.lua` (`PET_SPELL_NOPATH` at l.3054, `SPELL_FAILED_OUT_OF_RANGE`), so the -/// text — and its localization — comes from the VM like every other error line. +/// **The whole map, read off the handler** (`0x4bdb70`, decision 2033). The byte is decremented +/// and bounded — `dec eax; cmp eax,0x3; ja` — then indexes the four-entry jump table at +/// `0x4bdbe8`, so `0` and anything from `5` up fall past every arm and display nothing. Each arm +/// is a bare `push ; call CGGameUI::DisplayError 0x496720`, with no argument and no +/// state write: this opcode is a message and nothing else. +/// +/// | byte | vmangos `PetFeedback` | errorId | catalog row | +/// |---|---|---|---| +/// | `1` | `FEEDBACK_PET_DEAD` | `0x150` | `ERR_PET_SPELL_DEAD` | +/// | `2` | `FEEDBACK_NOTHING_TO_ATT` | `0x0a0` | `ERR_NO_ATTACK_TARGET` | +/// | `3` | `FEEDBACK_CANT_ATT_TARGET` | `0x0a1` | `ERR_INVALID_ATTACK_TARGET` | +/// | `4` | `FEEDBACK_NO_PATH_TO` | `0x151` | `ERR_PET_SPELL_NOPATH` | +/// +/// The four line up one-for-one with `Pet.h`'s enum and with its four `SendPetActionFeedback` +/// call sites. **That is what the two-entry map this replaces got wrong**: it read the sends as +/// "exactly two" and paired them with `1`/`2`, so a dead pet's refusal said "No path available +/// for your pet", a targetless attack said "Out of range", and `3`/`4` said nothing at all. +/// +/// **`ERR_PET_SPELL_NOPATH` has no `GlobalStrings.lua` string in 5875**, so a no-path order is +/// silent in the reference too — its own data-suppression face, not a gap here. The key +/// `PET_SPELL_NOPATH` ("No path available for your pet") *does* exist, which is what made the old +/// map look right on screen; it is not this row's key, and nothing in the shipped client raises +/// it. fn pet_feedback_key(reason: u8) -> Option<&'static str> { Some(match reason { - 1 => "PET_SPELL_NOPATH", - 2 => "SPELL_FAILED_OUT_OF_RANGE", + 1 => "ERR_PET_SPELL_DEAD", + 2 => "ERR_NO_ATTACK_TARGET", + 3 => "ERR_INVALID_ATTACK_TARGET", + 4 => "ERR_PET_SPELL_NOPATH", _ => return None, }) } -/// `SMSG_PET_CAST_FAILED` — the pet's cast refusal, queued onto the SAME red line as our own -/// through [`CastErrors`], because the resolver is spell-keyed and needs no pet-awareness. +/// `SMSG_PET_TAME_FAILURE` — the reason byte, straight onto [`PetTameFailures`] for the drain to +/// resolve (decision 2039). /// -/// What it deliberately does NOT do is touch our cast state: the caster is the pet, so there is no -/// pending cast of ours to revert, no GCD of ours to clear and no button of ours to unflash — the -/// three things `cast_result`'s own failure path does. Reusing that path would have made a pet's -/// refused Growl cancel the player's cast bar. +/// **Not only taming, despite the name**: vmangos raises it for Call Pet with no pet available +/// (`SpellEffects.cpp:3167`), Revive Pet on a live pet (`Spell.cpp:6136`), and any summon while a +/// pet is already out (`Spell.cpp:5463`) — which is why the reason vocabulary carries +/// `PETTAME_NOPETAVAILABLE`, `PETTAME_DEAD` and `PETTAME_NOTDEAD`, three values the taming spell +/// itself can never produce. +/// +/// No local state moves: the refusal is the server's last word on a spell that never took, and +/// the client's own cast bookkeeping was already unwound by the `SMSG_CAST_RESULT` that came with +/// it. +pub(super) fn pet_tame_failure(reason: u8, failures: &mut PetTameFailures) { + debug!( + "net: pet tame failure {reason} ({})", + benilla_protocol::messages::pet_tame_failure_key(reason) + ); + failures.0.push(reason); +} + +/// `SMSG_PET_NAME_INVALID` — the server refused the rename; raise `ERR_INVALID_PETNAME`. +/// +/// The packet carries nothing at all, so the opcode IS the message — which is exactly what the +/// reference's arm does (`0x5e3e33`: `push 0xf7; call 0x496720`, no body read). Decision 1066 +/// recorded the opposite ("a refused rename silently does nothing") from a carve that had +/// attributed `SMSG_PET_BROKEN`'s handler to this opcode; the two are different functions. +/// +/// Nothing else moves: the rename was optimistic-free by design (1066), so there is no local name +/// to roll back, and the popup has already closed. +pub(super) fn pet_name_invalid(errors: &mut UiErrorKeys) { + debug!("net: pet name refused"); + errors.0.push(UiError::key("ERR_INVALID_PETNAME")); +} + +/// `SMSG_PET_BROKEN` — the pet's loyalty hit zero and it left; raise `ERR_PET_BROKEN` +/// ("Your pet has run away"). +/// +/// Empty body, and the reference's handler `0x4bdc00` is five instructions that read none of it: +/// the *bar* teardown is not this packet's job. vmangos sends it immediately before +/// `Unsummon(PET_SAVE_AS_DELETED)` (`Pet.cpp:822`), so the zero-guid `SMSG_PET_SPELLS` that +/// actually clears the bar arrives on its own heels — which is why touching the bar here would be +/// a second, racing teardown rather than a fix. +pub(super) fn pet_broken(errors: &mut UiErrorKeys) { + debug!("net: pet ran away"); + errors.0.push(UiError::key("ERR_PET_BROKEN")); +} + +/// `SMSG_PET_ACTION_SOUND` — the pet's voice: resolve the guid, hand the selector to the sound +/// layer (decision 2039). +/// +/// The reference resolves the packet's guid through the object manager and **drops the packet on +/// a miss** (`0x604101 je`), which is what the index lookup below is: a bark for a unit that has +/// not streamed in has nothing to play from and no position to play at. +/// +/// The selector is not validated here — [`crate::sound::creature`]'s reader is where the +/// two-armed `cmp` lives, because that is where the reference has it too (`0x604106`/`0x60411c`). +pub(super) fn pet_action_sound( + pet_guid: u64, + talk: u32, + index: &GuidIndex, + talks: &mut MessageWriter, +) { + debug!("net: pet talk {talk} from {pet_guid:#x}"); + if let Some(&unit) = index.0.get(&pet_guid) { + talks.write(PetTalkMessage { unit, talk }); + } +} + +/// `SMSG_PET_DISMISS_SOUND` — the parting sound's model id and point, converted into Bevy space +/// for the sound layer (decision 2039). +/// +/// The `+1.0` on `z` is the reference's own (`0x6041d0 fadd [0x7ff9d8]`), applied **before** the +/// basis change because it is a WoW-space offset: the point on the wire is where the pet stood, +/// and the kit sounds a yard above it. +pub(super) fn pet_dismiss_sound( + model_id: u32, + position: [f32; 3], + sounds: &mut MessageWriter, +) { + debug!("net: pet dismissed — model {model_id} at {position:?}"); + sounds.write(PetDismissSoundMessage { + model_id, + pos: wow_to_bevy([position[0], position[1], position[2] + 1.0]), + }); +} + +/// `SMSG_PET_CAST_FAILED` — the pet's cast refusal, queued onto the SAME red line as our own +/// through [`CastErrors`], but **marked as the pet's** so the display picks the reference's pet +/// message table rather than the player's (`push_pet`, decision 2033). +/// +/// The reference handles this opcode in its own function, `Spell_C::HandlePetCastFailed` +/// (`0x6e8eb0`), which is not the player's `0x6e1a00` with a flag — it is a separate switch over +/// a separate 142-byte index table. Ten reasons resolve differently there, six of them to the +/// `ERR_PET_SPELL_*` rows that exist for no other purpose ("Your pet is in combat." where the +/// player reads "You are in combat"). The queue is shared; the table is not. +/// +/// Two things it deliberately does NOT do, both because the caster is the pet: +/// +/// - **Touch our cast state.** There is no pending cast of ours to revert, no GCD of ours to +/// clear and no button of ours to unflash — the three things `cast_result`'s own failure path +/// does. Reusing that path would have made a pet's refused Growl cancel the player's cast bar. +/// - **Write a combat-log line.** `0x6e1a00` calls the log formatter `0x62c360` beside its +/// `DisplayError`; `0x6e8eb0` calls neither it nor the error-sound `0x458a50` — its whole call +/// set is the two packet readers, `0x496720`, and the string plumbing behind it. So a pet's +/// refusal never prints "You fail to cast Growl: ..." in the log, and the drain's combat twin +/// skips it. pub(super) fn pet_cast_failed(spell_id: u32, reason: Option, errors: &mut CastErrors) { debug!("net: pet cast failed — spell {spell_id} reason {reason:?}"); if let Some(reason) = reason { - errors.push_local(spell_id, reason); + errors.push_pet(spell_id, reason); } } @@ -237,17 +362,47 @@ mod tests { assert!(!bar.attacking, "a different pet is not attacking"); } - /// Only the two codes vmangos actually sends resolve; anything else draws nothing rather than - /// putting a bare number on the red line. + /// The whole feedback map, in the reference's own order (`0x4bdbe8`) — and the bound that + /// makes `0` and everything from `5` up draw nothing rather than putting a bare number on the + /// red line. + /// + /// Written as the four keys in sequence rather than four asserts because the defect this + /// replaces was an *ordering* one: a two-entry map that paired the right vocabulary with the + /// wrong bytes still looked plausible read one row at a time. #[test] - fn only_the_shipped_feedback_codes_resolve() { + fn the_feedback_map_is_the_jump_table_at_0x4bdbe8() { let mut errors = UiErrorKeys::default(); - for reason in [0u8, 1, 2, 3, 200] { + for reason in [0u8, 1, 2, 3, 4, 5, 200] { pet_action_feedback(reason, &mut errors); } assert_eq!( errors.0.iter().map(|e| e.key).collect::>(), - ["PET_SPELL_NOPATH", "SPELL_FAILED_OUT_OF_RANGE"] + [ + "ERR_PET_SPELL_DEAD", + "ERR_NO_ATTACK_TARGET", + "ERR_INVALID_ATTACK_TARGET", + "ERR_PET_SPELL_NOPATH", + ] ); } + + /// Every arm names a real `DisplayError` row — which is the check the old map could not have + /// passed, and was never asked to: neither `PET_SPELL_NOPATH` nor `SPELL_FAILED_OUT_OF_RANGE` + /// is a catalog key, and `message_keys`' source walk only reads `ERR_*` literals, so both + /// slipped past it. Naming the rows is also what gives these lines their surface and their + /// error-speech: `ERR_NO_ATTACK_TARGET` and `ERR_INVALID_ATTACK_TARGET` carry type tags + /// `0x26`/`0x0b`, so the client speaks them. + #[test] + fn every_feedback_arm_is_a_catalog_row() { + for reason in 1..=4u8 { + let key = pet_feedback_key(reason).expect("an arm"); + let row = benilla_ui::messages::by_key(key) + .unwrap_or_else(|| panic!("{key} is not a message-catalog row")); + assert_eq!( + row.kind, + benilla_ui::messages::MsgKind::Error, + "{key} is the red line" + ); + } + } } diff --git a/crates/benilla-app/src/net/apply/spells.rs b/crates/benilla-app/src/net/apply/spells.rs index 8880c088..144c1643 100644 --- a/crates/benilla-app/src/net/apply/spells.rs +++ b/crates/benilla-app/src/net/apply/spells.rs @@ -184,6 +184,8 @@ pub(super) fn cast_result( spell_id, reason, arg, + // `SMSG_CAST_FAILED` is addressed to the caster, and this handler is ours. + caster: crate::ui_action::Caster::Player, }); } } diff --git a/crates/benilla-app/src/portrait/light.rs b/crates/benilla-app/src/portrait/light.rs index fea6eb35..bd242606 100644 --- a/crates/benilla-app/src/portrait/light.rs +++ b/crates/benilla-app/src/portrait/light.rs @@ -182,8 +182,11 @@ pub(crate) fn material_variant( /// `CharacterModelBase` (`0x3f8` B, the source-path string at `0x84351c`) and runs the ctor /// `0x505680`. `DressUpModel`/`TabardModel` are subclasses of the same base — same light. /// - That ctor configures the widget's embedded `CGLight` at `+0x324`: **directional** -/// (`0x71b620(light, 0)` @`0x5056c9`), **to-light direction `(0, 1, 0)`** (`0x71b6a0` @`0x505761` -/// — it overwrites an earlier `(0, −0.7071, −0.7071)` set at `0x5056e9`, dead code), **diffuse +/// (`0x71b620(light, 0)` @`0x5056c9`), **direction `(0, 1, 0)` — the direction the light +/// PROPAGATES** (`0x71b6a0` @`0x505761` writes `CGLight+0x24`, which `0x71bce0` negates — three +/// `fchs` at `71be7c`/`71be81`/`71be86` — before building the SH basis, wow-re +/// `modelframe-facing-cancels.md` Q4; 0638 read the field as a to-light vector, 2034 corrects +/// it — it overwrites an earlier `(0, −0.7071, −0.7071)` set at `0x5056e9`, dead code), **diffuse /// `(0.8, 0.8, 0.64)`** (`+0x3c`, @`0x505702`), **ambient `(0.7, 0.7, 0.7)`** (`+0x30`, /// @`0x50572e`), then **enables** it (`0x71b780(light, 1)` @`0x50576a`). /// - It is the widget's ONLY light: the per-frame fill callback `0x76d680` (registered on the @@ -196,17 +199,21 @@ pub(crate) fn material_variant( /// `glue-model-lighting.md §4`), so row 19's dial stays at the studio's `0.4` = intensity 1.0. /// /// **Why this reads so much darker than the studio light, and why that is the point.** The -/// reference's light is a pure SIDE light: WoW `(0, 1, 0)` is the model's own left, which -/// [`wow_to_bevy`] maps to Bevy `(−1, 0, 0)` — square across the camera axis, since the body rig -/// puts the eye on `−Z` looking at the model's front ([`framing::body_frame`]). Everything the pane -/// actually shows the viewer is therefore lit by **ambient alone** (`0.7`), with the diffuse only -/// grazing the figure's screen-right side. The studio light aims its diffuse from the camera's own +/// reference's light is a pure SIDE light: it travels toward WoW `(0, 1, 0)`, the model's own left +/// — Bevy `(−1, 0, 0)` under [`wow_to_bevy`] — so it falls on the figure from its right, square +/// across the camera axis, since the body rig puts the eye on `−Z` looking at the model's front +/// ([`framing::body_frame`]). Everything the pane actually shows the viewer is therefore lit by +/// **ambient alone** (`0.7`), with the diffuse only grazing the figure's screen-left side (its own +/// right; 0638 had it grazing the screen-right side, the same grazing light from the opposite +/// side, which is why the A/B that approved 0638's look could not tell the two apart — 2034). The +/// studio light aims its diffuse from the camera's own /// side, so the same surfaces got `0.58 + 0.85·(N·L≈1) ≈ 1.4` — roughly twice the reference, which /// is exactly the "too brightly lit" the director reported. pub(super) fn model_pane_light() -> LightBlob { // The builder takes the light's PROPAGATION direction (it negates it back into a to-light - // vector for the SH fold), so the ctor's to-light `(0,1,0)` goes in negated. - let sun_dir = -benilla_assets::coords::wow_to_bevy([0.0, 1.0, 0.0]); + // vector for the SH fold) — and the ctor's `(0,1,0)` IS the propagation direction + // (`CGLight+0x24`, negated by `0x71bce0` before the SH basis), so it goes in as written. + let sun_dir = benilla_assets::coords::wow_to_bevy([0.0, 1.0, 0.0]); LightBlob::model( [0.7, 0.7, 0.7], // CharacterModelBase ctor, CGLight+0x30 [0.8, 0.8, 0.64], // CharacterModelBase ctor, CGLight+0x3c @@ -250,8 +257,9 @@ mod tests { /// The reference `` light, pinned at the values the 1.12 `CharacterModelBase` /// constructor writes (decision 0638) — and, more importantly, pinned at the *consequence*: /// its light is square across the body rig's view axis, so what the pane shows the viewer is - /// ambient-lit only. A flipped sign or a swapped axis here is exactly the bug this replaced - /// (a frontal studio key, ~2× the reference on every surface the director can see). + /// ambient-lit only. A swapped axis here is the bug 0638 replaced (a frontal studio key, ~2× + /// the reference on every surface the director can see); a flipped sign is 0638's own reading + /// of `CGLight+0x24` as a to-light vector, which 2034 corrected to the propagation direction. #[test] fn model_pane_light_is_the_reference_widget_light_across_the_view_axis() { let blob = model_pane_light(); @@ -259,12 +267,18 @@ mod tests { assert_eq!(rows[0][..3], [0.7, 0.7, 0.7], "ambient (CGLight+0x30)"); assert_eq!(rows[1][..3], [0.8, 0.8, 0.64], "diffuse (CGLight+0x3c)"); - // Row 2 is the light's PROPAGATION direction; the ctor's to-light is WoW (0,1,0). + // Row 2 is the light's PROPAGATION direction, and the ctor's (0,1,0) IS that direction + // (`0x71bce0` negates `CGLight+0x24` before the SH basis — 2034); the to-light is its + // negation. let sun = Vec3::from_slice(&rows[2][..3]); + assert!( + sun.abs_diff_eq(benilla_assets::coords::wow_to_bevy([0.0, 1.0, 0.0]), 1e-6), + "propagation = wow_to_bevy(0,1,0): {sun:?}" + ); let to_light = -sun; assert!( - to_light.abs_diff_eq(Vec3::new(-1.0, 0.0, 0.0), 1e-6), - "to-light = wow_to_bevy(0,1,0): {to_light:?}" + to_light.abs_diff_eq(Vec3::new(1.0, 0.0, 0.0), 1e-6), + "to-light = -wow_to_bevy(0,1,0): {to_light:?}" ); // The body rig puts the eye on −Z looking at +Z (`framing::body_frame`), so a surface facing // the camera has normal −Z: N·L = 0, i.e. NO diffuse on anything the pane shows front-on. @@ -273,12 +287,10 @@ mod tests { facing_camera.dot(to_light).abs() < 1e-6, "the reference light grazes the pane's visible face, it does not key it" ); - // …while the figure's screen-right side (+X toward the camera's right is −X in world) is - // the one that catches it. - assert!( - Vec3::NEG_X.dot(to_light) > 0.99, - "lit from the viewer's right" - ); + // …while the figure's screen-LEFT side (the camera's right is −X in world, so +X is its + // left) is the one that catches it — the model's own right, the side the light travels + // away from. + assert!(Vec3::X.dot(to_light) > 0.99, "lit from the viewer's left"); } /// The studio light — the round portraits' — is the OPPOSITE arrangement on purpose: its key diff --git a/crates/benilla-app/src/sound/creature.rs b/crates/benilla-app/src/sound/creature.rs index 8abb2ba5..69aa16f0 100644 --- a/crates/benilla-app/src/sound/creature.rs +++ b/crates/benilla-app/src/sound/creature.rs @@ -24,8 +24,8 @@ use benilla_world::schedule::WorldStage; use super::kit::{ bark_chance_pass, object_sound_playing, play_kit, play_kit_ext, source_kit_playing, - stop_source_kit, unit_voice_playing, KitRef, Latch, PlayExtras, SoundCategory, SoundKits, - STAND_CHANCE, STAND_COOLDOWN, + stop_source_kit, stop_unit_voice, unit_voice_state, KitRef, Latch, PlayExtras, SoundCategory, + SoundKits, STAND_CHANCE, STAND_COOLDOWN, }; use super::{AudioListener, SoundConfig, SoundOutput}; @@ -48,6 +48,108 @@ fn load_creature_voices(mut commands: Commands, assets: Option> } } +/// `0x623a40`'s bark states, in the order its jump table `0x623afc` lists them — the number IS +/// the priority (higher wins, `0x623a82`). State `3` is a real arm that plays nothing and has no +/// known caller, so it has no constant. +const BARK_AGGRO: u8 = 0; +const BARK_PET_ORDER: u8 = 1; +const BARK_PET_ATTACK: u8 = 2; +const BARK_DEATH: u8 = 4; + +/// Step 3 of [`play_bark`] — `0x623a66`-`0x623a88`, the priority comparison alone. +/// +/// `latched` is the state of the bark currently sounding on this unit, or `None` for a free slot. +/// The reference's two `je`s skip the comparison entirely when the handle is absent or finished, +/// so a free slot admits **every** state including `0`; a held slot admits only a *strictly* +/// higher one (`jle` aborts on equal, which is what keeps a burst of identical barks to one). +const fn bark_admitted(latched: Option, state: u8) -> bool { + match latched { + None => true, + Some(held) => state > held, + } +} + +/// Step 5 of [`play_bark`] — the jump table at `0x623afc`, as a lookup. `0` means "this state +/// plays nothing", which covers both the table's own silent arm (state 3, `0x623af0`) and its +/// out-of-range default (`0x623aa0 cmp eax,4; ja`), and is also what an unpopulated column reads +/// as. All three are the same observable in the reference and the same `0` here. +const fn bark_kit(voice: &benilla_formats::CreatureVoice, state: u8) -> u32 { + match state { + BARK_AGGRO => voice.aggro, + BARK_PET_ORDER => voice.pet_order, + BARK_PET_ATTACK => voice.pet_attack, + BARK_DEATH => voice.death, + _ => 0, + } +} + +/// **The creature bark dispatcher, whole** — `0x623a40(ecx = unit, state)`, the one function +/// every one-shot creature vocal below goes through (decision 2039). Four states are live here: +/// `0` the HOSTILE aggro flare, `1` the pet's ORDER bark, `2` the pet's ATTACK bark, `4` the +/// death cry. (State `3` exists in the table and plays nothing — it still stops and latches.) +/// +/// Reading it as one function is the point: it was three call sites with three different subsets +/// of its gates before, and the pet barks could not be added correctly to any of them, because +/// what makes a pet bark audible over a pet's near-continuous aggro flares is exactly the +/// priority rule the aggro-only site had no reason to carry. +/// +/// The order of operations is the reference's, and it is load-bearing: +/// +/// 1. **No voice row, no bark** (`0x623a4a`) — the caller's `for_display` miss. +/// 2. **A server-pushed object sound live on this unit mutes it** (`0x623a59` → `0x4591f0`): a +/// scripted voice line is not talked over. Unlike the class route `0x623490`/`0x62f880`, this +/// one is a direct call, not a virtual — so it has **no player exemption**. +/// 3. **The priority latch** (`0x623a66`–`0x623a88`): while `[unit+0xb20]` holds a live handle, a +/// state `<=` the latched one is dropped outright. A free slot skips the comparison entirely. +/// 4. **Stop the old and latch the new** (`0x623a8d`/`0x623a95`) — *before* the column is read. +/// 5. **Then** resolve the column and play. **A zero column still silences step 4's victim** — +/// the handle is overwritten with the null result (`0x623aee`), so a state whose column is +/// unpopulated cuts short whatever it superseded and sounds nothing in its place. That is the +/// client's behaviour, not an oversight of ours. (It is not reachable through the pet barks on +/// a vmangos server: `SendPetTalk` is gated on `SUMMON_PET`, and only those four voices carry +/// the columns — see [`pet_talk_vocals`].) +#[allow(clippy::too_many_arguments)] +fn play_bark( + kits: &mut SoundKits, + assets: &WorldAssets, + out: &mut SoundOutput, + config: &SoundConfig, + listener: Vec3, + unit: Entity, + pos: Vec3, + voice: &benilla_formats::CreatureVoice, + state: u8, +) { + if object_sound_playing(out, unit) { + return; + } + if !bark_admitted(unit_voice_state(out, unit), state) { + return; + } + stop_unit_voice(out, unit); + let kit = bark_kit(voice, state); + if kit == 0 { + return; + } + if let Err(e) = play_kit_ext( + kits, + assets, + out, + config, + listener, + KitRef::Id(kit), + Some(pos), + SoundCategory::Sfx, + PlayExtras { + source: Some(unit), + latch: Latch::Voice(state), + ..default() + }, + ) { + warn!("creature bark state {state} (kit {kit}): {e:#}"); + } +} + /// Play the death vocal on a **live** death: a unit whose store transitions alive→dead. First /// sight already-dead records silently (a streamed corpse doesn't cry — the same distinction the /// animation driver makes for the settled-corpse pose). @@ -80,26 +182,20 @@ fn death_vocals( if !fresh_death { continue; } - let kit = net - .display_id - .and_then(|d| voices.0.for_display(d)) - .map(|v| v.death) - .unwrap_or(0); - if kit == 0 { + let Some(voice) = net.display_id.and_then(|d| voices.0.for_display(d)) else { continue; - } - if let Err(e) = play_kit( + }; + play_bark( &mut kits, &assets, &mut out, &config, listener, - KitRef::Id(kit), - Some(transform.translation), - SoundCategory::Sfx, - ) { - warn!("death vocal (kit {kit}): {e:#}"); - } + entity, + transform.translation, + voice, + BARK_DEATH, + ); } } @@ -179,57 +275,172 @@ fn ai_reaction_vocals( .and_then(|m| m.display_id) .or(net.display_id) }; - let kit = display - .and_then(|d| voices.0.for_display(d)) - .map(|v| if r.hostile { v.aggro } else { v.alert }) - .unwrap_or(0); - if kit == 0 { + let Some(voice) = display.and_then(|d| voices.0.for_display(d)) else { continue; - } - // The `AISOUNDDESC` gate (`0x4591f0`, from `0x6234cb` for ALERT's class route and - // `0x623a59` for HOSTILE's priority route): a server-pushed object sound live on this - // unit suppresses its own vocals, so a scripted voice line is not talked over. Applies to - // classes 0-3 and 8 — ALERT is 8 and HOSTILE takes the priority route, so both are in. - // The CGPlayer twin omits this gate entirely, hence the player exemption. - if net.kind != EntityKind::Player && object_sound_playing(&out, r.unit) { - continue; - } - // The `[unit+0xb20]` gate. Only HOSTILE latches the slot; ALERT neither tests nor stores - // it here (see the doc comment) and stays on the plain play. - if !r.hostile { - if let Err(e) = play_kit( + }; + // HOSTILE is the priority route — `0x623a40(0)`, [`play_bark`], which owns both the + // `AISOUNDDESC` mute and the `[unit+0xb20]` latch. + if r.hostile { + play_bark( &mut kits, &assets, &mut out, &config, listener, - KitRef::Id(kit), - Some(transform.translation), - SoundCategory::Sfx, - ) { - warn!("alert vocal (kit {kit}): {e:#}"); - } + r.unit, + transform.translation, + voice, + BARK_AGGRO, + ); continue; } - if unit_voice_playing(&out, r.unit) { - continue; // category 0 is the lowest: a live bark wins, this one is dropped + // ALERT is the *class* route (`vtable+0x88(8,0)` → `0x623490` → col 13) and is a + // different function: it neither tests nor stores the voice slot, so it stays a plain + // play. It does consult the same `AISOUNDDESC` pool (`0x4591f0` from `0x6234cb`) — but + // there through a virtual whose **CGPlayer twin `0x62f880` omits the gate**, which is why + // the player exemption lives on this leg and not in [`play_bark`]. + if voice.alert == 0 { + continue; } - if let Err(e) = play_kit_ext( + if net.kind != EntityKind::Player && object_sound_playing(&out, r.unit) { + continue; + } + if let Err(e) = play_kit( + &mut kits, + &assets, + &mut out, + &config, + listener, + KitRef::Id(voice.alert), + Some(transform.translation), + SoundCategory::Sfx, + ) { + warn!("alert vocal (kit {}): {e:#}", voice.alert); + } + } +} + +/// The pet's voice (`SMSG_PET_ACTION_SOUND` → the net bridge; decision 2039): the two talk +/// selectors, straight onto [`play_bark`] at states 1 and 2. +/// +/// The reference's handler `0x6040c0` is four instructions of its own once the guid is resolved: +/// selector `0` → `0x623a40(1)`, selector `1` → `0x623a40(2)`, **anything else → nothing** (there +/// is no default arm — `cmp eax,1; jne` falls to the epilogue). So the third value vmangos could +/// invent would be silent here too, and that is the client's answer, not a gap in ours. +/// +/// **This is a warlock-demon surface, and the two ends say so independently.** Only four voices +/// in the shipped file carry these columns — the Imp, Succubus, Doomguard and Voidwalker +/// (`benilla_formats`' `only_the_four_demon_voices_carry_pet_barks`) — and vmangos only *sends* +/// the packet for `GetPetType() == SUMMON_PET`, i.e. those same demons, at a **10% roll** per +/// order (`Unit.cpp:8939`, `PetHandler.cpp:522`, `PetAI.cpp:335`; the other 90% is +/// `SendPetAIReaction`, the growl). A hunter's pet never reaches either half. +#[allow(clippy::too_many_arguments)] +fn pet_talk_vocals( + mut talks: MessageReader, + units: Query<(&NetEntity, &Transform)>, + voices: Option>, + kits: Option>, + assets: Option>, + mut out: NonSendMut, + config: Res, + listener: Res, +) { + if talks.is_empty() { + return; + } + let (Some(voices), Some(mut kits), Some(assets)) = (voices, kits, assets) else { + return; + }; + let listener = listener.pos; + for t in talks.read() { + let state = match t.talk { + benilla_protocol::messages::PET_TALK_ORDER => BARK_PET_ORDER, + benilla_protocol::messages::PET_TALK_ATTACK => BARK_PET_ATTACK, + _ => continue, + }; + let Ok((net, transform)) = units.get(t.unit) else { + continue; + }; + // `0x6040c0` resolves the guid with `0x468460(ecx = 8)` — the **TYPEMASK_UNIT** bit test, + // which a player's cumulative mask also carries (a mind-controlled player is somebody's + // pet too) but a GameObject's does not. + if !matches!(net.kind, EntityKind::Unit | EntityKind::Player) { + continue; + } + // The pet's OWN voice row, not a mount-preferred one: `0x6040c0` resolves the packet's + // guid and hands that object straight to `0x623a40`, which reads `[unit+0xb40]` — the + // base row (`mount-composition.md` Q4's `+0xb44` redirect belongs to `0x60c480`, which is + // not on this path). + let Some(voice) = net.display_id.and_then(|d| voices.0.for_display(d)) else { + continue; + }; + play_bark( + &mut kits, + &assets, + &mut out, + &config, + listener, + t.unit, + transform.translation, + voice, + state, + ); + } +} + +/// A dismissed pet's parting sound (`SMSG_PET_DISMISS_SOUND` → the net bridge; decision 2039) — +/// `CreatureSoundData` column 29, at a bare world point. +/// +/// **Not a bark, and that is the whole shape of it.** `0x604140` never touches `0x623a40`: it +/// resolves the row *fresh by model id* (`CreatureModelData[id].SoundID` → `CreatureSoundData`, +/// no display step and no fallback), reads `[row+0x74]`, and plays it through the plain kit call +/// at the packet's own point — no unit, no voice latch, no `AISOUNDDESC` mute, no attach point. +/// It cannot be otherwise: there is no unit left to hang any of those on. +/// +/// That fresh, inlined resolve is also why column 29 was believed dead — a census over the +/// consumers of the *cached* `[unit+0xb40]` row can never reach it (wow-re +/// `object-layer/scratch/pet-feedback-opcodes.md`). +/// +/// **vmangos never sends this packet**, so nothing here fires against our server today. It is +/// built because the reference is the spec, and because it is what makes loading column 29 a +/// mechanism rather than a guess. +#[allow(clippy::too_many_arguments)] +fn pet_dismiss_sounds( + mut dismissals: MessageReader, + voices: Option>, + kits: Option>, + assets: Option>, + mut out: NonSendMut, + config: Res, + listener: Res, +) { + if dismissals.is_empty() { + return; + } + let (Some(voices), Some(mut kits), Some(assets)) = (voices, kits, assets) else { + return; + }; + let listener = listener.pos; + for d in dismissals.read() { + let kit = voices + .0 + .for_model(d.model_id) + .map(|v| v.pet_dismiss) + .unwrap_or(0); + if kit == 0 { + continue; + } + if let Err(e) = play_kit( &mut kits, &assets, &mut out, &config, listener, KitRef::Id(kit), - Some(transform.translation), + Some(d.pos), SoundCategory::Sfx, - PlayExtras { - source: Some(r.unit), - latch: Latch::Voice, - ..default() - }, ) { - warn!("aggro vocal (kit {kit}): {e:#}"); + warn!("pet dismiss sound (kit {kit}): {e:#}"); } } } @@ -499,6 +710,8 @@ pub(super) fn plugin(app: &mut App) { death_vocals, creature_anim_vocals, ai_reaction_vocals, + pet_talk_vocals, + pet_dismiss_sounds, fall_landing_vocals, creature_body_loops, ) @@ -546,7 +759,7 @@ mod tests { if slot.is_some_and(|(src, l, _)| occupies_voice_slot(src, l, bear)) { continue; // category 0 is the lowest — a live bark wins, this one is dropped } - slot = Some((Some(bear), Latch::Voice, t + clip)); + slot = Some((Some(bear), Latch::Voice(BARK_AGGRO), t + clip)); sounded.push(t); } sounded @@ -585,8 +798,78 @@ mod tests { Entity::from_raw_u32(1).expect("valid entity id"), Entity::from_raw_u32(2).expect("valid entity id"), ); - let barking = (Some(a), Latch::Voice); + let barking = (Some(a), Latch::Voice(BARK_AGGRO)); assert!(occupies_voice_slot(barking.0, barking.1, a)); assert!(!occupies_voice_slot(barking.0, barking.1, b)); } + + /// **The priority rule, at every boundary the reference's two jumps and one `jle` draw** + /// (decision 2039). The equal case is the one that matters most: it is what collapses a burst + /// of identical barks to one, and it is why the aggro flood of 1399 stays collapsed now that + /// the slot carries a state instead of a bare bool. + #[test] + fn a_held_bark_admits_only_a_strictly_higher_state() { + // A free slot admits everything, including the lowest state. + for state in [BARK_AGGRO, BARK_PET_ORDER, BARK_PET_ATTACK, BARK_DEATH] { + assert!(bark_admitted(None, state), "free slot, state {state}"); + } + // Equal is refused — `jle`, not `jl`. + for state in [BARK_AGGRO, BARK_PET_ORDER, BARK_PET_ATTACK, BARK_DEATH] { + assert!(!bark_admitted(Some(state), state), "equal, state {state}"); + } + // A pet's ATTACK bark cuts through its own aggro flare and its own order bark — the whole + // reason the state had to become a payload: a hunter's pet draws aggro flares in bursts + // (1399), and under a bare liveness gate the pet's own voice would never be heard. + assert!(bark_admitted(Some(BARK_AGGRO), BARK_PET_ORDER)); + assert!(bark_admitted(Some(BARK_AGGRO), BARK_PET_ATTACK)); + assert!(bark_admitted(Some(BARK_PET_ORDER), BARK_PET_ATTACK)); + // ...and nothing cuts through the death cry, which is the table's maximum. + assert!(bark_admitted(Some(BARK_PET_ATTACK), BARK_DEATH)); + for state in [BARK_AGGRO, BARK_PET_ORDER, BARK_PET_ATTACK] { + assert!( + !bark_admitted(Some(BARK_DEATH), state), + "over death: {state}" + ); + } + // The order bark does NOT interrupt an attack bark — the two pet states are ordered, and + // the wire sends them close together. + assert!(!bark_admitted(Some(BARK_PET_ATTACK), BARK_PET_ORDER)); + } + + /// The five-arm jump table, column for column — and its two silences. + #[test] + fn the_bark_table_maps_each_state_to_its_column() { + let voice = benilla_formats::CreatureVoice { + exertion: [0; 2], + injury: [0; 3], + death: 314, + stun: 0, + stand: 0, + footstep_class: 0, + aggro: 694, + wing_flap: 0, + wing_glide: 0, + alert: 1107, + fidget: [0; 4], + custom_attack: [0; 4], + loop_sound: 0, + impact_type: 0, + jump_start: 0, + jump_end: 0, + pet_attack: 9097, + pet_order: 9098, + pet_dismiss: 9096, + }; + assert_eq!(bark_kit(&voice, BARK_AGGRO), 694); + assert_eq!(bark_kit(&voice, BARK_PET_ORDER), 9098); + assert_eq!(bark_kit(&voice, BARK_PET_ATTACK), 9097); + assert_eq!(bark_kit(&voice, BARK_DEATH), 314); + // State 3 is a real arm of the table that reads no column, and everything past 4 is the + // range check's default. Both play nothing — and neither is `alert` (col 13) nor + // `pet_dismiss` (col 29): those two reach their kits through entirely different + // functions, off this table altogether. + assert_eq!(bark_kit(&voice, 3), 0); + assert_eq!(bark_kit(&voice, 5), 0); + assert_eq!(bark_kit(&voice, 255), 0); + } } diff --git a/crates/benilla-app/src/sound/kit.rs b/crates/benilla-app/src/sound/kit.rs index 566d341f..31497079 100644 --- a/crates/benilla-app/src/sound/kit.rs +++ b/crates/benilla-app/src/sound/kit.rs @@ -160,18 +160,27 @@ pub(super) enum Latch { Greeting, /// The one-shot creature bark's `[unit+0xb20]` — the per-unit slot of the reference's bark /// dispatch `0x623a40` (5-way jump table `0x623afc`; wow-re - /// `object-layer/scratch/smsg-ai-reaction.md`). + /// `object-layer/scratch/smsg-ai-reaction.md`, `feign-death-dyndead.md` §11). /// - /// The reference keeps the latched **category** beside the handle in `[unit+0xb24]` and uses - /// it for an interrupt rule — only a *strictly higher* category stops what is playing - /// (`0x623a8d`, after `0x623a95 call 0x7a5700`). benilla does not carry the category, because - /// the ordering of that 5-way table is an open wow-re question and guessing it would invent - /// an interrupt the client may not have. Today only the HOSTILE aggro bark (`0x623a40(0)`, - /// `CreatureSoundData` col 10) is routed through the slot, and it is byte-verified as the - /// table's **lowest** category: it never interrupts a playing bark and is dropped outright - /// while this latch is held — which is exactly what plain liveness already gives. When the - /// ordering is pinned, the category becomes a payload here. - Voice, + /// **The payload is the reference's `[unit+0xb24]` — the latched bark STATE**, which is the + /// interrupt rule's whole input: `0x623a82`/`0x623a88` abort a new bark whose state is `<=` + /// the one already sounding, and let a strictly higher one through (stopping the old at + /// `0x623a95`). It was left off until decision 2039 because the table's ordering was an open + /// question and inventing an interrupt is worse than missing one; the five arms are now read + /// out of the image dword for dword: + /// + /// | state | `CreatureSoundData` column | what | + /// |---|---|---| + /// | 0 | 10 | the HOSTILE aggro bark (`SMSG_AI_REACTION`) | + /// | 1 | 28 | the pet's ORDER bark (`SMSG_PET_ACTION_SOUND` selector 0) | + /// | 2 | 27 | the pet's ATTACK bark (`SMSG_PET_ACTION_SOUND` selector 1) | + /// | 3 | — | plays nothing, and *still* stops and latches | + /// | 4 | 6 | the death bark — the maximum, so nothing supersedes it | + /// + /// Carrying it on the channel is equivalent to carrying it on the unit, because the reference + /// only ever consults `[0xb24]` while `[0xb20]` holds a live handle (`0x623a74`/`0x623a7d` + /// jump past the comparison otherwise). + Voice(u8), /// A **server-pushed object sound** live on this unit — `SMSG_PLAY_OBJECT_SOUND` /// (opcode `0x278`), the `AISOUNDDESC` pool at `[0xb05f38]`. /// @@ -786,18 +795,34 @@ pub(super) fn play_kit_ext( } /// Does `unit` hold a **live one-shot voice channel** — the reference's `[unit+0xb20]` handle, -/// nonzero-gated (wow-re `object-layer/scratch/smsg-ai-reaction.md`)? The slot's liveness IS the -/// gate: the HOSTILE aggro bark is the lowest category in `0x623a40`'s table, so it never -/// interrupts what is sounding and is simply dropped while this is true. +/// nonzero-gated (wow-re `object-layer/scratch/smsg-ai-reaction.md`) — and if so, at which +/// **state**? `Some(state)` is the pair `[0xb20]` live + `[0xb24]`; `None` is a free slot, which +/// is `0x623a74`/`0x623a7d`'s "allowed, no comparison" path. /// -/// Scoped to channels that actually latched a category, which is the whole point — the reference +/// Scoped to channels that actually latched a state, which is the whole point — the reference /// keeps this handle separate from the combat drone (`0x623800` carries its own latch) and from /// the greeting line (`[unit+0xb1c]`), so a humming elemental or a talking quest-giver must not /// mute its own barks. -pub(super) fn unit_voice_playing(out: &SoundOutput, unit: Entity) -> bool { - out.channels - .iter() - .any(|c| occupies_voice_slot(c.source, c.latch, unit)) +pub(super) fn unit_voice_state(out: &SoundOutput, unit: Entity) -> Option { + out.channels.iter().find_map(|c| match c.latch { + Latch::Voice(state) if c.source == Some(unit) => Some(state), + _ => None, + }) +} + +/// Stop whatever holds `unit`'s voice slot — the reference's `0x623a95 call 0x7a5700` on +/// `&[unit+0xb20]`, which runs on **every** admitted bark, before the column is even read. So a +/// bark whose column is `0` still silences the one it superseded: the handle is overwritten with +/// the (null) result of playing nothing (`0x623aee`). +pub(super) fn stop_unit_voice(out: &mut SoundOutput, unit: Entity) { + out.channels.retain_mut(|c| { + if occupies_voice_slot(c.source, c.latch, unit) { + c.handle.stop(mixer::declick()); + false + } else { + true + } + }); } /// Does one channel, described by its `(source, voice)` identity, occupy `unit`'s **voice** slot @@ -806,7 +831,7 @@ pub(super) fn unit_voice_playing(out: &SoundOutput, unit: Entity) -> bool { /// them: a bark and a greeting line are different handles in the reference and must not mute each /// other. pub(super) fn occupies_voice_slot(source: Option, latch: Latch, unit: Entity) -> bool { - source == Some(unit) && latch == Latch::Voice + source == Some(unit) && matches!(latch, Latch::Voice(_)) } /// The complement — the greeting latch's own test (see [`source_playing`] for what else currently @@ -1377,7 +1402,7 @@ mod tests { fn the_voice_slot_and_the_greeting_latch_are_disjoint() { let bear = Entity::from_raw_u32(1).expect("valid entity id"); let other = Entity::from_raw_u32(2).expect("valid entity id"); - let bark = (Some(bear), Latch::Voice); + let bark = (Some(bear), Latch::Voice(0)); let greet = (Some(bear), Latch::Greeting); // The case decision 1399 was actually about: a channel tagged with the unit for // *ownership* — its body loop, a missile's travel loop, a water splash — takes no latch @@ -1498,7 +1523,7 @@ mod tests { "a greeting line holds [unit+0xb1c] — stealing it lets the unit re-greet at once" ); assert!( - !stealable(false, Latch::Voice), + !stealable(false, Latch::Voice(0)), "a bark holds [unit+0xb20] — stealing it lets the unit re-bark at once" ); assert!( diff --git a/crates/benilla-app/src/ui_action/cast_fail.rs b/crates/benilla-app/src/ui_action/cast_fail.rs index 74100dbe..6fa7b936 100644 --- a/crates/benilla-app/src/ui_action/cast_fail.rs +++ b/crates/benilla-app/src/ui_action/cast_fail.rs @@ -10,6 +10,14 @@ //! screen shows "Spell is not ready yet." while `SPELL_FAILED_NOT_READY` reads //! "Not yet recovered", and "Not enough rage" for a rage spell's NO_POWER. //! +//! **This layer — and only this layer — is per-CASTER** (decision 2033). `SMSG_PET_CAST_FAILED` +//! is not the same handler with a flag: it is `Spell_C::HandlePetCastFailed 0x6e8eb0`, a +//! separate switch over a separate 142-byte index table, and it overrides a different ten +//! reasons. Six of them are the `ERR_PET_SPELL_*` catalog rows, which have no other raise site +//! in the client — "Your pet is out of range." where the player reads "Out of range." — +//! and seven of the player's overrides are simply absent there. [`Caster`] picks the table; +//! layers 1 and 3 are shared code in the binary and shared code here. +//! //! 3. A per-reason **argument arm** (the second dispatch `0x6e1d8e`, 13 targets) fills that //! message's own `%s`/`%d` from a DBC or item name before it is displayed. The two arms whose //! tables benilla already loads live in [`FailArgs::fill`]; the drain fills three more before @@ -40,6 +48,8 @@ use benilla_formats::{AreaTableCatalog, SpellDisplay, SpellFocusCatalog, SpellMechanicCatalog}; +use super::Caster; + /// Wire reason → its `SPELL_FAILED_*` GlobalStrings key (the `0x6e23e0` table, byte-exact). pub(super) const CAST_FAIL_KEYS: [&str; 146] = [ "SPELL_FAILED_AFFECTING_COMBAT", // 0x00 @@ -315,6 +325,7 @@ impl CastFailLine { /// VM's GlobalStrings lookup (an absent or empty key resolves to `None`, the data-suppression /// face). Reasons beyond the table print their code — our debug affordance, not a ref string. pub(super) fn cast_fail_text( + caster: Caster, reason: u8, spell: Option<&SpellDisplay>, args: FailArgs<'_>, @@ -322,45 +333,100 @@ pub(super) fn cast_fail_text( ) -> Option { let get_text = |key: &str| get(key).filter(|s| !s.is_empty()); let get_display = |key: &'static str| get_text(key).map(|text| CastFailLine { key, text }); - // The errorId overrides (`0x6e1aab–0x6e1c5f`): the replaced-message reasons. - match reason { - 0x01 => return get_display("ERR_SPELL_FAILED_ALREADY_AT_FULL_HEALTH"), - 0x02 => { - let t = get_display("ERR_SPELL_FAILED_ALREADY_AT_FULL_POWER_S")?; - let power = spell.map_or(0, |d| d.power_type); - let name = get_text(power_keys(power).1).unwrap_or_default(); - return Some(t.fill(&name)); - } - 0x09 => return get_display("ERR_GENERIC_NO_TARGET"), - 0x17 => return None, // DONT_REPORT: control-flow hidden (jumps past DisplayError) - 0x18 => return get_display("ERR_SPELL_FAILED_EQUIPPED_ITEM"), - 0x28 => { - return get_display(if is_potion(spell) { - "ERR_POTION_COOLDOWN" - } else { - "ERR_ITEM_COOLDOWN" - }); - } - 0x3C => { - let key = if is_food(spell) { - "ERR_FOOD_COOLDOWN" - } else if is_potion(spell) { - "ERR_POTION_COOLDOWN" - } else if spell.is_some_and(|d| d.attributes & 0x10 != 0) { - "ERR_ABILITY_COOLDOWN" - } else { - "ERR_SPELL_COOLDOWN" - }; - return get_display(key); - } - 0x41 => return get_display("ERR_SPELL_FAILED_NOTUNSHEATHED"), - 0x4D => { - let power = spell.map_or(0, |d| d.power_type); - return get_display(power_keys(power).0); - } - 0x59 => return get_display("ERR_SPELL_OUT_OF_RANGE"), - 0x8E => return get_display("ERR_PLAY_TIME_EXCEEDED"), - _ => {} + // The errorId overrides — the replaced-message reasons. **Which table is asked is the + // caster's** (decision 2033): the reference does not flag one handler, it ships two, and they + // disagree on ten reasons. Everything past this match — the `SPELL_FAILED_*` vocabulary, the + // argument arms, the strip fallback — is shared, exactly as it is in the binary. + match caster { + // `Spell_C::HandlePetCastFailed 0x6e8eb0`: `cmp reason,0x8d; ja default`, then a 142-byte + // index table (`0x6e93d0`) over 15 jump targets (`0x6e9394`). Six of the ten overrides are + // the `ERR_PET_SPELL_*` catalog rows, which exist for this handler and nothing else — + // "Your pet is dead." where the player reads "You are dead". + Caster::Pet => match reason { + 0x00 => return get_display("ERR_PET_SPELL_AFFECTING_COMBAT"), // 0x14c @0x6e8fab + 0x13 => return get_display("ERR_PET_SPELL_DEAD"), // 0x150 @0x6e9017 + 0x32 => return get_display("ERR_PET_SPELL_NOPATH"), // 0x151 @0x6e9032 + 0x33 => return get_display("ERR_PET_SPELL_NOT_BEHIND"), // 0x14e @0x6e8fe1 + 0x59 => return get_display("ERR_PET_SPELL_OUT_OF_RANGE"), // 0x14d @0x6e8fc6 + 0x5F => return get_display("ERR_PET_SPELL_ROOTED"), // 0x14b @0x6e8f4f + 0x65 => return get_display("ERR_PET_SPELL_TARGETS_DEAD"), // 0x14f @0x6e8ffc + // `0x6e8f2a` computes its id: `((SpellRec+0x18 & 0x10) | 0x300) >> 4`, which is only + // ever `0x31` or `0x30`. The player's four-way (`0x6e1aab`) tests the spell CATEGORY + // first for food and potion; the pet's does not test it at all, because a pet eats + // and drinks nothing. + 0x3C => { + return get_display(if spell.is_some_and(|d| d.attributes & 0x10 != 0) { + "ERR_ABILITY_COOLDOWN" + } else { + "ERR_SPELL_COOLDOWN" + }); + } + // `0x6e8f6a`: health (`SpellRec+0x7c` == `-2`) takes `0x123`, everything else indexes + // `[0x8118f0 + 4*power]`. Those five dwords are byte-identical to the player's table + // at `0x8118dc` (`0x11f 0x120 0x121 0x122 0x168`), so this is the SAME pick, not a + // pet-specific one — which is why [`power_keys`] serves both and a hunter pet's focus + // ability reads "Not enough focus" either way. + 0x4D => { + let power = spell.map_or(0, |d| d.power_type); + return get_display(power_keys(power).0); + } + // What is deliberately absent is as load-bearing as what is here: the pet's index + // table sends `0x01`/`0x02` (already at full health/power), `0x09` (no target), + // `0x18` (equipped item), `0x28` (item cooldown), `0x41` (unsheathed) and `0x8e` + // (play time) to the generic arm, so a pet never says any of those seven lines. + // + // `0x17` DONT_REPORT is the one that looks like a divergence and is not: the player's + // handler hides it by control flow, the pet's routes it to the generic arm — but + // `SPELL_FAILED_DONT_REPORT` has no string in 5875's `GlobalStrings.lua`, so the + // passthrough below draws nothing and the two agree on screen. + // + // `0x56` ONLY_SHAPESHIFT has a real arm here (`0x6e921a`, errorId `0xd6`) that fills + // the template with a `SpellShapeshiftForm.dbc` name and displays it through the bare + // `"%s"` of `ERR_SPELL_FAILED_SHAPESHIFT_FORM_S` — the same net line the player's own + // unmodeled arm would produce, and stripped the same way here for the same reason + // (that DBC is not loaded). Same surface, same silence, no pet-specific work. + _ => {} + }, + // `Spell_C::HandleCastFailed 0x6e1a00` (`0x6e1aab`–`0x6e1c5f`). + Caster::Player => match reason { + 0x01 => return get_display("ERR_SPELL_FAILED_ALREADY_AT_FULL_HEALTH"), + 0x02 => { + let t = get_display("ERR_SPELL_FAILED_ALREADY_AT_FULL_POWER_S")?; + let power = spell.map_or(0, |d| d.power_type); + let name = get_text(power_keys(power).1).unwrap_or_default(); + return Some(t.fill(&name)); + } + 0x09 => return get_display("ERR_GENERIC_NO_TARGET"), + 0x17 => return None, // DONT_REPORT: control-flow hidden (jumps past DisplayError) + 0x18 => return get_display("ERR_SPELL_FAILED_EQUIPPED_ITEM"), + 0x28 => { + return get_display(if is_potion(spell) { + "ERR_POTION_COOLDOWN" + } else { + "ERR_ITEM_COOLDOWN" + }); + } + 0x3C => { + let key = if is_food(spell) { + "ERR_FOOD_COOLDOWN" + } else if is_potion(spell) { + "ERR_POTION_COOLDOWN" + } else if spell.is_some_and(|d| d.attributes & 0x10 != 0) { + "ERR_ABILITY_COOLDOWN" + } else { + "ERR_SPELL_COOLDOWN" + }; + return get_display(key); + } + 0x41 => return get_display("ERR_SPELL_FAILED_NOTUNSHEATHED"), + 0x4D => { + let power = spell.map_or(0, |d| d.power_type); + return get_display(power_keys(power).0); + } + 0x59 => return get_display("ERR_SPELL_OUT_OF_RANGE"), + 0x8E => return get_display("ERR_PLAY_TIME_EXCEEDED"), + _ => {} + }, } // The passthrough layer: errorId 0x2c ("%s") displays GetText(SPELL_FAILED_) as-is. let Some(key) = CAST_FAIL_KEYS.get(usize::from(reason)) else { @@ -420,9 +486,174 @@ mod tests { "ERR_SPELL_FAILED_NOTUNSHEATHED", "You have nothing to attack with.", ), + // The pet's own six, verbatim from the shipped file — and NO `ERR_PET_SPELL_NOPATH`, + // because 5875 ships none, which is a fact these tests rest on. + ("ERR_PET_SPELL_AFFECTING_COMBAT", "Your pet is in combat."), + ("ERR_PET_SPELL_DEAD", "Your pet is dead."), + ( + "ERR_PET_SPELL_NOT_BEHIND", + "Your pet must be behind its target.", + ), + ("ERR_PET_SPELL_OUT_OF_RANGE", "Your pet is out of range."), + ("ERR_PET_SPELL_ROOTED", "Your pet is unable to move."), + ("ERR_PET_SPELL_TARGETS_DEAD", "Your pet\'s target is dead."), + ("SPELL_FAILED_AFFECTING_COMBAT", "You are in combat"), + ("SPELL_FAILED_CASTER_DEAD", "You are dead"), + ("SPELL_FAILED_NOPATH", "No path available"), + ("SPELL_FAILED_NOT_BEHIND", "You must be behind your target"), + ("SPELL_FAILED_ROOTED", "You are unable to move"), + ("SPELL_FAILED_TARGETS_DEAD", "Your target is dead"), + ("SPELL_FAILED_BAD_IMPLICIT_TARGETS", "No target"), + ("SPELL_FAILED_NOT_UNSHEATHED", "You must be unsheathed"), + ("ERR_OUT_OF_FOCUS", "Not enough focus"), ]) } + /// **The six lines that exist for the pet and nobody else.** Each is a row the client raises + /// only from `Spell_C::HandlePetCastFailed 0x6e8eb0`, and each is checked against what the + /// *player* gets for the same wire reason — because "our pet reads the same string we do" is + /// exactly the shape of the defect decision 2033 corrects, and a one-sided assert would not + /// have caught it. + #[test] + fn the_pet_speaks_its_own_six_refusals() { + let m = gs(); + let g = getter(&m); + let say = |caster, reason| { + cast_fail_text(caster, reason, None, FailArgs::default(), &g).map(|l| l.text) + }; + for (reason, pet, player) in [ + (0x00, "Your pet is in combat.", "You are in combat"), + (0x13, "Your pet is dead.", "You are dead"), + ( + 0x33, + "Your pet must be behind its target.", + "You must be behind your target", + ), + (0x59, "Your pet is out of range.", "Out of range."), + ( + 0x5F, + "Your pet is unable to move.", + "You are unable to move", + ), + (0x65, "Your pet\'s target is dead.", "Your target is dead"), + ] { + assert_eq!( + say(Caster::Pet, reason).as_deref(), + Some(pet), + "pet {reason:#04x}" + ); + assert_eq!( + say(Caster::Player, reason).as_deref(), + Some(player), + "player {reason:#04x}" + ); + } + } + + /// **NOPATH is silent for the pet and spoken for the player**, which reads like a bug and is + /// the reference: the pet arm raises errorId `0x151`, whose key `ERR_PET_SPELL_NOPATH` has no + /// string in 5875's `GlobalStrings.lua`, so `DisplayError` shows nothing. The player's `0x32` + /// takes the passthrough and reads "No path available". + #[test] + fn the_pets_nopath_is_silent_because_5875_ships_no_string() { + let m = gs(); + let g = getter(&m); + assert_eq!( + cast_fail_text(Caster::Pet, 0x32, None, FailArgs::default(), &g), + None + ); + assert_eq!( + cast_fail_text(Caster::Player, 0x32, None, FailArgs::default(), &g) + .unwrap() + .text, + "No path available" + ); + } + + /// The pet's `0x3c` computes its id from one bit — `((SpellRec+0x18 & 0x10) | 0x300) >> 4` — + /// so it has only the ability/spell split. The player's tests the spell CATEGORY first and has + /// a food and a potion leg. A pet eats nothing, so a potion-category spell that says "Item is + /// not ready yet." for us says "Spell is not ready yet." for it. + #[test] + fn the_pet_has_no_food_or_potion_cooldown_leg() { + let m = gs(); + let g = getter(&m); + let potion = spell(0, 4, 0); + let ability = spell(0, 0, 0x10); + let text = |caster, d| { + cast_fail_text(caster, 0x3C, Some(d), FailArgs::default(), &g) + .unwrap() + .text + }; + assert_eq!(text(Caster::Player, &potion), "Item is not ready yet."); + assert_eq!(text(Caster::Pet, &potion), "Spell is not ready yet."); + // The one leg they share, so the collapse is a missing branch and not a missing table. + assert_eq!(text(Caster::Player, &ability), "Ability is not ready yet."); + assert_eq!(text(Caster::Pet, &ability), "Ability is not ready yet."); + } + + /// The pet's index table sends seven of the player's overrides to its generic arm, so those + /// lines are the player's alone. Checked on the two with the sharpest tell: `0x09`, where the + /// text changes, and `0x41`, where the **surface** changes too — the player's override is the + /// one yellow cast failure in the game, and the pet's passthrough is red. + #[test] + fn the_pet_does_not_take_the_players_overrides() { + use benilla_ui::messages::{kind_of, MsgKind}; + let m = gs(); + let g = getter(&m); + let line = |caster, reason| { + cast_fail_text(caster, reason, None, FailArgs::default(), &g).expect("a line") + }; + + assert_eq!(line(Caster::Player, 0x09).text, "You have no target."); + assert_eq!(line(Caster::Pet, 0x09).text, "No target"); + + let mine = line(Caster::Player, 0x41); + assert_eq!(mine.key, "ERR_SPELL_FAILED_NOTUNSHEATHED"); + assert_eq!(kind_of(mine.key), MsgKind::Info); + let its = line(Caster::Pet, 0x41); + assert_eq!(its.key, PASSTHROUGH); + assert_eq!(its.text, "You must be unsheathed"); + assert_eq!(kind_of(its.key), MsgKind::Error); + } + + /// `0x4d` NO_POWER is the override the two handlers **agree** on, and that is a byte fact + /// rather than an assumption: the pet's table at `0x8118f0` holds the same five dwords as the + /// player's at `0x8118dc`. So [`power_keys`] serves both, and a hunter pet's focus ability + /// reads the focus line on either path. + #[test] + fn the_pets_no_power_pick_is_the_players() { + let m = gs(); + let g = getter(&m); + let focus = spell(2, 0, 0); + for caster in [Caster::Player, Caster::Pet] { + assert_eq!( + cast_fail_text(caster, 0x4D, Some(&focus), FailArgs::default(), &g) + .unwrap() + .text, + "Not enough focus" + ); + } + } + + /// `0x17` DONT_REPORT looks like a divergence between the two handlers and is not: the + /// player's hides it by control flow, the pet's routes it to the generic arm — but + /// `SPELL_FAILED_DONT_REPORT` has no string in 5875, so both draw nothing. Worth a test + /// because the two mechanisms are different and only the *outcome* is shared, so a future + /// change to either one should have to notice. + #[test] + fn dont_report_is_silent_on_both_paths_for_two_different_reasons() { + let m = gs(); + let g = getter(&m); + assert!(!m.contains_key("SPELL_FAILED_DONT_REPORT")); + for caster in [Caster::Player, Caster::Pet] { + assert_eq!( + cast_fail_text(caster, 0x17, None, FailArgs::default(), &g), + None + ); + } + } + /// **The one cast failure that is not a red line.** Reason `0x41` overrides to /// `ERR_SPELL_FAILED_NOTUNSHEATHED` (id 320), and that record's kind is `1` — the yellow /// `UI_INFO_MESSAGE`, not the red `UI_ERROR_MESSAGE` its seventeen override neighbours take. @@ -441,16 +672,19 @@ mod tests { let m = gs(); let g = getter(&m); - let line = cast_fail_text(0x41, None, FailArgs::default(), &g).expect("a line"); + let line = + cast_fail_text(Caster::Player, 0x41, None, FailArgs::default(), &g).expect("a line"); assert_eq!(line.key, "ERR_SPELL_FAILED_NOTUNSHEATHED"); assert_eq!(kind_of(line.key), MsgKind::Info); // An override that IS red, and a passthrough that falls to errorId 0x2c. - let red = cast_fail_text(0x59, None, FailArgs::default(), &g).expect("a line"); + let red = + cast_fail_text(Caster::Player, 0x59, None, FailArgs::default(), &g).expect("a line"); assert_eq!(red.key, "ERR_SPELL_OUT_OF_RANGE"); assert_eq!(kind_of(red.key), MsgKind::Error); - let through = cast_fail_text(0x43, None, FailArgs::default(), &g).expect("a line"); + let through = + cast_fail_text(Caster::Player, 0x43, None, FailArgs::default(), &g).expect("a line"); assert_eq!(through.key, PASSTHROUGH); assert_eq!(kind_of(through.key), MsgKind::Error); } @@ -487,25 +721,25 @@ mod tests { let m = gs(); let g = getter(&m); assert_eq!( - cast_fail_text(0x43, None, FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x43, None, FailArgs::default(), &g) .unwrap() .text, "Out of ammo" ); assert_eq!( - cast_fail_text(0x59, None, FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x59, None, FailArgs::default(), &g) .unwrap() .text, "Out of range." ); assert_eq!( - cast_fail_text(0x76, None, FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x76, None, FailArgs::default(), &g) .unwrap() .text, "Target too close" ); assert_eq!( - cast_fail_text(0x09, None, FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x09, None, FailArgs::default(), &g) .unwrap() .text, "You have no target." @@ -513,21 +747,27 @@ mod tests { // 0x3c: plain spell → spell cooldown; Attr&0x10 → ability; potion category → potion. let plain = spell(0, 0, 0); assert_eq!( - cast_fail_text(0x3C, Some(&plain), FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x3C, Some(&plain), FailArgs::default(), &g) .unwrap() .text, "Spell is not ready yet." ); let ability = spell(1, 0, 0x10); assert_eq!( - cast_fail_text(0x3C, Some(&ability), FailArgs::default(), &g) - .unwrap() - .text, + cast_fail_text( + Caster::Player, + 0x3C, + Some(&ability), + FailArgs::default(), + &g + ) + .unwrap() + .text, "Ability is not ready yet." ); let potion = spell(0, 4, 0); assert_eq!( - cast_fail_text(0x3C, Some(&potion), FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x3C, Some(&potion), FailArgs::default(), &g) .unwrap() .text, "Item is not ready yet." @@ -542,14 +782,14 @@ mod tests { let g = getter(&m); let rage = spell(1, 0, 0); assert_eq!( - cast_fail_text(0x4D, Some(&rage), FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x4D, Some(&rage), FailArgs::default(), &g) .unwrap() .text, "Not enough rage" ); let mana = spell(0, 0, 0); assert_eq!( - cast_fail_text(0x4D, Some(&mana), FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x4D, Some(&mana), FailArgs::default(), &g) .unwrap() .text, "Not enough mana" @@ -563,16 +803,22 @@ mod tests { fn suppression_hex_fallback_and_template_strip() { let m = gs(); let g = getter(&m); - assert_eq!(cast_fail_text(0x17, None, FailArgs::default(), &g), None); - assert_eq!(cast_fail_text(0x08, None, FailArgs::default(), &g), None); assert_eq!( - cast_fail_text(0x92, None, FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x17, None, FailArgs::default(), &g), + None + ); + assert_eq!( + cast_fail_text(Caster::Player, 0x08, None, FailArgs::default(), &g), + None + ); + assert_eq!( + cast_fail_text(Caster::Player, 0x92, None, FailArgs::default(), &g) .unwrap() .text, "Spell failed (0x92)" ); assert_eq!( - cast_fail_text(0x5C, None, FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x5C, None, FailArgs::default(), &g) .unwrap() .text, "Missing reagent" @@ -596,26 +842,26 @@ mod tests { let g = |key: &str| s.lua().globals().get::(key).ok(); assert_eq!( - cast_fail_text(0x43, None, FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x43, None, FailArgs::default(), &g) .unwrap() .text, "Out of ammo" ); assert_eq!( - cast_fail_text(0x59, None, FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x59, None, FailArgs::default(), &g) .unwrap() .text, "Out of range." ); assert_eq!( - cast_fail_text(0x3C, None, FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x3C, None, FailArgs::default(), &g) .unwrap() .text, "Spell is not ready yet." ); let rage = spell(1, 0, 0); assert_eq!( - cast_fail_text(0x4D, Some(&rage), FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x4D, Some(&rage), FailArgs::default(), &g) .unwrap() .text, "Not enough rage" @@ -624,20 +870,72 @@ mod tests { // player reads IS the GlobalStrings value. A typo'd key here would degrade a real refusal // to a dead-looking button, which is what this test exists to catch. assert_eq!( - cast_fail_text(0x50, None, FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x50, None, FailArgs::default(), &g) .unwrap() .text, "Cannot use while swimming" ); assert_eq!( - cast_fail_text(0x58, None, FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x58, None, FailArgs::default(), &g) .unwrap() .text, "Can only use while swimming" ); // The data-suppression face on the real file: the absent keys show nothing. - assert_eq!(cast_fail_text(0x08, None, FailArgs::default(), &g), None); - assert_eq!(cast_fail_text(0x21, None, FailArgs::default(), &g), None); + assert_eq!( + cast_fail_text(Caster::Player, 0x08, None, FailArgs::default(), &g), + None + ); + assert_eq!( + cast_fail_text(Caster::Player, 0x21, None, FailArgs::default(), &g), + None + ); + + // The pet's table against the player's own shipped file (decision 2033). Both halves + // matter: the six rows resolve to the pet wording, and `ERR_PET_SPELL_NOPATH` resolves to + // NOTHING — the claim the `SILENT_IN_5875` entry rests on, checked here against the real + // `GlobalStrings.lua` rather than against a fixture that could simply have omitted it. + for (reason, pet, player) in [ + (0x00, "Your pet is in combat.", "You are in combat"), + (0x13, "Your pet is dead.", "You are dead"), + ( + 0x33, + "Your pet must be behind its target.", + "You must be behind your target", + ), + (0x59, "Your pet is out of range.", "Out of range."), + ( + 0x5F, + "Your pet is unable to move.", + "You are unable to move", + ), + (0x65, "Your pet\'s target is dead.", "Your target is dead"), + ] { + assert_eq!( + cast_fail_text(Caster::Pet, reason, None, FailArgs::default(), &g) + .map(|l| l.text) + .as_deref(), + Some(pet), + "pet {reason:#04x}" + ); + assert_eq!( + cast_fail_text(Caster::Player, reason, None, FailArgs::default(), &g) + .map(|l| l.text) + .as_deref(), + Some(player), + "player {reason:#04x}" + ); + } + assert_eq!( + cast_fail_text(Caster::Pet, 0x32, None, FailArgs::default(), &g), + None, + "5875 ships no ERR_PET_SPELL_NOPATH, so the reference shows nothing" + ); + assert!( + g("PET_SPELL_NOPATH").is_some(), + "and the near-miss key that made the old map look right IS shipped — which is the \ + whole trap" + ); // B255, end to end on the real data: the argument arms against the real DBCs and the real // GlobalStrings templates. Without the fill these read as the bare stems "Requires" and @@ -658,15 +956,19 @@ mod tests { // the wire. The names are lower-case adjectives in the shipped data, which is what makes // them read as the tail of the sentence. assert_eq!( - cast_fail_text(0x8D, None, args(12), &g).unwrap().text, + cast_fail_text(Caster::Player, 0x8D, None, args(12), &g) + .unwrap() + .text, "Can't do that while stunned" ); assert_eq!( - cast_fail_text(0x8D, None, args(5), &g).unwrap().text, + cast_fail_text(Caster::Player, 0x8D, None, args(5), &g) + .unwrap() + .text, "Can't do that while fleeing" ); // An unknown or absent mechanic strips to the bare stem rather than showing a raw `%s`. - assert!(!cast_fail_text(0x8D, None, args(999), &g) + assert!(!cast_fail_text(Caster::Player, 0x8D, None, args(999), &g) .unwrap() .text .contains('%')); @@ -674,26 +976,34 @@ mod tests { // 0x5e REQUIRES_SPELL_FOCUS: the Crown of the Earth phials' own refusal. Focus 12 is the // Starbreeze Village moonwell — using the Jade Phial at any *other* pool is the report. assert_eq!( - cast_fail_text(0x5E, None, args(12), &g).unwrap().text, + cast_fail_text(Caster::Player, 0x5E, None, args(12), &g) + .unwrap() + .text, "Requires Starbreeze Village Moonwell" ); assert_eq!( - cast_fail_text(0x5E, None, args(1), &g).unwrap().text, + cast_fail_text(Caster::Player, 0x5E, None, args(1), &g) + .unwrap() + .text, "Requires Anvil" ); // 0x5d REQUIRES_AREA: area 1657 is Darnassus. assert_eq!( - cast_fail_text(0x5D, None, args(1657), &g).unwrap().text, + cast_fail_text(Caster::Player, 0x5D, None, args(1657), &g) + .unwrap() + .text, "You need to be in Darnassus" ); // The fallbacks. An id the DBC doesn't name, and a wire word the server never sent, both // decline the arm and fall through to the strip — never a raw `%s` on screen. assert_eq!( - cast_fail_text(0x5E, None, args(999_999), &g).unwrap().text, + cast_fail_text(Caster::Player, 0x5E, None, args(999_999), &g) + .unwrap() + .text, "Requires" ); assert_eq!( - cast_fail_text(0x5D, None, FailArgs::default(), &g) + cast_fail_text(Caster::Player, 0x5D, None, FailArgs::default(), &g) .unwrap() .text, "You need to be in" @@ -707,6 +1017,7 @@ mod tests { }; assert_eq!( cast_fail_text( + Caster::Player, 0x5E, Some(&filling), FailArgs { diff --git a/crates/benilla-app/src/ui_action/errors.rs b/crates/benilla-app/src/ui_action/errors.rs index 823729bb..feef2638 100644 --- a/crates/benilla-app/src/ui_action/errors.rs +++ b/crates/benilla-app/src/ui_action/errors.rs @@ -57,15 +57,43 @@ pub(crate) struct CastFail { pub spell_id: u32, pub reason: u8, pub arg: Option, + /// **Whose** refusal this is — which decides which of the reference's two message tables + /// resolves it. See [`Caster`]. + pub caster: Caster, +} + +/// Who failed to cast — the one input that picks between the reference's **two** cast-failure +/// message tables (decision 2033). +/// +/// It is not a flag on one handler: `SMSG_CAST_FAILED` and `SMSG_PET_CAST_FAILED` land in two +/// separate functions, `0x6e1a00` and `0x6e8eb0`, each with its own reason -> errorId map. Ten of +/// the pet's reasons resolve to something the player's never says, six of them to the +/// `ERR_PET_SPELL_*` catalog rows that exist for no other purpose. Everything else about the two +/// — the first-layer `SPELL_FAILED_*` vocabulary, the argument arms, the strip fallback — is +/// literally the same code, which is why this rides the shared queue instead of forking it. +/// +/// [`super::cast_fail::cast_fail_text`] is the reader; nothing downstream of it asks. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) enum Caster { + /// Ours — `SMSG_CAST_FAILED` and every client-local refusal we raise for ourselves. + #[default] + Player, + /// The pet's or the charm's — `SMSG_PET_CAST_FAILED` only. + Pet, } impl CastFail { /// A **client-local** refusal — no wire argument (see [`Self::arg`]). + /// + /// Always the player's: every local refusal benilla raises is raised on its own cast, and the + /// reference has no client-side path that refuses on the pet's behalf — a pet's refusals are + /// all decided server-side and arrive as `SMSG_PET_CAST_FAILED`. pub(crate) const fn local(spell_id: u32, reason: u8) -> Self { Self { spell_id, reason, arg: None, + caster: Caster::Player, } } } @@ -83,6 +111,21 @@ impl CastErrors { spell_id, reason, arg: Some(arg), + caster: Caster::Player, + }); + } + + /// Queue the **pet's** wire refusal (`SMSG_PET_CAST_FAILED`). Same queue, same drain, same + /// sink — [`Caster::Pet`] is the whole difference, and it is read once, at the table pick. + /// + /// No argument word: `PetCastFailed::AppendBodyTo` writes none, so the reference's own + /// argument arms on this path have nothing to read. + pub(crate) fn push_pet(&mut self, spell_id: u32, reason: u8) { + self.0.push(CastFail { + spell_id, + reason, + arg: None, + caster: Caster::Pet, }); } } @@ -93,6 +136,18 @@ impl CastErrors { #[derive(Resource, Default)] pub(crate) struct MountErrors(pub Vec<(bool, u32)>); +/// Taming refusals queued for the UI error line, as the raw `SMSG_PET_TAME_FAILURE` reason byte +/// (decision 2039) — [`MountErrors`]' shape, and here for the same reason: it is a **wire code +/// family**, resolved by a code table, not a client-local key. +/// +/// It cannot ride [`UiErrorKeys`] because its message is a *nested* lookup, which is exactly what +/// the reference does: `0x6e6a20` resolves the reason's `PETTAME_*` key through the script VM +/// (`0x703bf0`) and then passes that **string** as `DisplayError(0xee)`'s argText, filling +/// `ERR_TAME_FAILED`'s lone `%s`. [`UiError`]'s `fill_s` is text the raise site already has; here +/// the raise site has only a byte, and the VM is only reachable at the drain. +#[derive(Resource, Default)] +pub(crate) struct PetTameFailures(pub Vec); + /// Where a client message is SHOWN is **not a decision this crate makes** — it is the `kind` field /// (`+0x04`) of the reference's message record, and it now arrives from the catalog /// ([`benilla_ui::messages`], decision 1770) instead of being hand-carried to every call site @@ -204,10 +259,21 @@ impl Shown { /// /// A key with no row falls back to [`MsgKind::Error`] and no sound. That cannot arise in the /// reference (a message is an *index*, so an unknown key is not expressible), so it only ever - /// means benilla named a key the client does not have — and - /// `every_error_key_in_the_source_is_a_catalog_row` is what keeps it unreachable. + /// means benilla named a key the client does not have. + /// + /// `every_error_key_in_the_source_is_a_catalog_row` was supposed to keep that unreachable and + /// **could not**: all 465 catalog keys begin `ERR_`, so that walk collects `"ERR_…"` literals + /// — which makes it blind to exactly the mistake it is guarding against, a raise site that + /// named something else. `PET_SPELL_NOPATH` and `SPELL_FAILED_OUT_OF_RANGE` sat on the pet's + /// feedback line for months, took this fallback every time, and no gate could see them + /// (decision 2033). Widening the walk is not possible from the text — `SPELL_FAILED_*` keys + /// are legitimate GlobalStrings lookups elsewhere in this very module — so the tripwire moves + /// to the funnel: this is where a non-row key becomes observable, so it says so. pub(crate) fn keyed(key: &str, text: String) -> Self { let record = benilla_ui::messages::by_key(key); + if record.is_none() { + warn!("message {key:?} is not a catalog row — surface and sound are a guess"); + } Self { record, kind: record.map_or(MsgKind::Error, |r| r.kind), diff --git a/crates/benilla-app/src/ui_action/feed.rs b/crates/benilla-app/src/ui_action/feed.rs index 7a5b145c..109fb9c3 100644 --- a/crates/benilla-app/src/ui_action/feed.rs +++ b/crates/benilla-app/src/ui_action/feed.rs @@ -39,8 +39,8 @@ use crate::net::{NetCommands, ObjectStore, SelfPlayer}; use super::errors::{first_missing_totem, first_short_reagent, mount_result_key}; use super::weapon_icon::{auto_attack_icon, substitutes_weapon_icon}; use super::{ - cast_fail, show_messages, ui_error_text, CastErrors, MessageSink, MountErrors, PlayerActions, - Shown, Spells, UiError, UiErrorKeys, UiErrorTexts, + cast_fail, show_messages, ui_error_text, CastErrors, MessageSink, MountErrors, PetTameFailures, + PlayerActions, Shown, Spells, UiError, UiErrorKeys, UiErrorTexts, }; /// What an ITEM action shows when its icon cannot be resolved — the reference's own hardcoded @@ -106,6 +106,7 @@ pub(super) fn feed_actions( mut actions: ResMut, mut cast_errors: ResMut, mut mount_errors: ResMut, + mut pet_tame_failures: ResMut, mut ui_error_keys: ResMut, mut ui_error_texts: ResMut, spells: Option>, @@ -150,8 +151,12 @@ pub(super) fn feed_actions( .drain(..) .filter_map(|fail| { let crate::ui_action::CastFail { - spell_id, reason, .. + spell_id, + reason, + caster, + .. } = fail; + let pet = caster == crate::ui_action::Caster::Pet; let d = spells.as_ref().and_then(|s| s.catalog.get(spell_id)); let get = |key: &str| script.lua().globals().get::(key).ok(); // 0x19/0x1a/0x1b EQUIPPED_ITEM_CLASS* — the other argument-formatted family whose @@ -177,7 +182,11 @@ pub(super) fn feed_actions( .map(|t| cast_fail::CastFailLine::passthrough(t.replace("%s", &name))); } } - if reason == 0x78 || reason == 0x5c { + // `0x78` TOTEMS / `0x5c` REAGENTS — **the player's arms only**. The pet handler's + // index table (`0x6e93d0`) sends both to its generic arm, so a pet's refusal takes the + // shared passthrough below and never runs the bag walk or the item query (decision + // 2033). + if !pet && (reason == 0x78 || reason == 0x5c) { let d = d?; let failing = if reason == 0x78 { self_store @@ -220,7 +229,12 @@ pub(super) fn feed_actions( // is what `0x6e1a00` holds when it calls the formatter — not the errorId-substituted // message the red line shows. An empty one drops the line rather than printing a // sentence with a hole, the same rule every other family here follows. - if let Some(display) = d { + // **The pet's refusal is not logged.** `0x6e1a00` calls the log formatter `0x62c360` + // beside its `DisplayError`; `Spell_C::HandlePetCastFailed 0x6e8eb0` calls neither it + // nor the error sound — its entire call set is the two packet readers, `0x496720` and + // the string plumbing. Before decision 2033 a pet's refused Growl printed "You fail to + // cast Growl: ..." in the combat log, attributed to the player. + if let (Some(display), false) = (d, pet) { // `0x62aff0`: `Attributes` bit 4 marks an ABILITY, which "performs" rather than // "casts". const ATTR_IS_ABILITY: u32 = 0x10; @@ -251,6 +265,7 @@ pub(super) fn feed_actions( } } let text = cast_fail::cast_fail_text( + caster, reason, d, cast_fail::FailArgs { @@ -265,7 +280,8 @@ pub(super) fn feed_actions( // Logging the reason, its wire argument and the resolved text makes an argument arm // that silently declined (a missing word, an unnamed id) legible from a probe run. debug!( - "ui_action: cast fail — spell {spell_id} reason {reason:#04x} arg {:?} → {:?}", + "ui_action: cast fail — {caster:?} spell {spell_id} reason {reason:#04x} \ + arg {:?} → {:?}", fail.arg, text ); text @@ -301,6 +317,22 @@ pub(super) fn feed_actions( .map(|(key, text)| Shown::keyed(key, text)), ); + // Taming refusals ([`PetTameFailures`]) — the one message whose argText is itself a + // GlobalStrings lookup. The reference resolves the reason's `PETTAME_*` key first + // (`0x6e6a20`'s `0x703bf0` call) and passes the resulting STRING as `DisplayError(0xee)`'s + // argument, so `ERR_TAME_FAILED` ("%s.") renders "Creature is too high level for you to + // tame." — two lookups, in this order, and neither can be folded into the other. + let tame_texts: Vec = pet_tame_failures + .0 + .drain(..) + .filter_map(|reason| { + let reason_key = benilla_protocol::messages::pet_tame_failure_key(reason); + let reason_text = script.lua().globals().get::(reason_key).ok()?; + super::keyed_line_s(&script, "ERR_TAME_FAILED", &[&reason_text]) + }) + .collect(); + show_messages(&mut script, &mut sink, "ui_action", tame_texts); + // Client-local by-key refusals (the `DisplayError` route — [`UiErrorKeys`]); the key IS the // GlobalStrings lookup, no code table between, and the key is also what names the surface: // [`UiError::kind`] reads the message record straight out of the catalog instead of the queue diff --git a/crates/benilla-app/src/ui_action/feed_tests.rs b/crates/benilla-app/src/ui_action/feed_tests.rs index 022f421d..9767afc5 100644 --- a/crates/benilla-app/src/ui_action/feed_tests.rs +++ b/crates/benilla-app/src/ui_action/feed_tests.rs @@ -17,7 +17,7 @@ use benilla_ui::script::UiScript; use bevy::prelude::*; use super::feed::{feed_actions, MISSING_ITEM_ICON}; -use super::{CastErrors, MountErrors, PlayerActions, UiErrorKeys, UiErrorTexts}; +use super::{CastErrors, MountErrors, PetTameFailures, PlayerActions, UiErrorKeys, UiErrorTexts}; use crate::entities::ItemDisplays; use crate::items::{test_template, Items}; use crate::net::{ClientCommand, NetCommands}; @@ -60,6 +60,7 @@ fn app_with_food_on_the_bar() -> (App, crossbeam_channel::Receiver() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::() // The cast-failure combat-log line (1703) rides the same drain. @@ -311,6 +312,7 @@ fn a_macro_slot_shows_the_macros_own_icon_and_follows_an_edit() { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::() // The cast-failure combat-log line (1703) rides the same drain. @@ -475,3 +477,143 @@ fn pre_resolved_lines_land_on_the_errors_frame_in_the_arms_colour() { "red UI_ERROR_MESSAGE for the notice, yellow UI_INFO_MESSAGE for the area trigger" ); } + +/// **A pet's refused cast is not written to the combat log** (decision 2033). +/// +/// `HandleCastFailed 0x6e1a00` calls the log formatter `0x62c360` beside its `DisplayError`; +/// `HandlePetCastFailed 0x6e8eb0` calls neither it nor the error sound — its whole call set is the +/// two packet readers, `0x496720`, and the string plumbing behind it. Until this guard the drain +/// treated both queue entries alike, so a pet's refused Growl printed "You fail to cast Growl: +/// ..." in the log, worded as the player's own failure. +/// +/// Driven through the real system with the two entries **side by side and the same spell**, so the +/// assert cannot pass by the line failing to build for some unrelated reason: one goes in, one +/// comes out. +#[test] +fn a_pets_refused_cast_writes_no_combat_log_line() { + use crate::ui_action::{CastFail, Caster, Spells}; + use benilla_formats::{SpellCatalog, SpellDisplay}; + + const GROWL: u32 = 2649; + + let (tx, _rx) = crossbeam_channel::unbounded(); + let mut app = App::new(); + let mut errors = CastErrors::default(); + // 0x5f ROOTED: an override on BOTH tables, so neither entry can be dropped for want of a + // string, and the red line still gets two lines — it is only the log that differs. + errors.0.push(CastFail { + spell_id: GROWL, + reason: 0x5F, + arg: None, + caster: Caster::Player, + }); + errors.push_pet(GROWL, 0x5F); + + app.insert_resource(PlayerActions::default()) + .insert_resource(errors) + .insert_resource(Spells { + catalog: SpellCatalog::from_displays(HashMap::from([( + GROWL, + SpellDisplay { + name: "Growl".into(), + ..Default::default() + }, + )])), + ..Spells::empty_for_tests() + }) + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .insert_resource(NetCommands(tx)); + let script = UiScript::new().unwrap(); + // Only the two strings the log line needs; the red line's own text is `cast_fail`'s business + // and has its own tests. + script + .run(r#"SPELL_FAILED_ROOTED = "You are unable to move";"#) + .unwrap(); + app.insert_non_send_resource(script); + app.add_systems(Update, feed_actions); + app.update(); + + assert_eq!( + app.world() + .resource::() + .pending_len(), + 1, + "the player's failure logs and the pet's does not" + ); +} + +/// **The taming refusal is TWO GlobalStrings lookups, in order** (decision 2039) — the one +/// message benilla shows whose argText is itself a key. +/// +/// The reference's `0x6e6a20` resolves `PETTAME_` through the script VM first and hands +/// the resulting *string* to `DisplayError(0xee)`, so `ERR_TAME_FAILED` ("%s.") renders the +/// reason's sentence with a period after it. Getting the order wrong is not a compile error and +/// not a silent no-op — it would print the literal key on the red line — so it is pinned here at +/// both ends of the reason table's bounds check. +#[test] +fn a_tame_failure_composes_the_reason_string_into_err_tame_failed() { + let (mut app, _rx) = app_with_food_on_the_bar(); + { + let script = app.world_mut().non_send_resource_mut::(); + // The two strings verbatim from 1.12's `GlobalStrings.lua`, plus a recorder for the + // event the sink fires — this test is about the composition, not the frame. + script + .run( + r#" + ERR_TAME_FAILED = "%s."; + PETTAME_TOOHIGHLEVEL = "Creature is too high level for you to tame"; + PETTAME_UNKNOWNERROR = "Unknown taming error"; + BENILLA_TEST_LINES = {}; + local f = CreateFrame("Frame") + f:RegisterEvent("UI_ERROR_MESSAGE") + f:SetScript("OnEvent", function() + table.insert(BENILLA_TEST_LINES, arg1) + end) + "#, + ) + .unwrap(); + } + let line = |app: &mut App, i: usize| { + app.world_mut() + .non_send_resource::() + .eval::>(&format!("return BENILLA_TEST_LINES[{i}]")) + .unwrap() + }; + + // 9 = PETTAME_TOOHIGHLEVEL, the middle of the jump table; 12 is vmangos's own out-of-range + // twelfth value, which the reference's `reason - 1 > 0xa` bound sends to the default arm. + app.world_mut().resource_mut::().0.push(9); + app.world_mut().resource_mut::().0.push(12); + app.update(); + + assert!( + app.world().resource::().0.is_empty(), + "the feed drains the queue" + ); + assert_eq!( + line(&mut app, 1).as_deref(), + Some("Creature is too high level for you to tame."), + "the reason string fills ERR_TAME_FAILED's %s — not the key, and not the reason alone" + ); + assert_eq!( + line(&mut app, 2).as_deref(), + Some("Unknown taming error."), + "out of range takes the default arm rather than showing nothing" + ); + + // The reference's data-suppression face: a reason whose PETTAME string is not loaded shows + // NOTHING rather than a bare "." — the inner lookup fails, so the whole line is suppressed. + app.world_mut().resource_mut::().0.push(1); + app.update(); + assert_eq!( + line(&mut app, 3), + None, + "an unresolvable reason shows nothing" + ); +} diff --git a/crates/benilla-app/src/ui_action/mod.rs b/crates/benilla-app/src/ui_action/mod.rs index 78f20b07..71a16efd 100644 --- a/crates/benilla-app/src/ui_action/mod.rs +++ b/crates/benilla-app/src/ui_action/mod.rs @@ -67,8 +67,8 @@ pub(crate) use cast_send::{CastCommit, CastLadder}; pub(crate) use cast_target::AutoSelfCast; pub(crate) use errors::{ attack_actor_blocked, attack_actor_refusal, keyed_line, keyed_line_s, reagent_totem_refusal, - show_messages, ui_error_text, CastErrors, CastFail, MessageSink, MountErrors, Shown, UiError, - UiErrorKeys, UiErrorTexts, + show_messages, ui_error_text, CastErrors, CastFail, Caster, MessageSink, MountErrors, + PetTameFailures, Shown, UiError, UiErrorKeys, UiErrorTexts, }; // `pub(crate)`: the requirement validator's mounted block is ONE gate in the reference // (`0x6094f0` @ `0x609c6c`) sitting under the ONE cast entry `TryCast 0x6e4b60` — but benilla still @@ -327,6 +327,7 @@ impl Plugin for UiActionPlugin { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() diff --git a/crates/benilla-app/src/ui_loot.rs b/crates/benilla-app/src/ui_loot.rs index 4bdb8713..ad643a85 100644 --- a/crates/benilla-app/src/ui_loot.rs +++ b/crates/benilla-app/src/ui_loot.rs @@ -6,19 +6,20 @@ //! at its fixed position ([`LootState::remove_slot`] — the layout never compacts while open); //! `SMSG_LOOT_CLEAR_MONEY` → the coin row becomes the same kind of gap //! ([`LootState::clear_money`]); `SMSG_LOOT_RELEASE_RESPONSE` → the window closes -//! ([`LootState::clear`]); the error shape → [`LootErrors`]; `SMSG_ITEM_PUSH_RESULT` → a queued -//! "You receive loot" line ([`LootState::receives`]). A removal that empties the window arms the -//! client-authoritative **auto-close** ([`LootState::auto_release`] — the real engine's -//! close-on-last-slot), released by [`drain_loot`]. +//! ([`LootState::clear`]); the error shape → a red line by GlobalStrings key, raised straight from +//! the bridge ([`crate::net::apply::loot::loot_error`] — it needs no queue on this side); +//! `SMSG_ITEM_PUSH_RESULT` → a queued "You receive loot" line ([`LootState::receives`]). A removal +//! that empties the window arms the client-authoritative **auto-close** +//! ([`LootState::auto_release`] — the real engine's close-on-last-slot), released by +//! [`drain_loot`]. //! -//! Each frame [`feed_loot`] surfaces the errors + receive lines on the red UI error line (the -//! equip-error path's exact shape — an ErrorsFrame-style v1 stopgap that migrates to the chat frame -//! next arc), resolves each wire [`LootItem`] to a Lua-facing [`LootRow`] (icon straight from the -//! wire `display_info_id` through the same `ItemDisplayInfo.dbc` catalog the bags use — no template -//! wait; name + quality via the ask-once item-template cache, `None`/re-fed while in flight), -//! prepends the synthesized coin row when the loot carries gold, pushes the snapshot -//! ([`benilla_ui::script::UiScript::set_loot`]), and fires `LOOT_OPENED` on open / `LOOT_UPDATE` on a -//! content change / `LOOT_CLOSED` on clear. [`drain_loot`] pulls the Lua intents back out: `LootSlot` +//! Each frame [`feed_loot`] surfaces the receive lines, resolves each wire [`LootItem`] to a +//! Lua-facing [`LootRow`] (icon straight from the wire `display_info_id` through the same +//! `ItemDisplayInfo.dbc` catalog the bags use — no template wait; name + quality via the ask-once +//! item-template cache, `None`/re-fed while in flight), prepends the synthesized coin row when the +//! loot carries gold, pushes the snapshot ([`benilla_ui::script::UiScript::set_loot`]), and fires +//! `LOOT_OPENED` on open / `LOOT_UPDATE` on a content change / `LOOT_CLOSED` on clear. +//! [`drain_loot`] pulls the Lua intents back out: `LootSlot` //! → coin ? [`ClientCommand::LootMoney`] : [`ClientCommand::AutostoreLootItem`] (the clicked 1-based //! row mapped to the item's **wire** loot slot); `CloseLoot` → [`ClientCommand::LootRelease`]. @@ -448,11 +449,6 @@ impl LootState { } } -/// A loot refusal (`SMSG_LOOT_RESPONSE`'s error shape) queued by the net bridge for the UI error line -/// — the loot twin of [`crate::ui_merchant::MerchantErrors`]. Carries the wire `u8` `LootError` code. -#[derive(Resource, Default)] -pub(crate) struct LootErrors(pub Vec); - /// The loot player knob (decision 0961): `autoLootDefault` — era's Controls-page checkbox (no /// 1.12 CVar exists; vanilla only had the shift-click), settable from the Options window /// through the CVar store (0954). The reference implements auto-loot ENGINE-side (era's own @@ -598,7 +594,6 @@ pub(crate) struct UiLootPlugin; impl Plugin for UiLootPlugin { fn build(&self, app: &mut App) { app.init_resource::() - .init_resource::() .init_resource::() .init_resource::() .init_resource::() @@ -618,32 +613,6 @@ impl Plugin for UiLootPlugin { } } -/// The client's message string for a `LootError` refusal (`SMSG_LOOT_RESPONSE`'s error shape); values -/// from [`benilla_protocol::messages::loot_error`] (VERIFIED vmangos `LootMgr.h`). Only the -/// subset a plain `CMSG_LOOT` can surface is spelled out; the rest print their code. -fn loot_error_text(reason: u8) -> String { - use benilla_protocol::messages::loot_error as e; - match reason { - e::DIDNT_KILL => "You don't have permission to loot that corpse.".into(), - e::TOO_FAR => "You are too far away to loot that.".into(), - e::BAD_FACING => "You can't loot that from there.".into(), - e::LOCKED => "Someone is already looting that corpse.".into(), - e::NOTSTANDING => "You need to be standing up to loot.".into(), - e::STUNNED => "You can't do that while stunned.".into(), - e::PLAYER_NOT_FOUND => "You can't loot that right now.".into(), - e::ALREADY_PICKPOCKETED => "Those pockets are already empty.".into(), - // The master looter's three refusals (decision 1675). These reach only the master looter, - // in answer to a `CMSG_LOOT_MASTER_GIVE` the server would not honour - // (`LootHandler.cpp:718-729`), and unlike the lines above they are QUOTED from 1.12's own - // GlobalStrings (l.1679-1681) rather than composed — the reference has real strings for - // exactly this trio. - e::MASTER_INV_FULL => "That player's inventory is full".into(), - e::MASTER_UNIQUE_ITEM => "Player has too many of that item already".into(), - e::MASTER_OTHER => "Can't assign item to that player".into(), - other => format!("You can't loot that ({other})."), - } -} - /// Copper → the loot coin-row name text: each nonzero denomination as " ", the words the real /// GlobalStrings coin words (`GOLD`/`SILVER`/`COPPER`), joined by a single space and dropping leading /// zeroes (25 → "25 Copper"; 10025 → "1 Gold 25 Copper"; 0 → "0 Copper"). The coin row's name is @@ -953,7 +922,6 @@ fn feed_loot( mut items: ResMut, icons: Option>, commands: Res, - mut errors: ResMut, mut chat: ResMut, mut last: Local>>, cfg: Res, @@ -976,15 +944,6 @@ fn feed_loot( props: props.as_deref(), enchants: enchants.as_deref(), }; - // Loot refusals + "You receive …" lines migrate to the chat window (decision 0084's chat arc): - // refusals as informational SYSTEM-yellow lines, receive lines as LOOT-green. The ErrorsFrame - // keeps only the cast/equip red toasts. - for reason in errors.0.drain(..) { - chat.push_event(crate::ui_chat::ChatEvent::text_only( - crate::ui_chat::ChatEventKind::System, - loot_error_text(reason), - )); - } drain_receives( &mut loot, &mut items, @@ -1486,7 +1445,6 @@ mod tests { let mut app = App::new(); app.add_message::() .init_resource::() - .init_resource::() .init_resource::() .init_resource::() .init_resource::() @@ -1752,7 +1710,6 @@ mod tests { let mut app = App::new(); app.add_message::() .init_resource::() - .init_resource::() .init_resource::() .init_resource::() .init_resource::() diff --git a/crates/benilla-app/src/ui_script/death_tests.rs b/crates/benilla-app/src/ui_script/death_tests.rs index 5ee046b9..a1ac60d7 100644 --- a/crates/benilla-app/src/ui_script/death_tests.rs +++ b/crates/benilla-app/src/ui_script/death_tests.rs @@ -385,11 +385,17 @@ fn the_ghost_predicates() { ..Default::default() }), ); - assert!(!s.eval::("return UnitIsDead(\"player\")").unwrap()); - assert!(s.eval::("return UnitIsGhost(\"player\")").unwrap()); + // The trio's shape is 1/nil, never a boolean (decision 2043), so these read the value rather + // than its truthiness — `== nil` is the comparison a boolean would invert. assert!(s - .eval::("return UnitIsDeadOrGhost(\"player\")") + .eval::("return UnitIsDead(\"player\") == nil") .unwrap()); + assert_eq!(s.eval::("return UnitIsGhost(\"player\")").unwrap(), 1); + assert_eq!( + s.eval::("return UnitIsDeadOrGhost(\"player\")") + .unwrap(), + 1 + ); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } diff --git a/crates/benilla-app/src/ui_script/questlog_tests.rs b/crates/benilla-app/src/ui_script/questlog_tests.rs index 0125d2db..64acfc2f 100644 --- a/crates/benilla-app/src/ui_script/questlog_tests.rs +++ b/crates/benilla-app/src/ui_script/questlog_tests.rs @@ -1807,3 +1807,112 @@ fn share_quest_click_queues_the_selected_quests_id() { assert!(s.take_quest_log_pushes().is_empty(), "drained"); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } + +/// **The reference's title width cap is live, it bites, and it does not wrap the row.** +/// +/// Stock `QuestLogFrame.lua:196-203` shrinks a tagged row's title to `275 − 15 − tagWidth` so a +/// long name cannot run under the right-flush `(Elite)`. Decision 1873 declined to implement that +/// cap in our own (now deleted) copy of the window, on the reading that an explicit width is a +/// **wrap** width here and would spill the title onto a second line inside the 16-unit row; +/// 1944 then put the reference's own file on the chain and the cap arrived with it, unrevisited. +/// +/// It is correct, and the reading was wrong about our own engine: the title FontString carries a +/// DECLARED height (``), which arms both overflow +/// regimes — the line stack and the height-gated ellipsis — so the paint is one truncated line, +/// which is the reference's own result (wow-re `fontstring-overflow.md`: the ellipsis gate is +/// `boxW > 0 && boxH > 0`, and only an AUTO-height FontString escapes it). The render half is +/// pinned on the real font in `ui_text::layout`'s +/// `a_capped_quest_log_title_ellipsizes_on_one_line`; this is the geometry half — that the row +/// really hands that pass a box narrower than its own text. +#[test] +fn the_stock_title_width_cap_bites_and_is_sticky_per_row_button() { + let _data = benilla_formats::wow_data_or_skip!(); + let mut s = UiScript::new().unwrap(); + s.set_screen_size(1024.0, 768.0); + // The cap is arithmetic over `questTitleTag:GetWidth()`, read inside the update — the app's + // measure seam is synchronous (`extract::seat_text_measurer`), so the harness installs its + // stand-in or every tag measures 0 and the cap comes out at the wrong number. + s.set_text_measurer(Box::new(super::FixedWidthFont(6.0))); + load_xml(&s, "Interface\\FrameXML\\Fonts.xml"); + load_xml(&s, "Interface\\FrameXML\\GlobalStrings.lua"); + load_xml(&s, r"Interface\FrameXML\MoneyFrame.lua"); + load_xml(&s, r"Interface\FrameXML\MoneyFrame.xml"); + load_xml(&s, r"Interface\FrameXML\UIParent.xml"); + load_xml(&s, "Interface\\FrameXML\\GameTooltip.xml"); + load_xml(&s, r"Interface\FrameXML\UIPanelTemplates.lua"); + load_xml(&s, r"Interface\FrameXML\UIPanelTemplates.xml"); + load_xml(&s, r"Interface\FrameXML\BasicControls.xml"); + load_xml(&s, r"Interface\FrameXML\LocaleProperties.lua"); + load_xml(&s, r"Interface\FrameXML\StaticPopup.xml"); + load_xml(&s, "ScrollTemplates.xml"); // our scroll kit + the placeholder icon + load_xml(&s, "Interface\\FrameXML\\CharacterFrameTemplates.xml"); + load_xml(&s, "Interface\\FrameXML\\MerchantFrame.xml"); + load_xml(&s, "Interface\\FrameXML\\BasicControls.xml"); + load_xml(&s, "Interface\\FrameXML\\ItemButtonTemplate.xml"); + load_xml(&s, "Interface\\FrameXML\\QuestFrame.xml"); + load_xml(&s, r"Interface\FrameXML\MainMenuBarMicroButtons.xml"); + load_xml(&s, "Interface\\FrameXML\\QuestLogFrame.xml"); + + // A real 1.11 Dungeon-tagged quest, and the longest title the tag branch has to survive. + const LONG: &str = "The Left Piece of Lord Valthalak's Amulet"; + let mut state = eight_entries(); + state.entries[0].title = LONG.into(); + state.entries[0].tag = Some("Dungeon".into()); + // Row 2 carries the same name with NO tag: the else branch never sets a width at all. + state.entries[1].title = LONG.into(); + state.entries[1].tag = None; + s.set_quest_log(state.clone()); + s.run("ToggleQuestLog()").unwrap(); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); + + let width = + |s: &mut UiScript, q: &str| s.eval::(&format!("return {q}:GetWidth()")).unwrap(); + let ink = |s: &mut UiScript, q: &str| { + s.eval::(&format!("return {q}:GetStringWidth()")) + .unwrap() + }; + + // The cap is the reference's own arithmetic, over the tag string it just painted. + let tag_w = width(&mut s, "QuestLogTitle1Tag"); + let capped = width(&mut s, "QuestLogTitle1NormalText"); + assert!( + (capped - (275.0 - 15.0 - tag_w)).abs() < 0.5, + "the row's title is capped at 275-15-tagWidth ({tag_w} wide): got {capped}" + ); + // And it BITES — the box is narrower than the title's own extent, which is the whole case + // 1873 refused. What the paint does with that is the render half's test. + let natural = ink(&mut s, "QuestLogTitle1NormalText"); + assert!(natural > capped, "the fixture title ({natural} px) must overflow the {capped} px cap or this test proves nothing"); + // The row's title box is one line tall — the declared height that arms both overflow regimes. + // An auto-height FontString would escape the ellipsis gate and stack a second line into a + // 16-unit row, which is exactly the defect that was feared. + assert!( + (s.eval::("return QuestLogTitle1NormalText:GetHeight()") + .unwrap() + - 10.0) + .abs() + < 0.5, + "the ButtonText's declared height (QuestLogFrame.xml:94-96)" + ); + + // An untagged row is never capped: the else branch's reset only fires above 275. + assert!( + (width(&mut s, "QuestLogTitle2NormalText") - ink(&mut s, "QuestLogTitle2NormalText")).abs() + < 0.5, + "an untagged title keeps its natural width" + ); + + // **The cap is sticky per row button, and that is the reference's too**: once the title has an + // explicit width, `questNormalText:GetWidth()` answers it (ours and the client's both), so the + // else branch's `> 275` reset can never fire and a row that has shown a tagged quest keeps the + // narrow box when an untagged one scrolls into it. Reproduced, not a benilla defect — but the + // day our metric reads stop being sticky this row silently changes width, so it is pinned. + state.entries[0].tag = None; + s.set_quest_log(state); + s.run("QuestLog_Update()").unwrap(); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); + assert!( + (width(&mut s, "QuestLogTitle1NormalText") - capped).abs() < 0.5, + "the capped box survives the row going untagged (the ref's own 275 reset never fires)" + ); +} diff --git a/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs b/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs index c13634a9..51ec3f76 100644 --- a/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs +++ b/crates/benilla-app/src/ui_script/tooltip_anchor_tests.rs @@ -157,6 +157,53 @@ fn unit_frame_hover_takes_the_default_corner_and_drops_on_leave() { assert!(s.errors().is_empty(), "errors: {:?}", s.errors()); } +/// **The title a world hover paints while the creature query is still in flight**, over the +/// shipped files: `UNKNOWNOBJECT` as `GlobalStrings.lua` defines it on the PLAYER'S OWN CHAIN. +/// Read out of the VM at the assert rather than written as a literal, because that is the whole +/// point of the resolver's `0x703bf0` read — a translated GlobalStrings translates the +/// placeholder too (decision 2040, closing 2002's residue). +/// +/// The engine half (the miss legs, the empty-global fallback, the `"player"` case) is +/// `benilla_ui`'s own `tooltip_unit` suite; what this adds is the chain: the string really is +/// defined, the plate really reads it, and the answer really replaces it. +#[test] +fn a_pending_name_hover_titles_the_chains_unknownobject() { + let _data = benilla_formats::wow_data_or_skip!(); + let mut s = harness(&["Interface\\FrameXML\\GlobalStrings.lua"]); + // The snapshot the feed pushes before `SMSG_CREATURE_QUERY_RESPONSE` lands: the descriptor is + // in, and the name and the type word — which ride the same record — are not. + s.set_unit( + "mouseover", + Some(UnitState { + name: None, + creature_type_name: None, + ..wolf() + }), + ); + assert!(s.world_tooltip_unit("mouseover"), "the hover shows"); + let global = s.eval::("return UNKNOWNOBJECT").unwrap(); + assert!( + !global.is_empty(), + "the chain's GlobalStrings.lua defines UNKNOWNOBJECT" + ); + assert_eq!( + s.eval::("return GameTooltipTextLeft1:GetText()") + .unwrap(), + global, + "a name in flight titles the plate with the GlobalString, not an empty line" + ); + + // The query answers; the next paint of the same hover carries the real name. + s.set_unit("mouseover", Some(wolf())); + assert!(s.world_tooltip_unit("mouseover")); + assert_eq!( + s.eval::("return GameTooltipTextLeft1:GetText()") + .unwrap(), + "Timber Wolf" + ); + assert!(s.errors().is_empty(), "errors: {:?}", s.errors()); +} + /// The detailed-tooltip fork (ref UnitFrame_OnEnter l.58-67, director-approved 0663): with tips on /// — the 1.12 default — the frame explains its RIGHT-CLICK MENU and returns BEFORE `SetUnit`, so /// the unit lines never render. Your own portrait always; another player's whenever they're your diff --git a/crates/benilla-app/src/ui_script/unit_frame_tests.rs b/crates/benilla-app/src/ui_script/unit_frame_tests.rs index 6930df0c..08b49e1c 100644 --- a/crates/benilla-app/src/ui_script/unit_frame_tests.rs +++ b/crates/benilla-app/src/ui_script/unit_frame_tests.rs @@ -1417,9 +1417,10 @@ fn a_feigning_target_paints_empty_bars_and_the_dead_text() { "the WORD is the GlobalString `DEAD` (l.898), never the key: a literal \"DEAD\" here \ is the caps bug the director caught on Onyxia" ); - assert!( - s.eval::(r#"return UnitIsDead("target")"#).unwrap(), - "UnitIsDead 0x517ac0's dynflag leg reaches the API too" + assert_eq!( + s.eval::(r#"return UnitIsDead("target")"#).unwrap(), + 1, + "UnitIsDead 0x517ac0's dynflag leg reaches the API too — as the number 1 (2043)" ); // He stands back up: the flag clears, and nothing about the body needed restoring. diff --git a/crates/benilla-app/src/ui_text/layout/mod.rs b/crates/benilla-app/src/ui_text/layout/mod.rs index 520f1f39..7a763775 100644 --- a/crates/benilla-app/src/ui_text/layout/mod.rs +++ b/crates/benilla-app/src/ui_text/layout/mod.rs @@ -586,6 +586,80 @@ mod measure_fits_render { } } + /// **A capped quest-log row title ellipsizes on ONE line — it never wraps into the row.** + /// + /// Stock `QuestLogFrame.lua:196-203` shrinks a tagged row's title to `275 − 15 − tagWidth` so + /// it cannot run under the right-flush `(Elite)`. Decision 1873 declined that cap on the + /// reading that an explicit width is a **wrap** width here, so a long title would spill onto a + /// second line inside the 16-unit row; 1944 then put the reference's own file on the chain and + /// the cap went live with it. The reading was wrong about our own engine, and this pins why: + /// the title FontString carries a DECLARED height (`` `y="10"`), which arms both + /// overflow regimes — the ellipsis gate (`boxW > 0 && boxH > 0`, wow-re + /// `fontstring-overflow.md`; [`measure::ellipsize_to_fit`]) and the line stack + /// ([`overflow::lines_allowed`]) — and each on its own holds the paint to one line. The + /// reference does exactly this: its gate holds on the same declared height and its display + /// string is `prefix + "..."`. + /// + /// The geometry is the reference's own: row `300×16`, title `GameFontNormal` (FRIZQT__ 12) in a + /// `y="10"` ButtonText (`QuestLogFrame.xml:5-7, 92-103`). + #[test] + fn a_capped_quest_log_title_ellipsizes_on_one_line() { + let Some(mut e) = test_engine(1.0) else { + eprintln!("skipping: no install / font chain"); + return; + }; + // GameFontNormal: FRIZQT__ at 12 (Fonts.xml l.70-75). The row's title box is 10 tall. + let s = spec(12.0); + let r = measure::resolve(&mut e, &s); + const BOX_H: f32 = 10.0; + // The longest tag `QuestInfo.dbc` ships is "World Event"; "(Dungeon)" is the longest one a + // quest actually carries in bulk (411 quests, decision 1873's census). Either way the cap + // is `275 - 15 - tagWidth`. + for tag in ["(Elite)", "(Dungeon)", "(World Event)"] { + let cap = 275.0 - 15.0 - measure_text(&mut e, tag, None, s).0; + for title in [ + // A real 1.11 Dungeon-tagged quest, indented as the row indents it. + " The Left Piece of Lord Valthalak's Amulet", + " The Right Piece of Lord Valthalak's Amulet", + " Bring Me The Head of Nekrum Gutchewer!", + ] { + e.ensure_metrics(r.face, r.ppem, title); + e.ensure_metrics(r.face, r.ppem, "..."); + // The cap bites: uncapped the title is one line, capped it needs more than one. + assert_eq!( + measure::wrapped_rows_for_test(&e, r, title, 275.0), + 1, + "{title:?} fits the uncapped row" + ); + assert!( + measure::wrapped_rows_for_test(&e, r, title, cap) > 1, + "{title:?} must overflow the {cap}px cap for this test to mean anything" + ); + // Regime 2, the line stack: a 10-tall box emits ONE line whatever the wrap says. + assert_eq!( + overflow::lines_allowed(BOX_H, r.size), + 1, + "the row's 10-tall title box stacks exactly one line" + ); + // Regime 3, the ellipsis: the drawn string is a prefix + "..." that fits that line. + let cap_rows = overflow::lines_fitting(BOX_H.max(r.size), r.size) + 1; + let display = overflow::ellipsize_in_box(title, BOX_H, r.size, |candidate| { + measure::wrapped_rows_capped_for_test(&e, r, candidate, cap, cap_rows) + }) + .unwrap_or_else(|| panic!("{title:?} at cap {cap} must ellipsize")); + assert!( + display.ends_with("..."), + "the truncation marker is three dots, got {display:?}" + ); + assert_eq!( + measure::wrapped_rows_for_test(&e, r, &display, cap), + 1, + "{display:?} must fit the row's single line at cap {cap}" + ); + } + } + } + /// **One line is ONE baseline — at every DPI, size and seat.** /// /// The emit pass's whole claim (decision 1342) is that a line is flat *by construction*: one diff --git a/crates/benilla-app/src/ui_unit.rs b/crates/benilla-app/src/ui_unit.rs index f955bfe2..1f22b2e8 100644 --- a/crates/benilla-app/src/ui_unit.rs +++ b/crates/benilla-app/src/ui_unit.rs @@ -1008,7 +1008,12 @@ pub(crate) fn enrich_unit( let Some(entry) = benilla_protocol::guid::entry(guid) else { return; }; - if let Some(rec) = names.creature_record(entry) { + // The ask-once creature record — `CGUnit+0xb30`, filled from `SMSG_CREATURE_QUERY_RESPONSE`. + // `None` is the round trip before the answer lands, and it is a REAL state the plate is drawn + // in, not a "nothing is known yet" to render blank: the name reads `UNKNOWNOBJECT` there + // (decision 2040), and the lines below split on exactly what the record does or does not gate. + let rec = names.creature_record(entry); + if let Some(rec) = rec { state.subtitle = rec.subname.clone(); state.creature_type_name = creature_type_word(rec.creature_type).map(str::to_string); // The client's one rank getter, both gates (`gated_rank`, decision 0782) — never `rec.rank` @@ -1017,25 +1022,31 @@ pub(crate) fn enrich_unit( state.rank = crate::names::gated_rank(Some(rec), Some(store)); state.civilian = rec.civilian; state.racial_leader = rec.racial_leader; - // The faction-name line ("Stormwind", between level and PvP) — the unit builder's tail - // block, every gate transcribed: the record's HIDE_FACTION_TOOLTIP type flag (0x10, the - // `0x612610` gate), the template → Faction.dbc hop, the reputation-slot gate - // (rep_index ≥ 0), and the race/class slot walk with its hidden flag (0x4). The record - // gate also stands in for the bytes' "no creature info → pass": before the query - // answers we have no name line either, and the tooltip rebuilds when it lands. - if rec.type_flags & 0x10 == 0 { - state.faction_name = (|| { - let catalog = factions?.catalog(); - let faction_id = catalog.template(store.0.unit_faction_template()?)?.faction; - let info = catalog.reputation_faction(faction_id)?; - let self_store = self_store?; - let race = self_store.0.unit_race().unwrap_or(0); - let class = self_store.0.unit_class().unwrap_or(0); - info.tooltip_shows_for(race, class) - .then(|| catalog.faction_name(faction_id).map(str::to_string)) - .flatten() - })(); - } + } + // The faction-name line ("Stormwind", between level and PvP) — the unit builder's tail block, + // every gate transcribed: the record's HIDE_FACTION_TOOLTIP type flag (0x10), the template → + // Faction.dbc hop, the reputation-slot gate (rep_index ≥ 0), and the race/class slot walk with + // its hidden flag (0x4). + // + // **Its entry gate is NOT the record.** `0x612610` reads `[unit+0xb30]` and returns 1 when + // there is none — that is how a PLAYER passes a gate whose only field lives in CreatureInfo, + // and a creature whose query has not answered takes the identical leg. So the line resolves + // off the DESCRIPTOR's `UNIT_FIELD_FACTIONTEMPLATE`, which is already streamed, and shows + // under a pending name exactly as it does under a known one. It was gated on the record here + // on the premise that "before the query answers we have no name line either" — the premise + // decision 2002 corrected, and 2040 with it. + if rec.is_none_or(|r| r.type_flags & 0x10 == 0) { + state.faction_name = (|| { + let catalog = factions?.catalog(); + let faction_id = catalog.template(store.0.unit_faction_template()?)?.faction; + let info = catalog.reputation_faction(faction_id)?; + let self_store = self_store?; + let race = self_store.0.unit_race().unwrap_or(0); + let class = self_store.0.unit_class().unwrap_or(0); + info.tooltip_shows_for(race, class) + .then(|| catalog.faction_name(faction_id).map(str::to_string)) + .flatten() + })(); } } @@ -2484,6 +2495,99 @@ mod tests { assert_eq!(s.rank, 0, "an un-queried creature has no classification"); } + /// **The faction-name line does not wait for the creature query** (decision 2040). + /// + /// Its entry gate `0x612610` reads `[unit+0xb30]` and **returns 1 when there is none** — the + /// leg a PLAYER takes through a gate whose only field lives in CreatureInfo, and the leg a + /// creature takes for the round trip before `SMSG_CREATURE_QUERY_RESPONSE` lands. Everything + /// the line itself needs is on the descriptor (`UNIT_FIELD_FACTIONTEMPLATE`) and already + /// streamed, so it shows under the pending `UNKNOWNOBJECT` title rather than arriving a round + /// trip after it. It was gated on the record here, on the premise that a pending creature had + /// no name line either — the premise decision 2002 corrected. + /// + /// The record still owns the line's ONE creature-side gate, `HIDE_FACTION_TOOLTIP` (type flag + /// `0x10`), which is why the answer landing can take the line away again — the reference's + /// own sequence, not a flicker of ours. + #[test] + fn the_faction_line_does_not_wait_for_the_creature_query() { + use benilla_protocol::messages::ObjectFields; + + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let catalog = benilla_formats::load_faction_catalog(&mut chain).expect("Faction.dbc"); + let factions = crate::target::Factions::from_catalog(catalog); + + /// `UNIT_FIELD_FACTIONTEMPLATE` / `UNIT_FIELD_BYTES_0` — absolute descriptor indices. + const FACTIONTEMPLATE: u16 = 35; + const BYTES_0: u16 = 36; + /// The local player the slot walk is matched against: race 1 (human), class 1 (warrior), + /// packed as `UNIT_FIELD_BYTES_0` bytes 0 and 1. + const HUMAN_WARRIOR: u32 = 1 | (1 << 8); + /// A creature entry the cache below is deliberately never told about. + const ENTRY: u32 = 299; + const GUID: u64 = (0xF130u64 << 48) | ((ENTRY as u64) << 24) | 0x7; + + let me = ObjectStore(ObjectFields::from_pairs(&[(BYTES_0, HUMAN_WARRIOR)])); + // The first template id whose faction carries a reputation slot this character can see. + // Derived from the real DBC rather than guessed, so the test names no id it cannot justify. + let (template_id, expected) = (1u32..3000) + .find_map(|id| { + let f = factions.catalog().template(id)?.faction; + let info = factions.catalog().reputation_faction(f)?; + info.tooltip_shows_for(1, 1) + .then(|| factions.catalog().faction_name(f)) + .flatten() + .map(|n| (id, n.to_string())) + }) + .expect("some faction template shows a tooltip line to a human warrior"); + let store = ObjectStore(ObjectFields::from_pairs(&[(FACTIONTEMPLATE, template_id)])); + + let line_for = |names: &NameCache| { + let mut state = UnitState::default(); + enrich_unit(&mut state, GUID, names, &store, Some(&factions), Some(&me)); + state + }; + + // The query is still in flight: no record, so no name, no subtitle and no rank — and the + // faction line all the same. + let pending = line_for(&NameCache::default()); + assert_eq!(pending.name, None, "the name is the thing still in flight"); + assert_eq!(pending.subtitle, None); + assert_eq!( + pending.faction_name.as_deref(), + Some(expected.as_str()), + "the faction line resolves off the descriptor alone" + ); + + let record = |type_flags: u32| crate::names::CreatureRecord { + name: "Stormwind Guard".into(), + subname: None, + creature_type: 7, + pet_family: 0, + rank: 0, + type_flags, + civilian: false, + racial_leader: false, + display_id: 0, + }; + let mut answered = NameCache::default(); + answered.insert_creature(ENTRY, Some(record(0))); + assert_eq!( + line_for(&answered).faction_name.as_deref(), + Some(expected.as_str()), + "the answer landing keeps the line it was already showing" + ); + + // The one creature-side gate the record does own. + let mut hidden = NameCache::default(); + hidden.insert_creature(ENTRY, Some(record(0x10))); + assert_eq!( + line_for(&hidden).faction_name, + None, + "HIDE_FACTION_TOOLTIP takes the line away once the record says so" + ); + } + /// The PvP-preference announcement law (decision 0652), as the reference's changed-bits handler /// runs it: silent on first sight, one pair per real edge, and the OFF text is the one that /// explains the five-minute wait — the whole reason the toggle doesn't read as dead. diff --git a/crates/benilla-app/tests/message_keys.rs b/crates/benilla-app/tests/message_keys.rs index 9ea22004..7a75310b 100644 --- a/crates/benilla-app/tests/message_keys.rs +++ b/crates/benilla-app/tests/message_keys.rs @@ -95,10 +95,21 @@ fn every_error_key_in_the_source_is_a_catalog_row() { #[test] fn every_error_key_in_the_source_resolves_to_real_text() { /// Keys whose row the shipped `GlobalStrings.lua` has **no string for**, so the reference - /// itself shows nothing when it raises them. Both are documented where they are raised — - /// `ui_items::equip_error` (errorId 362) and `ui_action::cast_fail` (the pet-happiness arm). - /// A third entry here is a defect until someone proves otherwise. - const SILENT_IN_5875: &[&str] = &["ERR_CANT_BE_DISENCHANTED", "ERR_NOT_HAPPY_ENOUGH"]; + /// itself shows nothing when it raises them. Each is documented where it is raised — + /// `ui_items::equip_error` (errorId 362), `ui_action::cast_fail` (the pet-happiness arm), and + /// `net::apply::pet` (errorId 337, both pet handlers' NOPATH arm). A fourth entry here is a + /// defect until someone proves otherwise. + /// + /// `ERR_PET_SPELL_NOPATH` is the one worth reading twice, because it is a trap: 5875 *does* + /// ship a `PET_SPELL_NOPATH` ("No path available for your pet"), one letter-group away and + /// perfectly plausible on screen — but it is not row 337's key and no code path in the client + /// raises it. benilla named it for months and showed the wrong string for the wrong byte + /// (decision 2033). Silence here is the reference's behaviour, not a gap. + const SILENT_IN_5875: &[&str] = &[ + "ERR_CANT_BE_DISENCHANTED", + "ERR_NOT_HAPPY_ENOUGH", + "ERR_PET_SPELL_NOPATH", + ]; let data = match benilla_formats::wow_data() { Some(d) => d, @@ -268,3 +279,42 @@ fn every_sounding_catalog_row_also_has_text_to_show() { } assert_eq!(sounding, 86, "56 voice lines + 30 named cues"); } + +/// **The non-`ERR_` half of the same tripwire, for the one family that has one** (decision 2039): +/// every string `pet_tame_failure_key` can return must be a real global of the 1.12 client. +/// +/// The walk above collects `"ERR_…"` literals, which is every catalog row — and is therefore +/// blind to a raise site that names something else. `SMSG_PET_TAME_FAILURE` is precisely that +/// shape: the message it raises is `ERR_TAME_FAILED`, but the *text* comes from a second lookup +/// of a `PETTAME_*` key, and an invented one there would fail exactly the way this file's header +/// describes — silently, with no line shown and nobody to file it. +/// +/// Checked against `reference/1.12-globals.tsv`, the running client's own `_G` (decision 1189), +/// rather than against the install: it is in-tree, so this holds on a machine with no client. +#[test] +fn every_pettame_key_is_a_real_1_12_global() { + let tsv = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../reference/1.12-globals.tsv" + ); + let text = std::fs::read_to_string(tsv).expect("the reference surface"); + let globals: BTreeSet<&str> = text + .lines() + .filter(|l| !l.starts_with('#')) + .filter_map(|l| l.split('\t').next()) + .collect(); + + // The whole reachable image of the table: 1..=11 index it, everything else is the default + // arm. `u8` is small enough to just walk. + let reachable: BTreeSet<&str> = (0..=u8::MAX) + .map(benilla_protocol::messages::pet_tame_failure_key) + .collect(); + assert_eq!(reachable.len(), 12, "eleven arms plus the default"); + for key in &reachable { + assert!(globals.contains(key), "{key} is not a 1.12 global"); + } + // The message the fill goes into, and the two bodiless arms beside it. + for key in ["ERR_TAME_FAILED", "ERR_INVALID_PETNAME", "ERR_PET_BROKEN"] { + assert!(globals.contains(key), "{key} is not a 1.12 global"); + } +} diff --git a/crates/benilla-formats/src/creature_sound.rs b/crates/benilla-formats/src/creature_sound.rs index a864c2d4..dcc82e29 100644 --- a/crates/benilla-formats/src/creature_sound.rs +++ b/crates/benilla-formats/src/creature_sound.rs @@ -7,6 +7,13 @@ //! FootstepTerrainLookup **class**, NOT a kit), AggroID, WingFlapID, WingGlideID, AlertID, //! Fidget[4], CustomAttack[4], NPCSoundID, LoopSoundID, CreatureImpactType (0 flesh · 1 stone · //! 2 wood · 3 ethereal), JumpStartID, JumpEndID, PetAttackID, PetOrderID, PetDismissID`. +//! Columns **27/28/29** are the pet's own voice (decision 2039): 27 and 28 are what the bark +//! dispatcher `0x623a40` reaches at states 2 and 1 — `SMSG_PET_ACTION_SOUND`'s two selectors — +//! and 29 is `SMSG_PET_DISMISS_SOUND`'s, resolved fresh by model id at `0x604140` rather than off +//! the cached row, which is why a census over the cache's consumers missed it. The **names** here +//! are the community's and stay INFERRED — a DBC carries no column names — but the shipped kits +//! corroborate them: the four rows that populate 27/28 name `A__KILL` and `A__ORDER`. +//! The column nothing reads is **22**, which is also empty on all 406 rows. //! Spot-check row 26: exertion 312/313, injury 315/316/0, death 314, stun 690, stand 317, //! footstep class 8, aggro 694, alert 1107 — all coherent kit-id ranges. //! The chain is `UNIT_FIELD_DISPLAYID` → `CreatureDisplayInfo.SoundID`, **falling back to @@ -54,11 +61,27 @@ pub struct CreatureVoice { pub impact_type: u32, pub jump_start: u32, pub jump_end: u32, + /// **Column 27** — the bark a pet gives when it takes an attack order + /// (`SMSG_PET_ACTION_SOUND` selector `PET_TALK_ATTACK`, the reference's bark state **2**, + /// read at `0x623ad3` as `[row+0x6c]`). + pub pet_attack: u32, + /// **Column 28** — the bark a pet gives when it acknowledges an ordered spell + /// (`SMSG_PET_ACTION_SOUND` selector `PET_TALK_SPECIAL_SPELL`, the reference's bark state + /// **1**, read at `0x623ac8` as `[row+0x70]`). + pub pet_order: u32, + /// **Column 29** — what a dismissed pet says as it goes (`SMSG_PET_DISMISS_SOUND`'s handler + /// `0x604140`, read at `0x6041c4`/`0x6041dc` as `[row+0x74]`). + /// + /// Not a bark: it is off `0x623a40`'s table entirely, played at a bare world position with no + /// unit, no latch and no attach point — because by the time it sounds the pet is gone. + pub pet_dismiss: u32, } -/// display id → voice rows, joined through `CreatureDisplayInfo.SoundID`. +/// display id → voice rows, joined through `CreatureDisplayInfo.SoundID`, plus the raw +/// `CreatureModelData.SoundID` join the dismiss sound needs on its own. pub struct CreatureVoiceCatalog { display_to_sound: HashMap, + model_to_sound: HashMap, rows: HashMap, } @@ -68,6 +91,17 @@ impl CreatureVoiceCatalog { self.rows.get(self.display_to_sound.get(&display_id)?) } + /// The voice set for a **`CreatureModelData` id**, through that table's own `SoundID` + /// (col 13) with no display in between. + /// + /// The odd one out, and deliberately so: every other caller starts from + /// `UNIT_FIELD_DISPLAYID`, but `SMSG_PET_DISMISS_SOUND` names a model directly and the + /// reference resolves it exactly this way (`0x604140`: `[[0xc0de68] + id*4] + 0x34` → + /// `[[0xc0de54] + sound*4]`) — no display step, no fallback. + pub fn for_model(&self, model_id: u32) -> Option<&CreatureVoice> { + self.rows.get(self.model_to_sound.get(&model_id)?) + } + pub fn len(&self) -> usize { self.rows.len() } @@ -130,6 +164,9 @@ pub fn load_creature_voice_catalog(chain: &mut Chain) -> Result Result_KILL`, `A__ORDER` and + /// `A__DISMISS`. That naming is an *independent* confirmation of which column is + /// which: the reference sends `PET_TALK_ATTACK` to bark state 2 = column 27, + /// `PET_TALK_SPECIAL_SPELL` to state 1 = column 28, and `SMSG_PET_DISMISS_SOUND` to column 29 + /// — and the data calls those three `_KILL`, `_ORDER` and `_DISMISS`, in that order, three + /// times over (decision 2039; it closes wow-re's open question about the names, + /// `feign-death-dyndead.md` §12). + /// + /// **A hunter pet is therefore silent on all three, faithfully** — its row's columns are 0, + /// so the reference reads a zero kit and plays nothing. The Felhunter has no kit in the file + /// at all. Skips without client data. + #[test] + fn only_the_four_demon_voices_carry_pet_barks() { + let data = crate::wow_data_or_skip!(); + let mut chain = crate::open_chain(&data).expect("open chain"); + let cat = load_creature_voice_catalog(&mut chain).expect("load creature voices"); + let kits = crate::load_sound_kit_catalog(&mut chain).expect("load SoundEntries"); + + let named = |id: u32| kits.get(id).map(|k| k.name.clone()).unwrap_or_default(); + let mut carrying: Vec<(u32, String, String, String)> = cat + .rows + .iter() + .filter(|(_, v)| v.pet_attack != 0 || v.pet_order != 0 || v.pet_dismiss != 0) + .map(|(id, v)| { + ( + *id, + named(v.pet_attack), + named(v.pet_order), + named(v.pet_dismiss), + ) + }) + .collect(); + carrying.sort_unstable(); + // Casing is the file's own — `A_Imp_Dismiss` and `A_DOOMGUARD_DISMISS01` are how those two + // rows are written, and the lookup is case-insensitive anyway. + assert_eq!( + carrying, + vec![ + ( + 11, + "A_IMP_KILL".into(), + "A_IMP_ORDER".into(), + "A_Imp_Dismiss".into() + ), + ( + 37, + "A_SUCCUBUS_KILL".into(), + "A_SUCCUBUS_ORDER".into(), + "A_SUCCUBUS_DISMISS".into() + ), + ( + 68, + "A_DOOMGUARD_KILL".into(), + "A_DOOMGUARD_ORDER".into(), + "A_DOOMGUARD_DISMISS01".into() + ), + ( + 162, + "A_VOIDWALKER_KILL".into(), + "A_VOIDWALKER_ORDER".into(), + "A_VOIDWALKER_DISMISS".into() + ), + ], + ); + + // Reachable from real displays, not orphan rows: the Imp's voice is the one 25 of them + // share, and the wolf a hunter tames carries none of the three. + let imp = cat.for_display(904).expect("an imp display resolves"); + assert_eq!( + (imp.pet_attack, imp.pet_order, imp.pet_dismiss), + (9097, 9098, 9096) + ); + let wolf = cat.for_display(903).expect("the Elwynn wolf resolves"); + assert_eq!( + (wolf.pet_attack, wolf.pet_order, wolf.pet_dismiss), + (0, 0, 0) + ); + + // The dismiss sound's own join — `CreatureModelData.SoundID` with **no display step** + // (`0x604140`). The imp's model must reach the same row its displays do, or the packet + // would be silent for the one pet it exists for. + let by_model = cat + .for_model( + *cat.model_to_sound + .iter() + .find(|(_, s)| **s == 11) + .expect("a model resolves the imp voice") + .0, + ) + .expect("model join"); + assert_eq!(by_model.pet_dismiss, 9096); + } } diff --git a/crates/benilla-protocol/src/events.rs b/crates/benilla-protocol/src/events.rs index 475dba71..bfb6ed16 100644 --- a/crates/benilla-protocol/src/events.rs +++ b/crates/benilla-protocol/src/events.rs @@ -681,6 +681,22 @@ pub enum SessionEvent { /// The pet's cast refusal (`SMSG_PET_CAST_FAILED`) — [`Self::CastResult`]'s vocabulary, but /// the caster is the pet, so it never touches OUR cast state. PetCastFailed { spell_id: u32, reason: Option }, + /// A refused tame / Call Pet / Revive Pet (`SMSG_PET_TAME_FAILURE`): one + /// `PetTameFailureReason` byte, whose text is a `PETTAME_*` GlobalStrings key filling + /// `ERR_TAME_FAILED`. + PetTameFailure { reason: u8 }, + /// A refused pet rename (`SMSG_PET_NAME_INVALID`) — no payload; the reference raises + /// `ERR_INVALID_PETNAME` on the strength of the opcode alone. + PetNameInvalid, + /// The pet's loyalty hit zero and it ran away (`SMSG_PET_BROKEN`) — no payload; + /// `ERR_PET_BROKEN`. + PetBroken, + /// The pet's voice (`SMSG_PET_ACTION_SOUND`) — one of two talk selectors on a named unit, + /// resolved against that unit's own `CreatureSoundData` row. + PetActionSound { pet_guid: u64, talk: u32 }, + /// A dismissed pet's parting sound (`SMSG_PET_DISMISS_SOUND`) — a `CreatureModelData` id and + /// the raw WoW point to play its column-29 kit at. No guid, because the pet has gone. + PetDismissSound { model_id: u32, position: [f32; 3] }, /// An item template's display head (`SMSG_ITEM_QUERY_SINGLE_RESPONSE`, answering our /// `CMSG_ITEM_QUERY_SINGLE`). Keyed by template entry; `None` = the server doesn't know it /// (undiscovered) — cached negative, like an unknown creature entry. Boxed for the same reason diff --git a/crates/benilla-protocol/src/events/decode.rs b/crates/benilla-protocol/src/events/decode.rs index 59be31b3..92d9f312 100644 --- a/crates/benilla-protocol/src/events/decode.rs +++ b/crates/benilla-protocol/src/events/decode.rs @@ -136,6 +136,18 @@ pub fn decode(packet: ServerPacket) -> Vec { CastOutcome::Failed { reason, .. } => Some(reason), }, }], + ServerPacket::PetTameFailure { reason } => vec![SessionEvent::PetTameFailure { reason }], + ServerPacket::PetNameInvalid => vec![SessionEvent::PetNameInvalid], + ServerPacket::PetBroken => vec![SessionEvent::PetBroken], + ServerPacket::PetActionSound { pet_guid, talk } => { + vec![SessionEvent::PetActionSound { pet_guid, talk }] + } + ServerPacket::PetDismissSound { model_id, position } => { + vec![SessionEvent::PetDismissSound { + model_id, + position: [position.x, position.y, position.z], + }] + } ServerPacket::ItemQueryResponse { entry, info } => { vec![SessionEvent::ItemTemplate { entry, info }] } diff --git a/crates/benilla-protocol/src/messages/mod.rs b/crates/benilla-protocol/src/messages/mod.rs index ca78bbfe..ba7a2ed7 100644 --- a/crates/benilla-protocol/src/messages/mod.rs +++ b/crates/benilla-protocol/src/messages/mod.rs @@ -185,12 +185,13 @@ pub use page_text::page_text_query; pub use parse::parse_server; pub use pet::{ pet_abandon, pet_action, pet_cancel_aura, pet_rename, pet_set_action, pet_spell_autocast, - pet_stop_attack, pet_unlearn, PetActionEntry, PetMode, PetSpellCooldown, PetSpells, - PetUnlearnConfirm, PET_ACTION_SLOTS, PET_ACT_COMMAND, PET_ACT_DISABLED, PET_ACT_ENABLED, - PET_ACT_PASSIVE, PET_ACT_REACTION, PET_AUTOCAST_ALLOWED, PET_AUTOCAST_ON, PET_COMMAND_ATTACK, - PET_COMMAND_DISMISS, PET_COMMAND_FOLLOW, PET_COMMAND_STAY, PET_COOLDOWN_PERMANENT, - PET_REACT_AGGRESSIVE, PET_REACT_DEFENSIVE, PET_REACT_PASSIVE, PET_STATE_BAR_DISABLED, - PET_TYPE_SPELL_FIRST, PET_TYPE_SPELL_LAST, PET_UNUSABLE_UNIT_FLAGS, + pet_stop_attack, pet_tame_failure_key, pet_unlearn, PetActionEntry, PetMode, PetSpellCooldown, + PetSpells, PetUnlearnConfirm, PET_ACTION_SLOTS, PET_ACT_COMMAND, PET_ACT_DISABLED, + PET_ACT_ENABLED, PET_ACT_PASSIVE, PET_ACT_REACTION, PET_AUTOCAST_ALLOWED, PET_AUTOCAST_ON, + PET_COMMAND_ATTACK, PET_COMMAND_DISMISS, PET_COMMAND_FOLLOW, PET_COMMAND_STAY, + PET_COOLDOWN_PERMANENT, PET_REACT_AGGRESSIVE, PET_REACT_DEFENSIVE, PET_REACT_PASSIVE, + PET_STATE_BAR_DISABLED, PET_TALK_ATTACK, PET_TALK_ORDER, PET_TYPE_SPELL_FIRST, + PET_TYPE_SPELL_LAST, PET_UNUSABLE_UNIT_FLAGS, }; pub use petition::{ offer_petition, petition_buy, petition_decline, petition_query, petition_rename, diff --git a/crates/benilla-protocol/src/messages/opcode.rs b/crates/benilla-protocol/src/messages/opcode.rs index 90e79459..b0446672 100644 --- a/crates/benilla-protocol/src/messages/opcode.rs +++ b/crates/benilla-protocol/src/messages/opcode.rs @@ -210,6 +210,65 @@ pub const SMSG_PET_ACTION_FEEDBACK: u16 = 0x02C6; // 710 /// because the caster that failed is the pet, not us. pub const SMSG_PET_CAST_FAILED: u16 = 0x0138; // 312 +// The three remaining pet-feedback arms, carved together (decision 2039). Each was name-table-only +// until then, and each has a handler in the reference that does something visible. +/// **The taming refusal** — one `u8` reason from `PetTameFailureReason`. Not only taming: vmangos +/// sends it for Call Pet and Revive Pet too (`SpellEffects.cpp:3167/3174`, `Spell.cpp:5463/6113`). +/// +/// The reference's handler `0x6e97e0` reads the byte and hands it to `0x6e6a20`, which turns +/// `reason - 1` into one of eleven `PETTAME_*` GlobalStrings keys through the jump table at +/// `0x6e6ac0`, resolves it (`0x703bf0`) and raises `DisplayError(0xee)` = +/// `ERR_TAME_FAILED` (`"%s."`) with that string as the fill. Anything outside `1..=11` — `0` and +/// `12` included — takes the default arm's `PETTAME_UNKNOWNERROR`. The map is in +/// [`super::pet::pet_tame_failure_key`]. +pub const SMSG_PET_TAME_FAILURE: u16 = 0x0173; // 371 +/// **The refused rename** — an EMPTY body, and a visible error all the same. +/// +/// vmangos's `SendPetNameInvalid` drops both the reason code and the name with the comment "not +/// read by vanilla client" (`PetHandler.cpp:542-548`), and the reference agrees: its arm reads no +/// body. What it does read is the *opcode* — 0x178 lands in the shared dispatcher `0x5e38c0`, +/// whose index byte at `0x5e4c6c[0]` selects target `0x5e4c40[0]` = `0x5e3e33`, five instructions: +/// `push 0xf7; call 0x496720` = `DisplayError(247)` = **`ERR_INVALID_PETNAME`**, "Error, invalid +/// name entered.". +/// +/// **Decision 1066 said the opposite** — that a refused rename "silently does nothing, which is +/// what the reference does too" — on a carve that had attributed [`SMSG_PET_BROKEN`]'s handler +/// `0x4bdc00` to this opcode. `re/net/opcode-handlers.tsv` maps 0x178 to `0x5e38c0` and 0x2AF to +/// `0x4bdc00`; they are different functions raising different messages. +pub const SMSG_PET_NAME_INVALID: u16 = 0x0178; // 376 +/// **The pet ran away** — an empty body; vmangos sends it when a hunter pet's loyalty hits zero +/// (`Pet.cpp:822`, immediately before `Unsummon(PET_SAVE_AS_DELETED)`). +/// +/// Handler `0x4bdc00`: `push 0x1a3; call 0x496720` = `DisplayError(419)` = `ERR_PET_BROKEN`, +/// "Your pet has run away". It reads no body, writes no state and fires no event — the bar's +/// teardown rides the `SMSG_PET_SPELLS` zero-guid form as usual (wow-re +/// `ui/scratch/pet-action-bar-api.md` §11c.5). +pub const SMSG_PET_BROKEN: u16 = 0x02AF; // 687 +/// **The pet's voice** — `u64 petGuid` then a `u32` *talk* selector (not a `SoundEntries` id): +/// `0` = `PET_TALK_SPECIAL_SPELL`, `1` = `PET_TALK_ATTACK` (vmangos `Pet.h:98`, +/// `Unit::SendPetTalk`). +/// +/// Handler `0x6040c0` resolves the guid, then calls the creature bark dispatcher +/// `0x623a40(unit, state)` with **state 1 for selector 0 and state 2 for selector 1** — i.e. +/// `CreatureSoundData` columns 28 (`PetOrder`) and 27 (`PetAttack`). Any other selector plays +/// nothing. The bark rides the unit's ordinary one-shot voice slot and its priority latch, so it +/// is not a free-standing sound (see `crate::sound::creature`'s bark core on the benilla side). +pub const SMSG_PET_ACTION_SOUND: u16 = 0x0324; // 804 +/// **The dismissed pet's parting sound** — `u32 creatureModelDataId` then a `f32` x/y/z, and the +/// odd one of the family in every way. +/// +/// Handler `0x604140` reads those four fields, walks **`CreatureModelData[id]` → its own +/// `SoundID` (col 13, `[row+0x34]`) → `CreatureSoundData` → column 29**, and plays that kit at the +/// given point with `z + 1.0` (`0x7ff9d8`) and a free-picked variation. No unit is involved: it is +/// off the bark dispatcher's table entirely, takes no voice latch and has no attach point — +/// because by the time it sounds the pet has gone. That fresh, inlined, by-id resolve is also why +/// a census over the *cached* `[unit+0xb40]` row's consumers concluded column 29 was dead. +/// +/// **vmangos never sends it** (no `SendPetDismissSound` anywhere in the tree; the opcode is +/// registered only as server-bound). Built because the reference is the spec and this handler is +/// what makes column 29 load-bearing — not because anything we talk to can trigger it. +pub const SMSG_PET_DISMISS_SOUND: u16 = 0x0325; // 805 + /// The client's ack that a server-authored spline (`SMSG_MONSTER_MOVE` to our own guid — Charge, /// knockback, taxi) finished. Body: a `MovementInfo` at the endpoint, the `splineId` being acked, /// and a trailing float the server `read_skip`s (VERIFIED vmangos `Opcodes_1_12_1.h`: 713; @@ -408,11 +467,10 @@ pub const CMSG_PET_ABANDON: u16 = 0x0176; // 374 /// disappears after the first rename (`PetHandler.cpp:302-345`). Nothing client-side clears it — /// wow-re's census found no writer of that byte anywhere in `.text`. /// -/// A refused name answers with `SMSG_PET_NAME_INVALID` (0x178), which is **not modelled** and does -/// not need to be: vmangos sends it with an empty body — `SendPetNameInvalid` drops both the reason -/// code and the name with the comment "not read by vanilla client" (`PetHandler.cpp:542-548`) — and -/// the shipped 1.12 `FrameXML` has no event and no string for it. It lands in `ServerPacket::Other` -/// under its own name, which is the whole of what there is to do with it. +/// A refused name answers with [`SMSG_PET_NAME_INVALID`] (0x178). Its body really is empty — but +/// the reference is **not** silent about it: the handler raises `ERR_INVALID_PETNAME` on the red +/// line. Decision 1066's claim that it does nothing was a mis-attributed handler; see that +/// constant's own note. pub const CMSG_PET_RENAME: u16 = 0x0177; // 375 /// VERIFIED vmangos `Opcodes_1_12_1.h`: 94 (decision 0236). Body in diff --git a/crates/benilla-protocol/src/messages/packet.rs b/crates/benilla-protocol/src/messages/packet.rs index 820be38d..74186562 100644 --- a/crates/benilla-protocol/src/messages/packet.rs +++ b/crates/benilla-protocol/src/messages/packet.rs @@ -643,6 +643,27 @@ pub enum ServerPacket { spell_id: u32, outcome: CastOutcome, }, + /// `SMSG_PET_TAME_FAILURE` — one `PetTameFailureReason` byte; the red line's text comes from + /// [`super::pet::pet_tame_failure_key`]. + PetTameFailure { + reason: u8, + }, + /// `SMSG_PET_NAME_INVALID` — the refused rename. Empty body: the opcode IS the message. + PetNameInvalid, + /// `SMSG_PET_BROKEN` — the pet's loyalty hit zero and it ran away. Empty body. + PetBroken, + /// `SMSG_PET_ACTION_SOUND` — the pet's voice: which unit, and which of the two talk + /// selectors ([`super::pet::PET_TALK_ORDER`] / [`super::pet::PET_TALK_ATTACK`]). + PetActionSound { + pet_guid: u64, + talk: u32, + }, + /// `SMSG_PET_DISMISS_SOUND` — a `CreatureModelData` id and the point to play its column-29 + /// kit at. No guid: the pet is already gone. + PetDismissSound { + model_id: u32, + position: Vector3d, + }, /// `SMSG_ATTACKSTART` — a unit began melee auto-attack (including our own echo). AttackStart { attacker: u64, @@ -1661,6 +1682,11 @@ impl ServerPacket { ServerPacket::PetSpells(_) => "SMSG_PET_SPELLS".into(), ServerPacket::PetMode(_) => "SMSG_PET_MODE".into(), ServerPacket::PetActionFeedback { .. } => "SMSG_PET_ACTION_FEEDBACK".into(), + ServerPacket::PetTameFailure { .. } => "SMSG_PET_TAME_FAILURE".into(), + ServerPacket::PetNameInvalid => "SMSG_PET_NAME_INVALID".into(), + ServerPacket::PetBroken => "SMSG_PET_BROKEN".into(), + ServerPacket::PetActionSound { .. } => "SMSG_PET_ACTION_SOUND".into(), + ServerPacket::PetDismissSound { .. } => "SMSG_PET_DISMISS_SOUND".into(), ServerPacket::PetCastFailed { .. } => "SMSG_PET_CAST_FAILED".into(), ServerPacket::AttackStart { .. } => "SMSG_ATTACKSTART".into(), ServerPacket::AttackStop { .. } => "SMSG_ATTACKSTOP".into(), diff --git a/crates/benilla-protocol/src/messages/parse.rs b/crates/benilla-protocol/src/messages/parse.rs index e10c1d65..32142648 100644 --- a/crates/benilla-protocol/src/messages/parse.rs +++ b/crates/benilla-protocol/src/messages/parse.rs @@ -612,6 +612,21 @@ pub fn parse_server(opcode: u16, body: &[u8]) -> io::Result { let (spell_id, outcome) = pet::read_pet_cast_failed(&mut r)?; ServerPacket::PetCastFailed { spell_id, outcome } } + opcode::SMSG_PET_TAME_FAILURE => ServerPacket::PetTameFailure { + reason: pet::read_pet_tame_failure(&mut r)?, + }, + // Both bodies really are empty — the opcode is the whole message on either (vmangos's + // `AppendBodyTo` writes nothing, and the reference's handlers read nothing). + opcode::SMSG_PET_NAME_INVALID => ServerPacket::PetNameInvalid, + opcode::SMSG_PET_BROKEN => ServerPacket::PetBroken, + opcode::SMSG_PET_ACTION_SOUND => { + let (pet_guid, talk) = pet::read_pet_action_sound(&mut r)?; + ServerPacket::PetActionSound { pet_guid, talk } + } + opcode::SMSG_PET_DISMISS_SOUND => { + let (model_id, position) = pet::read_pet_dismiss_sound(&mut r)?; + ServerPacket::PetDismissSound { model_id, position } + } opcode::SMSG_ATTACKSTART => { let (attacker, victim) = attack::read_attack_start(&mut r)?; ServerPacket::AttackStart { attacker, victim } diff --git a/crates/benilla-protocol/src/messages/pet.rs b/crates/benilla-protocol/src/messages/pet.rs index 53ae0993..5b58449f 100644 --- a/crates/benilla-protocol/src/messages/pet.rs +++ b/crates/benilla-protocol/src/messages/pet.rs @@ -22,7 +22,7 @@ use std::io::{self, Read}; -use crate::wire::{read_u16_le, read_u32_le, read_u64_le, read_u8}; +use crate::wire::{read_u16_le, read_u32_le, read_u64_le, read_u8, Vector3d}; /// Pet action-bar slots (vmangos `MAX_UNIT_ACTION_BAR_INDEX` = `ACTION_BAR_INDEX_END(10) - /// ACTION_BAR_INDEX_START(0)`, `Objects/UnitDefines.h:781-787`); the client's own @@ -375,6 +375,72 @@ pub(super) fn read_pet_action_feedback(r: &mut impl Read) -> io::Result { read_u8(r) } +/// Read `SMSG_PET_TAME_FAILURE` (VERIFIED vmangos `PetTameFailure::AppendBodyTo`, +/// `Server/Packets/Pet.cpp:122-125`): one reason byte. [`pet_tame_failure_key`] is what the +/// reference does with it. +pub(super) fn read_pet_tame_failure(r: &mut impl Read) -> io::Result { + read_u8(r) +} + +/// The `SMSG_PET_TAME_FAILURE` reason -> the `GlobalStrings.lua` key whose text fills +/// `ERR_TAME_FAILED`'s lone `%s` — the reference's jump table at `0x6e6ac0`, read out of the +/// image dword for dword. +/// +/// `0x6e6a20` computes `reason - 1` and bounds it at `0xa`, so **`1..=11` index the table and +/// everything else takes the default arm** `0x6e6a87` -> `PETTAME_UNKNOWNERROR`. That default is +/// reachable in play: vmangos's own enum has a twelfth value (`PETTAME_UNKNOWNERROR = 12`) which +/// falls out of range and lands on it, arriving at the same string by the other road. +/// +/// The eleven pairs are byte-identical to vmangos's `PetTameFailureReason` +/// (`src/game/SharedDefines.h:1707`) — the two ends agree without either having been fitted to +/// the other. +pub fn pet_tame_failure_key(reason: u8) -> &'static str { + match reason { + 1 => "PETTAME_INVALIDCREATURE", + 2 => "PETTAME_TOOMANY", + 3 => "PETTAME_CREATUREALREADYOWNED", + 4 => "PETTAME_NOTTAMEABLE", + 5 => "PETTAME_ANOTHERSUMMONACTIVE", + 6 => "PETTAME_UNITSCANTTAME", + 7 => "PETTAME_NOPETAVAILABLE", + 8 => "PETTAME_INTERNALERROR", + 9 => "PETTAME_TOOHIGHLEVEL", + 10 => "PETTAME_DEAD", + 11 => "PETTAME_NOTDEAD", + _ => "PETTAME_UNKNOWNERROR", + } +} + +/// `SMSG_PET_ACTION_SOUND`'s selector: the pet acknowledging an **order** (vmangos +/// `PET_TALK_SPECIAL_SPELL`, sent when a pet bar spell press is accepted — +/// `PetHandler.cpp:523`, `PetAI.cpp:336`). +pub const PET_TALK_ORDER: u32 = 0; +/// `SMSG_PET_ACTION_SOUND`'s selector: the pet **attacking** (vmangos `PET_TALK_ATTACK`, sent +/// from `Unit.cpp:8941` when an attack order is taken). +pub const PET_TALK_ATTACK: u32 = 1; + +/// Read `SMSG_PET_ACTION_SOUND` (VERIFIED vmangos `PetActionSound::AppendBodyTo`, +/// `Server/Packets/Pet.cpp:96-100`, and the reference's own reader at `0x6040ca`/`0x6040db`): +/// `u64 petGuid` then the `u32` talk selector. +/// +/// The selector is NOT a `SoundEntries` id — it is [`PET_TALK_ORDER`] / [`PET_TALK_ATTACK`], and +/// the sound comes off the pet's own `CreatureSoundData` row. Worth saying because the opcode's +/// name invites the other reading, and a two-value selector and a kit id are indistinguishable +/// on the wire. +pub(super) fn read_pet_action_sound(r: &mut impl Read) -> io::Result<(u64, u32)> { + Ok((read_u64_le(r)?, read_u32_le(r)?)) +} + +/// Read `SMSG_PET_DISMISS_SOUND` — `u32 creatureModelDataId` then `f32` x/y/z. +/// +/// VERIFIED off **the reference's own reader**, which is the only authority there is: `0x604140` +/// takes one `0x418e30` (u32) then three `0x419130` (f32) in that order, and vmangos has no +/// sender to cross-check against. The `+1.0` the handler adds to `z` afterwards is a *play* +/// detail, not a wire one, so it is not applied here. +pub(super) fn read_pet_dismiss_sound(r: &mut impl Read) -> io::Result<(u32, Vector3d)> { + Ok((read_u32_le(r)?, Vector3d::read(r)?)) +} + /// Read `SMSG_PET_CAST_FAILED` (VERIFIED vmangos `PetCastFailed::AppendBodyTo`, /// `Server/Packets/Pet.cpp:127-132`): `u32 spellId, u8 status, u8 reason`. The pet's twin of /// `SMSG_CAST_RESULT` — the same `SpellCastResult` vocabulary, decoded the same way (status `2` = diff --git a/crates/benilla-protocol/tests/simple_packets.rs b/crates/benilla-protocol/tests/simple_packets.rs index 052fc900..b19b4e22 100644 --- a/crates/benilla-protocol/tests/simple_packets.rs +++ b/crates/benilla-protocol/tests/simple_packets.rs @@ -229,20 +229,22 @@ fn set_faction_standing_parses_and_decodes() { /// opcode numbers. #[test] fn unknown_opcode_decodes_to_packet_dropped() { - // 0x0324 = SMSG_PET_ACTION_SOUND — assigned in 1.12.1 but deliberately unparsed by benilla. + // 0x021E = SMSG_SET_REST_START — assigned in 1.12.1 but deliberately unparsed by benilla. // (The sentinel used to be 0x0319 `MSG_MOVE_TIME_SKIPPED`, which decision 1935 gave a parse - // arm: an observed mover's skipped time has to reach its relay chain or that unit hitches.) - let packet = messages::parse_server(0x0324, &hx("0102030405")).unwrap(); - assert!(matches!(packet, ServerPacket::Other { opcode: 0x0324 })); + // arm; then 0x0324 `SMSG_PET_ACTION_SOUND`, which decision 2039 gave one — the pet's voice + // turned out to drive two real `CreatureSoundData` columns. A sentinel earning a parse arm is + // this test working, not this test breaking.) + let packet = messages::parse_server(0x021E, &hx("0102030405")).unwrap(); + assert!(matches!(packet, ServerPacket::Other { opcode: 0x021E })); match decode(packet).as_slice() { [SessionEvent::PacketDropped { - opcode: 0x0324, + opcode: 0x021E, unparseable: false, }] => {} other => panic!("expected one PacketDropped event, got {other:?}"), } // The generated name table: a known opcode resolves, an unassigned number doesn't. - assert_eq!(messages::opcode_name(0x0324), Some("SMSG_PET_ACTION_SOUND")); + assert_eq!(messages::opcode_name(0x021E), Some("SMSG_SET_REST_START")); assert_eq!( messages::opcode_name(messages::opcode::SMSG_UPDATE_OBJECT), Some("SMSG_UPDATE_OBJECT") @@ -530,3 +532,94 @@ fn client_control_update_reads_a_packed_mover_and_the_allow_byte() { } )); } + +/// The hunter pet-feedback family (decision 2039) — the three arms that were name-table-only, on +/// their real bodies. +/// +/// The two **empty** bodies are the point of half this test: `SMSG_PET_NAME_INVALID` and +/// `SMSG_PET_BROKEN` carry nothing at all (vmangos's `AppendBodyTo` writes nothing; the +/// reference's handlers read nothing), so "parses" here means "the opcode alone produced the +/// event" — and a body that arrives with junk on it must not change that. +#[test] +fn pet_feedback_family_parses_and_decodes() { + use benilla_protocol::messages::opcode; + + // SMSG_PET_TAME_FAILURE: one reason byte. 9 = PETTAME_TOOHIGHLEVEL. + let p = messages::parse_server(opcode::SMSG_PET_TAME_FAILURE, &hx("09")).unwrap(); + assert!(matches!(p, ServerPacket::PetTameFailure { reason: 9 })); + assert!(matches!( + decode(p)[..], + [SessionEvent::PetTameFailure { reason: 9 }] + )); + // The reason -> GlobalStrings key table, at both ends of the reference's bounds check + // (`0x6e6a20`: `reason - 1` bounded at `0xa`) and in the middle. + assert_eq!(messages::pet_tame_failure_key(1), "PETTAME_INVALIDCREATURE"); + assert_eq!( + messages::pet_tame_failure_key(5), + "PETTAME_ANOTHERSUMMONACTIVE" + ); + assert_eq!(messages::pet_tame_failure_key(11), "PETTAME_NOTDEAD"); + // Out of range on BOTH sides falls to the default arm — including vmangos's own 12th value, + // which is `PETTAME_UNKNOWNERROR` in the enum and reaches the same string by the other road. + assert_eq!(messages::pet_tame_failure_key(0), "PETTAME_UNKNOWNERROR"); + assert_eq!(messages::pet_tame_failure_key(12), "PETTAME_UNKNOWNERROR"); + assert_eq!(messages::pet_tame_failure_key(200), "PETTAME_UNKNOWNERROR"); + + // SMSG_PET_NAME_INVALID / SMSG_PET_BROKEN: empty bodies, and unread ones — a body with bytes + // on it still decodes to the bare event, because neither handler reads one. + for body in ["", "deadbeef"] { + let p = messages::parse_server(opcode::SMSG_PET_NAME_INVALID, &hx(body)).unwrap(); + assert!(matches!(p, ServerPacket::PetNameInvalid), "body {body:?}"); + assert!(matches!(decode(p)[..], [SessionEvent::PetNameInvalid])); + + let p = messages::parse_server(opcode::SMSG_PET_BROKEN, &hx(body)).unwrap(); + assert!(matches!(p, ServerPacket::PetBroken), "body {body:?}"); + assert!(matches!(decode(p)[..], [SessionEvent::PetBroken])); + } + + // SMSG_PET_ACTION_SOUND: u64 guid then the u32 talk selector. + let body = hx("2a0000000000401001000000"); + let p = messages::parse_server(opcode::SMSG_PET_ACTION_SOUND, &body).unwrap(); + assert!(matches!( + p, + ServerPacket::PetActionSound { + pet_guid: 0x1040_0000_0000_002A, + talk: messages::PET_TALK_ATTACK, + } + )); + assert!(matches!( + decode(p)[..], + [SessionEvent::PetActionSound { + pet_guid: 0x1040_0000_0000_002A, + talk: 1 + }] + )); + + // SMSG_PET_DISMISS_SOUND: a CreatureModelData id then a raw WoW x/y/z. Model 726, at + // (-8949.95, -132.493, 83.5312) — the human starting point, chosen because its bytes are + // recognisable if the field order ever slips. The handler's `+1.0` on z is a PLAY detail and + // must NOT appear here. + let mut body = hx("d6020000"); // model 726 + for f in [-8949.95_f32, -132.493, 83.5312] { + body.extend_from_slice(&f.to_le_bytes()); + } + let p = messages::parse_server(opcode::SMSG_PET_DISMISS_SOUND, &body).unwrap(); + assert!(matches!( + p, + ServerPacket::PetDismissSound { + model_id: 726, + position: benilla_protocol::wire::Vector3d { + x: -8949.95, + y: -132.493, + z: 83.5312, + }, + } + )); + assert!(matches!( + decode(p)[..], + [SessionEvent::PetDismissSound { + model_id: 726, + position: [-8949.95, -132.493, 83.5312], + }] + )); +} diff --git a/crates/benilla-ui/src/script/taxi.rs b/crates/benilla-ui/src/script/taxi.rs index f48fe7ee..054f9eee 100644 --- a/crates/benilla-ui/src/script/taxi.rs +++ b/crates/benilla-ui/src/script/taxi.rs @@ -240,13 +240,29 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; - // → whether the given unit is riding a taxi. Only our own player is tracked (the reference - // UI only ever asks about "player"); any other token reads false. + // `UnitOnTaxi(unit)` (`0x517a40`) → **the number 1 or nil**, never a Lua boolean — the unit + // predicate family's one return shape (decisions 1830, 2043; the push law is + // `crate::script::binding_abi::predicate`). It lives here rather than beside its 22 siblings + // because the ride flag it reads is this module's, not `UnitState`'s — which is exactly how it + // came to be the one predicate still answering a `bool` after the family was fixed. + // + // Only our own player is tracked (the reference UI only ever asks about `"player"`); any other + // token reads nil. + // + // The token argument is `Value`, not `String`, so the miss is the reference's own message: the + // binding carries an `lua_isstring` gate at `0x517a48` whose failure arm is + // `luaL_error("Usage: UnitOnTaxi(\"unit\")")` (wow-re + // `ui/scratch/nil-unit-token-arg-law.md` §10, row 33). A Rust `String` parameter raised mlua's + // own type-conversion error there instead. g.set( "UnitOnTaxi", - lua.create_function(|lua, unit: String| { + lua.create_function(|lua, unit: mlua::Value| { + let unit = + crate::script::binding_abi::string_arg(lua, unit, r#"Usage: UnitOnTaxi("unit")"#)?; let model = lua.app_data_ref::().expect("model"); - Ok(unit.eq_ignore_ascii_case("player") && model.taxi_riding) + Ok(crate::script::binding_abi::predicate( + unit.eq_ignore_ascii_case("player") && model.taxi_riding, + )) })?, )?; diff --git a/crates/benilla-ui/src/script/tests/taxi.rs b/crates/benilla-ui/src/script/tests/taxi.rs index 12420251..a6856c66 100644 --- a/crates/benilla-ui/src/script/tests/taxi.rs +++ b/crates/benilla-ui/src/script/tests/taxi.rs @@ -50,7 +50,7 @@ fn taxi_snapshot_surfaces_and_intents_drain() { assert(GetNumRoutes(1) == 0 and GetNumRoutes(2) == 1) assert(math.abs(TaxiGetSrcX(2, 1) - 0.5) < 1e-6) assert(math.abs(TaxiGetDestY(2, 1) - 0.8) < 1e-6) - assert(UnitOnTaxi("player") == false) + assert(UnitOnTaxi("player") == nil) -- 1/nil, never a boolean (2043) TakeTaxiNode(2) CloseTaxiMap() @@ -74,6 +74,6 @@ fn taxi_snapshot_surfaces_and_intents_drain() { // The ride flag reaches UnitOnTaxi. s.set_on_taxi(true); - s.run(r#"assert(UnitOnTaxi("player") == true)"#).unwrap(); + s.run(r#"assert(UnitOnTaxi("player") == 1)"#).unwrap(); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } diff --git a/crates/benilla-ui/src/script/tests/tooltip_unit.rs b/crates/benilla-ui/src/script/tests/tooltip_unit.rs index e537432d..42b6dbaa 100644 --- a/crates/benilla-ui/src/script/tests/tooltip_unit.rs +++ b/crates/benilla-ui/src/script/tests/tooltip_unit.rs @@ -56,6 +56,133 @@ fn creature_line_law() { assert!(s.take_errors().is_empty()); } +/// The snapshot the app pushes for a creature the client has only just seen: the DESCRIPTOR's +/// fields are in (level, reaction, the skinnable flag) and everything the creature record carries +/// — name, subtitle, type word, rank, civilian/leader — is still absent, because they all arrive +/// together in the one `SMSG_CREATURE_QUERY_RESPONSE` that fills `CGUnit+0xb30`. +fn unqueried_wolf() -> UnitState { + UnitState { + exists: true, + has_object: true, + name: None, + health: 30, + max_health: 50, + level: 10, + reaction: 2, // hostile + skinnable: true, + guid: 0xF130_0000_0000_0001, + ..Default::default() + } +} + +/// **A name still in flight titles the plate `UNKNOWNOBJECT` — never an empty line** (decision +/// 2040, closing 2002's residue). +/// +/// The builder's name read is `CGUnit_C::GetUnitName 0x609210` (`0x52a187`), the same resolver +/// `UnitName 0x517020` delegates to, and every one of its misses — a null `CGUnit+0xb30` +/// (`0x609353 je 0x609324`) among them — ends at the same `FrameScript_GetText("UNKNOWNOBJECT")` +/// tail. So the verb and the plate answer the same string for the same instant, and because the +/// string is read out of the VM's `_G` it translates with `GlobalStrings.lua`. +/// +/// The builder has NO counterpart to `UnitName`'s two nils: the `"player"` fast path is the +/// *binding's* (`0x517083`, before any resolve), so a player whose name has not arrived titles +/// `UNKNOWNOBJECT` too; and a token resolving to nothing never reaches a builder at all — the +/// entry gate hands back no object, `SetUnit` answers nil and no plate is drawn. +/// +/// Failure looks like the empty first line the residue named: `TextLeft1` reads `""`, the plate +/// measures to a blank row, and the level line reads as the tooltip's title. +#[test] +fn a_pending_name_titles_unknownobject_and_the_answer_replaces_it() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.set_unit("target", Some(unqueried_wolf())); + s.set_player_req_state(PlayerReqState { + level: 12, + ..Default::default() + }); + s.run( + r#" + UNKNOWNOBJECT = "Unknown" -- GlobalStrings.lua:4444, enUS + local a = CreateFrame("Button", "UF1"); a:SetPoint("CENTER", 0, 0); a:SetSize(10, 10) + local tt = CreateFrame("GameTooltip", "TT") + tt:SetOwner(a, "ANCHOR_RIGHT") + assert(tt:SetUnit("target") == 1, "a resolved unit shows, name or no name") + assert(TTTextLeft1:GetText() == UNKNOWNOBJECT, + "the title is the GlobalString, got '" .. tostring(TTTextLeft1:GetText()) .. "'") + -- The record's other lines are absent with it: no subtitle, and the level line takes the + -- bare TOOLTIP_UNIT_LEVEL template because both the CLASS and TYPE slots are the record's. + assert(TTTextLeft2:GetText() == "Level 10", "got " .. TTTextLeft2:GetText()) + -- The descriptor's own lines are NOT the record's and show anyway. + assert(TTTextLeft3:GetText() == "Skinnable") + "#, + ) + .unwrap(); + + // The query answers: the very next render replaces the placeholder with the real name and + // fills the record's lines. (Live, the hover feed re-drives this without the mouse moving — + // its rebuild key is the whole line-affecting snapshot, `ui_tooltip::lines_view`.) + let mut answered = unqueried_wolf(); + answered.name = Some("Timber Wolf".into()); + answered.subtitle = Some("Alpha".into()); + answered.creature_type_name = Some("Beast".into()); + answered.rank = 2; + s.set_unit("target", Some(answered)); + s.run( + r#" + assert(TT:SetUnit("target") == 1) + assert(TTTextLeft1:GetText() == "Timber Wolf", "the answer replaced UNKNOWNOBJECT") + assert(TTTextLeft2:GetText() == "Alpha") + assert(TTTextLeft3:GetText() == "Level 10 Beast (Elite)", "got " .. TTTextLeft3:GetText()) + assert(TTTextLeft4:GetText() == "Skinnable") + "#, + ) + .unwrap(); + + // An EMPTY (or absent) global is the same miss as no global — `0x609324`'s own check, which + // falls to the binary's literal `0x860fa4`. + s.set_unit("target", Some(unqueried_wolf())); + s.run( + r#" + UNKNOWNOBJECT = "" + assert(TT:SetUnit("target") == 1) + assert(TTTextLeft1:GetText() == "Unknown Being", "got " .. TTTextLeft1:GetText()) + "#, + ) + .unwrap(); + + // The `"player"` fast path is the BINDING's, not the builder's: a player whose name-cache row + // has not answered titles the same placeholder, where `UnitName("player")` would push nil. + s.set_unit( + "player", + Some(UnitState { + exists: true, + has_object: true, + name: None, + level: 12, + is_player: true, + player_controlled: true, + guid: 0x0000_0000_0000_0007, + ..Default::default() + }), + ); + s.run( + r#" + UNKNOWNOBJECT = "Unknown" + assert(UnitName("player") == nil, "the binding's own fast path still pushes nil (2002)") + assert(TT:SetUnit("player") == 1) + assert(TTTextLeft1:GetText() == UNKNOWNOBJECT, + "the builder has no player fast path, got " .. tostring(TTTextLeft1:GetText())) + "#, + ) + .unwrap(); + + // The GUID-0 counterpart: a recognised token naming nothing resolves to no object, so no + // builder runs — nil, and no plate. (There is no third answer; the builder never sees it.) + s.run(r#"assert(TT:SetUnit("party4") == nil, "an absent unit draws no plate at all")"#) + .unwrap(); + assert!(s.take_errors().is_empty()); +} + /// The faction-name line sits between the level line and "PvP" (the builder-tail block the §2 /// order omitted — the director's Marshal McBride reference: Level, Stormwind, PvP); the /// CIVILIAN line is the dishonorable-kill warning, whole gate (`0x612550`): PvP bit + civilian diff --git a/crates/benilla-ui/src/script/tooltip_unit.rs b/crates/benilla-ui/src/script/tooltip_unit.rs index dfe8b841..f68c39d3 100644 --- a/crates/benilla-ui/src/script/tooltip_unit.rs +++ b/crates/benilla-ui/src/script/tooltip_unit.rs @@ -3,7 +3,9 @@ //! //! - NAME (gold — FrameXML recolors `TextLeft1` by reaction on `UPDATE_MOUSEOVER_UNIT`, exactly //! like the reference's `GameTooltip_UnitColor`; the guild line is likewise FrameXML's and -//! joins when guild data streams); +//! joins when guild data streams). A name still in flight reads `UNKNOWNOBJECT`, never an +//! empty line — the builder's name read is `0x609210`, the same resolver `UnitName` uses, and +//! its miss tail is the same string (decisions 2002/2040); //! - the creature SUBTITLE ("Stable Master") — white; //! - the LEVEL line, composed from three slots over the four `TOOLTIP_UNIT_LEVEL*` templates: //! level text (`"??"` for a world boss, a much-higher hostile, or level ≤ 0 — the hostile @@ -31,7 +33,7 @@ use super::tooltip::{append_line, clear_content, fire_cleared, show_or_hide_empt // The grey band + trivial/GREY check (`0x5f0700`, the CIVILIAN line's last gate: a green-or- // better con never warns of a dishonorable kill) and the "??" gate live in one shared home // (`unit.rs`), alongside `UnitLevel`'s −1 return and the `GetQuestGreenRange` binding. -use super::unit::{is_civilian_kill, level_reads_unknown}; +use super::unit::{is_civilian_kill, level_reads_unknown, unknownobject}; use super::{KindState, Model, UnitState}; use crate::layout::{Anchor, Point}; use crate::widget::FrameHandle; @@ -138,13 +140,23 @@ fn render_unit(lua: &Lua, this: &Table, token: &str) -> mlua::Result { show_or_hide_empty(lua, h); return Ok(false); }; - append_line( - lua, - this, - (u.name.clone().unwrap_or_default(), GOLD), - None, - false, - )?; + // The NAME line. A unit whose name query has not answered yet does NOT title an empty plate: + // the builder resolves the name through `CGUnit_C::GetUnitName 0x609210` (`0x52a187`), and + // every one of that function's misses — a creature whose `creaturecache.wdb` record + // (`CGUnit+0xb30`) is still null, a pet whose `petnamecache.wdb` row is absent or stale, a + // player row `namecache.wdb` has not answered — falls to the SAME + // `FrameScript_GetText("UNKNOWNOBJECT")` tail `UnitName` falls to. One seam, one resolver + // ([`unknownobject`], decisions 2002/2040), so the verb and the plate can never disagree. + // + // The builder has no counterpart to `UnitName`'s two nils: the `"player"` fast path is the + // *binding's* (`0x517083`, before any resolve), and a token resolving to GUID 0 never reaches + // a builder at all — the entry gate `0x468460(typemask 8)` hands back no object, `SetUnit` + // answers nil and no plate is drawn (the `unit` early-return above). + let title = match &u.name { + Some(n) => n.clone(), + None => unknownobject(lua)?.to_str()?.to_string(), + }; + append_line(lua, this, (title, GOLD), None, false)?; if let Some(sub) = &u.subtitle { append_line(lua, this, (sub.clone(), WHITE), None, false)?; } diff --git a/crates/benilla-ui/src/script/unit/bindings.rs b/crates/benilla-ui/src/script/unit/bindings.rs index 4072ab50..48d85621 100644 --- a/crates/benilla-ui/src/script/unit/bindings.rs +++ b/crates/benilla-ui/src/script/unit/bindings.rs @@ -1,13 +1,20 @@ //! The `Unit*` global registrations (see the parent module's doc for the seam and the return //! shapes): every binding reads the per-token [`UnitState`](super::UnitState) snapshot store //! through the parent's `with_unit`/`pick_unit_token` helpers. +//! +//! **Every predicate here returns through one function** — [`super::unit_predicate`] when it reads +//! a snapshot field, [`predicate`] when it computes its own bool. Neither ever hands mlua a Rust +//! `bool`: 1.12 pushes the number `1` or `nil` and has no boolean on its binding surface at all +//! (decisions 1830, 2043). A new predicate that open-codes `Value::Integer(1)`/`Value::Nil`, or +//! returns a `bool`, is the drift those two records exist to stop. use mlua::{Lua, Value}; +use super::super::binding_abi::predicate; use super::super::Model; use super::{ check_unit_token, classification_word, grey_band, level_reads_unknown, pick_unit_token, - with_unit, SelectionRequest, + unit_predicate, unknownobject, with_unit, SelectionRequest, }; /// The two class ids `GetComboPoints 0x51a190` accepts — the literals `4` and `0xb` it compares @@ -19,19 +26,6 @@ use super::{ const CLASS_ROGUE: u32 = 4; const CLASS_DRUID: u32 = 11; -/// `FrameScript_GetText("UNKNOWNOBJECT")` as the name resolvers call it (`0x517220`, `0x609324`): -/// the VM's own GlobalString — read out of `_G` exactly as `0x703bf0` reads it, so a translated -/// `GlobalStrings.lua` translates this too (enUS: `"Unknown"`, `GlobalStrings.lua:4444`) — and, -/// when the global is missing or empty, the binary's own literal `"Unknown Being"` (`0x860fa4`, -/// the bytes at that address in `WoW.exe`). Always a string: this is the "name not yet known" -/// state, and it is never nil. Decision 2002. -fn unknownobject(lua: &Lua) -> mlua::Result { - match lua.globals().get::("UNKNOWNOBJECT") { - Ok(Value::String(s)) if !s.as_bytes().is_empty() => Ok(Value::String(s)), - _ => Ok(Value::String(lua.create_string("Unknown Being")?)), - } -} - /// Register the `Unit*` globals reading the per-token snapshot store (the same style/place the /// object model and stdlib register their globals — bare globals on `_G`, matching the live API /// surface). @@ -41,7 +35,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitExists", lua.create_function(|lua, token: Option| { - with_unit(lua, &token, false, |u| u.exists) + unit_predicate(lua, &token, |u| u.exists) })?, )?; @@ -62,11 +56,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsVisible", lua.create_function(|lua, token: Option| { - Ok(if with_unit(lua, &token, false, |u| u.has_object)? { - Value::Integer(1) - } else { - Value::Nil - }) + unit_predicate(lua, &token, |u| u.has_object) })?, )?; @@ -115,7 +105,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { })?; // One value, and it is the NUMBER 1 or nil — never a boolean, the same shape the // rest of this family answers in. - Ok(if hit { Value::Integer(1) } else { Value::Nil }) + Ok(predicate(hit)) })?, )?; } @@ -157,11 +147,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { .as_ref() .and_then(|t| model.unit(t)) .map_or(0, |u| u.guid); - Ok(if by_flag || guid == model.party.leader_guid { - Value::Integer(1) - } else { - Value::Nil - }) + Ok(predicate(by_flag || guid == model.party.leader_guid)) })?, )?; @@ -213,7 +199,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { { Value::Nil } - Some(None) => unknownobject(lua)?, + Some(None) => Value::String(unknownobject(lua)?), }; Ok((name, Value::Nil)) })?, @@ -282,11 +268,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsCorpse", lua.create_function(|lua, token: Option| { - Ok(if with_unit(lua, &token, false, |u| u.corpse_object)? { - Value::Integer(1) - } else { - Value::Nil - }) + unit_predicate(lua, &token, |u| u.corpse_object) })?, )?; @@ -311,11 +293,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { r#"Usage: UnitCanAttack("unit", "otherUnit")"#, )?); let token = pick_unit_token(&a, &b); - Ok(if with_unit(lua, &token, false, |u| u.can_attack)? { - Value::Integer(1) - } else { - Value::Nil - }) + unit_predicate(lua, &token, |u| u.can_attack) })?, )?; @@ -339,7 +317,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { token, r#"Usage: UnitIsDead("unit")"#, )?); - with_unit(lua, &token, false, |u| u.dead) + unit_predicate(lua, &token, |u| u.dead) })?, )?; @@ -353,7 +331,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { token, r#"Usage: UnitIsGhost("unit")"#, )?); - with_unit(lua, &token, false, |u| u.ghost) + unit_predicate(lua, &token, |u| u.ghost) })?, )?; g.set( @@ -364,7 +342,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { token, r#"Usage: UnitIsDeadOrGhost("unit")"#, )?); - with_unit(lua, &token, false, |u| u.dead || u.ghost) + unit_predicate(lua, &token, |u| u.dead || u.ghost) })?, )?; @@ -422,11 +400,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { )?); let token = pick_unit_token(&a, &b); let r = with_unit(lua, &token, 0u8, |u| u.reaction)?; - Ok(if (1..=2).contains(&r) { - Value::Integer(1) - } else { - Value::Nil - }) + Ok(predicate((1..=2).contains(&r))) })?, )?; g.set( @@ -446,11 +420,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { )?); let token = pick_unit_token(&a, &b); let r = with_unit(lua, &token, 0u8, |u| u.reaction)?; - Ok(if r >= 5 { - Value::Integer(1) - } else { - Value::Nil - }) + Ok(predicate(r >= 5)) })?, )?; @@ -480,10 +450,10 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { )?); let model = lua.app_data_ref::().expect("model app_data"); let player_level = model.player_req.level; - Ok(match unit.and_then(|u| model.unit(&u)) { - Some(u) if super::is_civilian_kill(u, player_level) => Value::Integer(1), - _ => Value::Nil, - }) + Ok(predicate( + unit.and_then(|u| model.unit(&u)) + .is_some_and(|u| super::is_civilian_kill(u, player_level)), + )) })?, )?; // UnitPlayerControlled(unit) → 1 if a PLAYER is driving this unit, else nil. `UNIT_FIELD_FLAGS` @@ -495,20 +465,16 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { "UnitPlayerControlled", lua.create_function(|lua, unit: Option| { let model = lua.app_data_ref::().expect("model app_data"); - Ok(match unit.and_then(|u| model.unit(&u)) { - Some(u) if u.player_controlled => Value::Integer(1), - _ => Value::Nil, - }) + Ok(predicate( + unit.and_then(|u| model.unit(&u)) + .is_some_and(|u| u.player_controlled), + )) })?, )?; g.set( "UnitIsPlayer", lua.create_function(|lua, token: Option| { - Ok(if with_unit(lua, &token, false, |u| u.is_player)? { - Value::Integer(1) - } else { - Value::Nil - }) + unit_predicate(lua, &token, |u| u.is_player) })?, )?; @@ -542,11 +508,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { if !ua.exists || !ub.exists { return Ok(Value::Nil); } - if a == b || (ua.guid != 0 && ua.guid == ub.guid) { - Ok(Value::Integer(1)) - } else { - Ok(Value::Nil) - } + Ok(predicate(a == b || (ua.guid != 0 && ua.guid == ub.guid))) })?, )?; @@ -570,7 +532,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { "Usage: UnitAffectingCombat(\"unit\")", )?; let hot = with_unit(lua, &Some(token), false, |u| u.exists && u.in_combat)?; - Ok(if hot { Value::Integer(1) } else { Value::Nil }) + Ok(predicate(hot)) })?, )?; @@ -599,11 +561,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { "HasFullControl", lua.create_function(|lua, ()| { let model = lua.app_data_ref::().expect("model app_data"); - Ok(if model.player_control { - Value::Integer(1) - } else { - Value::Nil - }) + Ok(predicate(model.player_control)) })?, )?; @@ -642,7 +600,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { !model.party.members.is_empty() }; let hit = grouped && (in_group(u.guid) || in_group(u.owner)); - Ok(if hit { Value::Integer(1) } else { Value::Nil }) + Ok(predicate(hit)) })?, )?; } @@ -663,7 +621,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { .iter() .any(|m| m.guid != 0 && m.guid == u.guid) }); - Ok(if hit { Value::Integer(1) } else { Value::Nil }) + Ok(predicate(hit)) })?, )?; @@ -693,7 +651,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { .members .iter() .any(|m| m.guid != 0 && m.guid == u.guid))); - Ok(if hit { Value::Integer(1) } else { Value::Nil }) + Ok(predicate(hit)) })?, )?; @@ -720,7 +678,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { let ok = with_unit(lua, &token, false, |u| { u.exists && u.is_player && u.reaction >= 5 })?; - Ok(if ok { Value::Integer(1) } else { Value::Nil }) + Ok(predicate(ok)) })?, )?; @@ -746,8 +704,16 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { )?; // The party-frame status predicates (decision 0434 §2/§3): connection, AFK/DND, and the two PvP - // flags. All 1/nil, the live API's own shape for these — unlike UnitIsDead/UnitIsGhost above (a - // stated v1 shortcut, module doc), these are new and follow the era shape from the start. + // flags. All 1/nil, through the family's one push site (2043). + // + // **`UnitIsAFK`/`UnitIsDND` are OURS, not the reference's** — this comment used to claim they + // "follow the era shape from the start", which named the wrong authority. wow-re's raw byte + // census over the whole image finds ZERO occurrences of `UnitIsAFK`, `UnitIsDND`, `IsAFK` or + // `IsDND`, as bindings or as strings (positive control: `UnitIsPVP\0` and `UnitIsGhost\0` + // each return exactly 1) — `ui/scratch/nil-unit-token-arg-law.md` §11. 1.12 surfaces a + // member's AFK/DND state through `GetGuildRosterInfo` and the `CHAT_FLAG_AFK`/`CHAT_FLAG_DND` + // GlobalStrings; there is no unit predicate for it. They wear the family's shape because that + // is the right shape for an invention of ours to wear, not because a binding was read. g.set( "UnitIsConnected", lua.create_function(|lua, token: Value| { @@ -756,31 +722,19 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { token, r#"Usage: UnitIsConnected("unit")"#, )?); - Ok(if with_unit(lua, &token, false, |u| u.is_connected)? { - Value::Integer(1) - } else { - Value::Nil - }) + unit_predicate(lua, &token, |u| u.is_connected) })?, )?; g.set( "UnitIsAFK", lua.create_function(|lua, token: Option| { - Ok(if with_unit(lua, &token, false, |u| u.is_afk)? { - Value::Integer(1) - } else { - Value::Nil - }) + unit_predicate(lua, &token, |u| u.is_afk) })?, )?; g.set( "UnitIsDND", lua.create_function(|lua, token: Option| { - Ok(if with_unit(lua, &token, false, |u| u.is_dnd)? { - Value::Integer(1) - } else { - Value::Nil - }) + unit_predicate(lua, &token, |u| u.is_dnd) })?, )?; // UnitIsPVP reads the same `pvp` field the unit tooltip's "PvP" line already does (one flag, @@ -793,11 +747,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { token, r#"Usage: UnitIsPVP("unit")"#, )?); - Ok(if with_unit(lua, &token, false, |u| u.pvp)? { - Value::Integer(1) - } else { - Value::Nil - }) + unit_predicate(lua, &token, |u| u.pvp) })?, )?; g.set( @@ -808,11 +758,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { token, r#"Usage: UnitIsPVPFreeForAll("unit")"#, )?); - Ok(if with_unit(lua, &token, false, |u| u.is_pvp_ffa)? { - Value::Integer(1) - } else { - Value::Nil - }) + unit_predicate(lua, &token, |u| u.is_pvp_ffa) })?, )?; // UnitFactionGroup(unit) → (englishGroup, localizedName), the pair the PvP icon law reads: @@ -932,7 +878,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { r#"Usage: UnitHasRelicSlot("unit")"#, )?); let has = with_unit(lua, &token, false, |u| u.has_relic_slot)?; - Ok(if has { Value::Integer(1) } else { Value::Nil }) + Ok(predicate(has)) })?, )?; @@ -1137,13 +1083,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "UnitIsCharmed", lua.create_function(|lua, token: Option| { - with_unit(lua, &token, Value::Nil, |u| { - if u.charmed { - Value::Number(1.0) - } else { - Value::Nil - } - }) + unit_predicate(lua, &token, |u| u.charmed) })?, )?; @@ -1251,11 +1191,7 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { "IsResting", lua.create_function(|lua, ()| { let model = lua.app_data_ref::().expect("model app_data"); - Ok(if model.resting { - Value::Integer(1) - } else { - Value::Nil - }) + Ok(predicate(model.resting)) })?, )?; // PartialPlayTime() / NoPlayTime() → 1/nil: the two anti-addiction play-time regimes, read @@ -1268,22 +1204,14 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { "PartialPlayTime", lua.create_function(|lua, ()| { let model = lua.app_data_ref::().expect("model app_data"); - Ok(if model.partial_play_time { - Value::Integer(1) - } else { - Value::Nil - }) + Ok(predicate(model.partial_play_time)) })?, )?; g.set( "NoPlayTime", lua.create_function(|lua, ()| { let model = lua.app_data_ref::().expect("model app_data"); - Ok(if model.no_play_time { - Value::Integer(1) - } else { - Value::Nil - }) + Ok(predicate(model.no_play_time)) })?, )?; // GetBillingTimeRested() → the account's rested billing MINUTES, as one number, always @@ -1505,12 +1433,11 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { "ClearTarget", lua.create_function(|lua, ()| { let mut model = lua.app_data_mut::().expect("model app_data"); - if model.unit("target").is_some_and(|u| u.exists) { + let had = model.unit("target").is_some_and(|u| u.exists); + if had { model.target_clear = true; - Ok(Value::Integer(1)) - } else { - Ok(Value::Nil) } + Ok(predicate(had)) })?, )?; diff --git a/crates/benilla-ui/src/script/unit/mod.rs b/crates/benilla-ui/src/script/unit/mod.rs index 73692244..c2e677ac 100644 --- a/crates/benilla-ui/src/script/unit/mod.rs +++ b/crates/benilla-ui/src/script/unit/mod.rs @@ -7,16 +7,20 @@ //! `UnitHealth("player")` therefore resolves against a [`UnitState`] the Bevy side deposited that //! frame — the same shape a real addon sees, with none of the coupling. //! -//! Return shapes follow the live API (warcraft.wiki.gg): `UnitName` returns the name *or nil*; +//! Return shapes follow the reference: `UnitName` returns the name *or nil*; //! `UnitHealth`/`UnitHealthMax`/`UnitLevel` return numbers (`UnitLevel` returns **−1** for a //! unit whose level "can't be told" — [`level_reads_unknown`], the target frame's skull gate); -//! a unit that doesn't exist reports `UnitExists` false and the numbers `0` (nil for the name), +//! a unit that doesn't exist reports `UnitExists` nil and the numbers `0` (nil for the name), //! exactly as the client does for an absent unit token. Tokens (`"player"`, `"target"`, …) are opaque strings the host never //! interprets — the app decides what each maps to. //! +//! **Every predicate in this family answers the number `1` or `nil`, never a Lua boolean**, and it +//! answers it through the one helper — [`unit_predicate`], over +//! [`super::binding_abi::predicate`]. Decision 2043; the law is 1830's, which settled the same +//! question for the widget predicates a family earlier. +//! //! v1 gaps, stated not hidden: the snapshot carries only the *active* power slot (see the -//! `UnitMana` note below); `UnitIsDead` returns a Lua boolean rather than the client's `1`/nil -//! (truthy either way — the shape a caller branches on is identical); `UnitIsConnected`'s backing +//! `UnitMana` note below); `UnitIsConnected`'s backing //! field defaults `false`, and no app-side feed sets it `true` for live units yet (decision 0434 //! §2's party-frame predicates land the binding first, the feed follows — see the field doc). @@ -338,6 +342,34 @@ pub struct UnitState { pub in_combat: bool, } +/// `FrameScript_GetText("UNKNOWNOBJECT")` as the client's name resolvers call it — the VM's own +/// GlobalString, read out of `_G` exactly as `0x703bf0` reads it (so a translated +/// `GlobalStrings.lua` translates this too; enUS `"Unknown"`, `GlobalStrings.lua:4444`), falling +/// back to the binary's own literal `"Unknown Being"` (`0x860fa4`) when the global is missing or +/// empty. Always a string — this is the "name not yet known" state, never a nil. +/// +/// **ONE home, two callers**, the same reason [`is_civilian_kill`] is a function. `UnitName` +/// `0x517020` reaches it at `0x517220`, and `CGUnit_C::GetUnitName 0x609210` — which the UNIT +/// TOOLTIP builder `0x529fe0` calls for its title line (`0x52a187`) — reaches the *same* tail at +/// `0x609324`, from every one of its misses: +/// +/// ```text +/// 609353 je 0x609324 ; creature: [CGUnit+0xb30] (creaturecache.wdb) still null +/// 6092ce je 0x609324 ; pet: petnamecache.wdb row absent, or its stamp stale +/// 609265 je 0x609324 ; player: namecache.wdb has not answered +/// 609324 push 0x0 / or edx,-1 / mov ecx,0x850ed0 ("UNKNOWNOBJECT") / call 0x703bf0 +/// 60933c mov eax,0x860fa4 ; the global missing or empty -> "Unknown Being" +/// ``` +/// +/// One resolver, so the verb and the plate can never disagree about what a unit whose name is +/// still in flight is called (decisions 2002, 2040). +pub fn unknownobject(lua: &Lua) -> mlua::Result { + match lua.globals().get::("UNKNOWNOBJECT") { + Ok(mlua::Value::String(s)) if !s.as_bytes().is_empty() => Ok(s), + _ => lua.create_string("Unknown Being"), + } +} + /// The grey-band table `0x80ae98` (a byte-identical twin at `0x81dda8` drives the nameplate's /// grey level colour), indexed by `playerLevel / 5` and clamped to 12 past the table. ONE home /// for its three consumers: [`unit_is_grey`] (tooltip CIVILIAN gate + the plate's skull/grey @@ -691,6 +723,31 @@ fn with_unit( }) } +/// A unit **predicate** binding's whole body: resolve the token, map its snapshot to a bool, and +/// push the family's one return shape — the number `1` or `nil`, never a Lua boolean. +/// +/// [`with_unit`] answers whatever `f` answers, so a predicate written on it *directly* hands mlua a +/// Rust `bool`, and mlua pushes tag 1. That is the one shape 1.12 cannot produce: `UnitExists` +/// (`0x515fb0`) pushes `lua_pushnumber` (`0x6f3810`, tag 3, the double `1.0`) or `lua_pushnil` +/// (`0x6f37f0`, tag 0) and nothing else, and so does every other predicate in the family. So the +/// whole family goes through [`super::binding_abi::predicate`] — 1830's widget law, which is the +/// *same* law, finally applied to the unit surface (decision 2043). A predicate that hand-computes +/// its own bool calls that helper directly; one that reads a snapshot field calls this. Both end at +/// the one push site, which is what stops the family drifting apart again. +/// +/// **Not every unit binding is a predicate.** `UnitReaction` answers the reaction *number*, +/// `UnitLevel` a level (with `-1` for "can't be told"), `UnitPowerType` a power index whose miss is +/// the number `0` and never nil. Those keep their own shapes and must not be routed here. +fn unit_predicate( + lua: &Lua, + token: &Option, + f: impl FnOnce(&UnitState) -> bool, +) -> mlua::Result { + Ok(super::binding_abi::predicate(with_unit( + lua, token, false, f, + )?)) +} + /// The `Unit*`/`GetQuestGreenRange` Lua binding registrations — split from this module's /// state+laws half purely for size (same seam as the other script modules' install fns). mod bindings; diff --git a/crates/benilla-ui/src/script/unit/tests.rs b/crates/benilla-ui/src/script/unit/tests.rs index 00d1a285..2b712665 100644 --- a/crates/benilla-ui/src/script/unit/tests.rs +++ b/crates/benilla-ui/src/script/unit/tests.rs @@ -342,7 +342,7 @@ fn set_unit_is_read_by_the_bindings() { let mut s = UiScript::new().unwrap(); s.set_unit("player", Some(player())); - assert!(s.eval::(r#"return UnitExists("player")"#).unwrap()); + assert_eq!(s.eval::(r#"return UnitExists("player")"#).unwrap(), 1); assert_eq!( s.eval::(r#"return UnitName("player")"#).unwrap(), "Benilla" @@ -353,13 +353,19 @@ fn set_unit_is_read_by_the_bindings() { 100 ); assert_eq!(s.eval::(r#"return UnitLevel("player")"#).unwrap(), 12); - assert!(!s.eval::(r#"return UnitIsDead("player")"#).unwrap()); + assert!(s + .eval::(r#"return UnitIsDead("player") == nil"#) + .unwrap()); } #[test] fn absent_token_reports_not_existing_with_zero_numbers() { let s = UiScript::new().unwrap(); - assert!(!s.eval::(r#"return UnitExists("target")"#).unwrap()); + // **nil, not `false`** — the family's false leg (decision 2043). `== nil` is the comparison a + // boolean would invert, so it is the one worth asserting. + assert!(s + .eval::(r#"return UnitExists("target") == nil"#) + .unwrap()); assert_eq!(s.eval::(r#"return UnitHealth("target")"#).unwrap(), 0); assert_eq!( s.eval::(r#"return UnitHealthMax("target")"#).unwrap(), @@ -369,7 +375,9 @@ fn absent_token_reports_not_existing_with_zero_numbers() { assert!(s .eval::(r#"return UnitName("target") == nil"#) .unwrap()); - assert!(!s.eval::(r#"return UnitIsDead("target")"#).unwrap()); + assert!(s + .eval::(r#"return UnitIsDead("target") == nil"#) + .unwrap()); } #[test] @@ -387,8 +395,8 @@ fn a_dead_unit_reports_dead_and_zero_health() { ..Default::default() }), ); - assert!(s.eval::(r#"return UnitExists("target")"#).unwrap()); - assert!(s.eval::(r#"return UnitIsDead("target")"#).unwrap()); + assert_eq!(s.eval::(r#"return UnitExists("target")"#).unwrap(), 1); + assert_eq!(s.eval::(r#"return UnitIsDead("target")"#).unwrap(), 1); assert_eq!(s.eval::(r#"return UnitHealth("target")"#).unwrap(), 0); // Name unknown (no name-query yet) → `UNKNOWNOBJECT`, never nil: the unit RESOLVED, and the // reference pushes nil for a zero GUID only (decision 2002; the shape is pinned below). A bare @@ -534,9 +542,11 @@ fn unit_xp_reads_the_player_globals_only_for_the_player_token() { fn set_unit_none_removes_the_token() { let mut s = UiScript::new().unwrap(); s.set_unit("player", Some(player())); - assert!(s.eval::(r#"return UnitExists("player")"#).unwrap()); + assert_eq!(s.eval::(r#"return UnitExists("player")"#).unwrap(), 1); s.set_unit("player", None); - assert!(!s.eval::(r#"return UnitExists("player")"#).unwrap()); + assert!(s + .eval::(r#"return UnitExists("player") == nil"#) + .unwrap()); assert_eq!(s.eval::(r#"return UnitHealth("player")"#).unwrap(), 0); } @@ -945,9 +955,11 @@ fn a_unit_token_resolves_whatever_its_case() { .unwrap(), 12 ); - assert!(s - .eval::(&format!(r#"return UnitExists("{spelling}")"#)) - .unwrap()); + assert_eq!( + s.eval::(&format!(r#"return UnitExists("{spelling}")"#)) + .unwrap(), + 1 + ); } for spelling in ["target", "Target", "TARGET"] { assert_eq!( @@ -998,7 +1010,8 @@ fn seating_a_token_folds_its_key_too() { // …and clearing it through the other spelling really clears it, rather than leaving a shadow. s.set_unit("TARGET", None); assert!( - !s.eval::(r#"return UnitExists("target")"#).unwrap(), + s.eval::(r#"return UnitExists("target") == nil"#) + .unwrap(), "removal folds too — no shadowed entry survives" ); } @@ -1182,9 +1195,9 @@ fn unit_is_visible_is_object_presence_not_existence() { None, "no object -> nil, even though the token still exists" ); - assert!( - s.eval::(r#"return UnitExists("party1") and true or false"#) - .unwrap(), + assert_eq!( + s.eval::(r#"return UnitExists("party1")"#).unwrap(), + 1, "...and UnitExists must NOT have moved with it — the roster fallback is the difference" ); @@ -1525,3 +1538,365 @@ fn a_two_token_predicate_raises_on_either_nil_argument() { assert!(s.run(&format!("{none}()")).is_ok(), "{none} takes no token"); } } + +// ── The family's return SHAPE, pinned per binding ─────────────────────────────────────────────── + +/// **Every `Unit*` predicate answers the number `1` or `nil` — never a Lua boolean** (decision +/// 2043; the law is 1830's, made for the widget predicates a family earlier). +/// +/// The reference cannot produce a boolean here. Its pushes are `lua_pushnumber 0x6f3810` (tag 3, +/// the double `1.0`) and `lua_pushnil 0x6f37f0` (tag 0); `lua_pushboolean 0x6f39f0` has seven call +/// sites image-wide and not one is a registered binding body, so **tag 1 is never written** +/// (wow-re `ui/scratch/binding-shape-arity-law.md`, `ui/scratch/button-enabled-state.md`, restated +/// `ui.md` l.4700 and l.7348; the per-binding carves for `UnitExists 0x515fb0`, +/// `UnitIsVisible 0x516030`, `UnitIsTapped 0x519c90`, `UnitAffectingCombat 0x517e10`, +/// `UnitPlayerControlled 0x516410`, `UnitInRaid 0x516350`, `UnitIsCharmed 0x516cf0` and +/// `UnitHasRelicSlot 0x519e50` each state it outright). +/// +/// **Truthiness is exactly why this needs a test rather than a reading.** `if UnitIsDead(u)` and +/// `if not UnitIsDead(u)` are identical under either shape, and mlua's `bool` conversion maps +/// `Integer(1)` to `true` as happily as it maps `Boolean(true)` — so a test that reads the value +/// as a Rust `bool` passes under the bug. Only `type(...)`, `== 1` and `== nil` separate them, and +/// `== nil` is the one that *inverts*: a boolean `false` is not `nil`, so a caller comparing +/// against `nil` concludes the opposite of the truth. +/// +/// The table is the whole registered family, so a new predicate added outside the seam fails here +/// rather than shipping its own shape. +#[test] +fn every_unit_predicate_is_one_or_nil_and_never_a_boolean() { + // (binding, the call as an addon writes it, a UnitState that makes it TRUE) + let table: Vec<(&str, &str, UnitState)> = vec![ + ( + "UnitExists", + r#"UnitExists("target")"#, + UnitState { + exists: true, + ..Default::default() + }, + ), + ( + "UnitIsVisible", + r#"UnitIsVisible("target")"#, + UnitState { + exists: true, + has_object: true, + ..Default::default() + }, + ), + ( + "UnitIsDead", + r#"UnitIsDead("target")"#, + UnitState { + exists: true, + dead: true, + ..Default::default() + }, + ), + ( + "UnitIsGhost", + r#"UnitIsGhost("target")"#, + UnitState { + exists: true, + ghost: true, + ..Default::default() + }, + ), + ( + "UnitIsDeadOrGhost", + r#"UnitIsDeadOrGhost("target")"#, + UnitState { + exists: true, + dead: true, + ..Default::default() + }, + ), + ( + "UnitIsCorpse", + r#"UnitIsCorpse("target")"#, + UnitState { + exists: true, + corpse_object: true, + ..Default::default() + }, + ), + ( + "UnitIsPlayer", + r#"UnitIsPlayer("target")"#, + UnitState { + exists: true, + is_player: true, + ..Default::default() + }, + ), + ( + "UnitPlayerControlled", + r#"UnitPlayerControlled("target")"#, + UnitState { + exists: true, + player_controlled: true, + ..Default::default() + }, + ), + ( + "UnitIsCharmed", + r#"UnitIsCharmed("target")"#, + UnitState { + exists: true, + charmed: true, + ..Default::default() + }, + ), + ( + "UnitIsTapped", + r#"UnitIsTapped("target")"#, + UnitState { + exists: true, + tapped: true, + ..Default::default() + }, + ), + ( + "UnitIsTappedByPlayer", + r#"UnitIsTappedByPlayer("target")"#, + UnitState { + exists: true, + tapped_by_player: true, + ..Default::default() + }, + ), + ( + "UnitAffectingCombat", + r#"UnitAffectingCombat("target")"#, + UnitState { + exists: true, + in_combat: true, + ..Default::default() + }, + ), + ( + "UnitIsConnected", + r#"UnitIsConnected("target")"#, + UnitState { + exists: true, + is_connected: true, + ..Default::default() + }, + ), + ( + "UnitIsAFK", + r#"UnitIsAFK("target")"#, + UnitState { + exists: true, + is_afk: true, + ..Default::default() + }, + ), + ( + "UnitIsDND", + r#"UnitIsDND("target")"#, + UnitState { + exists: true, + is_dnd: true, + ..Default::default() + }, + ), + ( + "UnitIsPVP", + r#"UnitIsPVP("target")"#, + UnitState { + exists: true, + pvp: true, + ..Default::default() + }, + ), + ( + "UnitIsPVPFreeForAll", + r#"UnitIsPVPFreeForAll("target")"#, + UnitState { + exists: true, + is_pvp_ffa: true, + ..Default::default() + }, + ), + ( + "UnitHasRelicSlot", + r#"UnitHasRelicSlot("target")"#, + UnitState { + exists: true, + has_relic_slot: true, + ..Default::default() + }, + ), + ( + "UnitCanAttack", + r#"UnitCanAttack("player", "target")"#, + UnitState { + exists: true, + can_attack: true, + ..Default::default() + }, + ), + ( + "UnitIsEnemy", + r#"UnitIsEnemy("player", "target")"#, + UnitState { + exists: true, + reaction: 1, + ..Default::default() + }, + ), + ( + "UnitIsFriend", + r#"UnitIsFriend("player", "target")"#, + UnitState { + exists: true, + reaction: 5, + ..Default::default() + }, + ), + ( + "UnitCanCooperate", + r#"UnitCanCooperate("player", "target")"#, + UnitState { + exists: true, + is_player: true, + reaction: 5, + ..Default::default() + }, + ), + ( + "UnitIsUnit", + r#"UnitIsUnit("target", "target")"#, + UnitState { + exists: true, + guid: 42, + ..Default::default() + }, + ), + ]; + + for (name, call, live) in table { + // ---- the TRUE leg: the number 1, one value, and NOT a boolean ---- + let mut s = UiScript::new().unwrap(); + s.set_unit("player", Some(player())); + s.set_unit("target", Some(live)); + + assert_eq!( + s.eval::(&format!("return type({call})")).unwrap(), + "number", + "{name} pushes a boolean where the reference pushes tag 3 (lua_pushnumber)" + ); + assert_eq!( + s.eval::(&format!("return {call}")).unwrap(), + 1, + "{name}'s true leg is the number 1" + ); + assert!( + s.eval::(&format!("return {call} == 1")).unwrap(), + "{name}: the corpus idiom `== 1` must match on the true leg" + ); + assert!( + !s.eval::(&format!("return {call} == true")).unwrap(), + "{name}: `== true` must NEVER match — the reference cannot push tag 1" + ); + assert_eq!( + s.eval::(&format!("return select('#', {call})")) + .unwrap(), + 1, + "{name} returns exactly one value on the true leg" + ); + + // ---- the FALSE leg: nil, one value, and NOT `false` ---- + // The token is cleared rather than re-flagged, so this also covers the "no such unit" arm + // the reference shares with a false predicate. + let mut s = UiScript::new().unwrap(); + s.set_unit("player", Some(player())); + s.set_unit("target", None); + + assert!( + s.eval::(&format!("return {call} == nil")).unwrap(), + "{name}'s false leg is nil — a boolean `false` is not nil, and a caller comparing \ + against nil would read the opposite of the truth" + ); + assert!( + !s.eval::(&format!("return {call} == false")).unwrap(), + "{name}: `== false` must never match" + ); + assert_eq!( + s.eval::(&format!("return select('#', {call})")) + .unwrap(), + 1, + "{name} returns exactly one value on the false leg too — nil, never zero values" + ); + } +} + +/// The corpus idiom, spelled the way an addon spells it, on a live token and a dead one. +/// +/// This is the shape a `bool` breaks and truthiness hides: `if UnitExists("target") == 1` is +/// simply never true under a boolean, and `if UnitExists("target") == nil` is never true either — +/// the branch silently picks the wrong arm in both directions at once. +#[test] +fn the_equals_one_idiom_branches_correctly_on_a_live_and_a_dead_token() { + let mut s = UiScript::new().unwrap(); + s.set_unit("player", Some(player())); + s.set_unit("target", Some(player())); + + assert_eq!( + s.eval::( + r#"if UnitExists("target") == 1 then return "live" else return "dead" end"# + ) + .unwrap(), + "live" + ); + + s.set_unit("target", None); + assert_eq!( + s.eval::( + r#"if UnitExists("target") == 1 then return "live" else return "dead" end"# + ) + .unwrap(), + "dead" + ); + assert_eq!( + s.eval::( + r#"if UnitExists("target") == nil then return "gone" else return "here" end"# + ) + .unwrap(), + "gone", + "the `== nil` half — the comparison a boolean INVERTS" + ); +} + +/// `UnitOnTaxi` (`0x517a40`) is the family's one predicate registered outside `unit/`, over the +/// taxi module's own ride flag — which is how it stayed a `bool` when its 22 siblings were fixed. +/// Same shape, and the same `Usage:` raise its gate at `0x517a48` carries. +#[test] +fn unit_on_taxi_is_one_or_nil_and_raises_the_reference_usage() { + let mut s = UiScript::new().unwrap(); + s.set_unit("player", Some(player())); + + assert!(s + .eval::(r#"return UnitOnTaxi("player") == nil"#) + .unwrap()); + s.set_on_taxi(true); + assert_eq!( + s.eval::(r#"return type(UnitOnTaxi("player"))"#) + .unwrap(), + "number" + ); + assert_eq!(s.eval::(r#"return UnitOnTaxi("player")"#).unwrap(), 1); + assert!(!s + .eval::(r#"return UnitOnTaxi("player") == true"#) + .unwrap()); + // Any other token is nil, not false — only our own player is tracked. + assert!(s + .eval::(r#"return UnitOnTaxi("target") == nil"#) + .unwrap()); + + let err = s.eval::("return UnitOnTaxi()").unwrap_err(); + assert!( + err.to_string().contains(r#"Usage: UnitOnTaxi("unit")"#), + "the gate's own message, not mlua's type error: {err}" + ); +} diff --git a/crates/benilla-world/src/doodad_anim/mat_anim.rs b/crates/benilla-world/src/doodad_anim/mat_anim.rs index 3028b7e9..297ec129 100644 --- a/crates/benilla-world/src/doodad_anim/mat_anim.rs +++ b/crates/benilla-world/src/doodad_anim/mat_anim.rs @@ -363,12 +363,24 @@ pub fn register_uv( bevy::log::warn_once!("mat-anim table full — a UV-scroll batch stays at its seed"); return; }; - let Some(mat) = materials.get_mut(id) else { + // **Through the parked half, not `Assets::get_mut`** (decision 2038). A material this + // spawner just built is not in the store yet — `model_render::lazy` reserves its handle and + // parks the value until something view-visible binds it — so the store-only read returned + // `None` for *every* registration, freed the slot again and left the registry empty: no UV + // scroll and no animated tint anywhere in the world, silently, from the day deferral landed. + let Some(seed) = crate::model_render::lazy::with_material_mut(materials, id, |mat| { + let seed = [mat.extension.sun_scale.z, mat.extension.sun_scale.w]; + mat.extension.anim_slots.x = f32::from(slot); + seed + }) else { + // Neither half holds it: a dead or foreign handle, and nothing to animate. Loud, because + // the silent version of this branch is what cost three days of frozen water. + bevy::log::warn_once!( + "mat-anim: no material behind {id} — a UV-scroll batch stays at its seed" + ); table.free(slot); return; }; - let seed = [mat.extension.sun_scale.z, mat.extension.sun_scale.w]; - mat.extension.anim_slots.x = f32::from(slot); if let UvLoop::PerSeq { host, .. } = &anim { // The breadcrumb for the lane that has no other tell: a per-placement material is invisible // in every count (it is one more material, one more row), so "did the bubbles take the new @@ -431,10 +443,22 @@ pub(super) fn tick_anim_materials( bevy::platform::collections::HashSet>, >, ) { - if (uv_reg.0.is_empty() && tint_reg.0.is_empty()) - || crate::dev_state::deterministic_run() - || matanim_off(&real) - { + if uv_reg.0.is_empty() && tint_reg.0.is_empty() { + // **The tripwire for decision 2038's whole class.** [`AnimMatPart`] and the registration + // next to it share ONE predicate at the spawn site, so a world holding marked parts with + // an empty registry is never "a scene with no animated content" — it is this lane dead, + // which is exactly how every waterfall, canal and lava sheet in the game froze silently + // for three days. Free: the marker is archetype-filtered, so this is one iterator probe + // on a frame that was about to return anyway, and it says nothing in the ordinary empty + // case. + if parts.iter().next().is_some() { + bevy::log::warn_once!( + "mat-anim: parts are marked animated but NOTHING is registered — the UV/tint lane is dead" + ); + } + return; + } + if crate::dev_state::deterministic_run() || matanim_off(&real) { return; } drawn.clear(); @@ -459,7 +483,12 @@ pub(super) fn tick_anim_materials( // and its slot zeroes back to identity ([`crate::mat_anim_table`]'s free law). let gseq_now = f64::from(now); uv_reg.0.retain(|id, entry| { - if !materials.contains(*id) { + // **Alive means either half holds it** (decision 2038): a material still parked by + // `model_render::lazy` — built, handle reserved, nothing view-visible bound to it yet — + // is not in the store, and `Assets::contains` alone reads that as death. Evicting there + // would drop the entry a spawn just made and never rebuild it, because registration + // happens once, at spawn. + if !crate::model_render::lazy::holds(&materials, *id) { table.free(entry.slot); return false; } @@ -472,7 +501,7 @@ pub(super) fn tick_anim_materials( true }); tint_reg.0.retain(|id, entry| { - if !materials.contains(*id) { + if !crate::model_render::lazy::holds(&materials, *id) { table.free(entry.slot); return false; } @@ -589,19 +618,169 @@ pub fn register_tint( bevy::log::warn_once!("mat-anim table full — a tint batch stays at its seed"); return; }; - let Some(mat) = materials.get_mut(id) else { + // The parked half, exactly as [`register_uv`] reaches it (decision 2038). + let Some(seed) = crate::model_render::lazy::with_material_mut(materials, id, |mat| { + let seed = [ + mat.extension.tint.x, + mat.extension.tint.y, + mat.extension.tint.z, + ]; + mat.extension.anim_slots.y = f32::from(slot); + seed + }) else { + bevy::log::warn_once!("mat-anim: no material behind {id} — a tint batch stays at its seed"); table.free(slot); return; }; - let seed = [ - mat.extension.tint.x, - mat.extension.tint.y, - mat.extension.tint.z, - ]; - mat.extension.anim_slots.y = f32::from(slot); reg.0.insert(id, TintAnimEntry { anim, slot, seed }); } +/// **The animated-material probe** (`WOW_MATANIM_PROBE=`): once, `secs` after boot, print +/// one line per live [`AnimMatPart`] — the parts whose material can be a [`UvAnimMaterials`] / +/// [`TintAnimMaterials`] key — with everything that decides whether it actually moves on screen: +/// +/// ```text +/// matanim # mat [ FAR][ PARKED] anim_slots (1, 0, 0) vis Inherited drawn 1 +/// uv slot 1 seed (+0.0000,+0.0000) row (+0.0000,-0.2122) live (+0.0000,-0.2122) +/// shared period 9.967s loop (+0.0000,+0.0000) (+0.0000,-0.2542) (+0.0000,-0.5083) (+0.0000,-0.7542) +/// ``` +/// (one line per part; wrapped here) +/// +/// Four questions, one line each, because a frozen scroll can fail at any of them and the pixels +/// say the same thing every time: is the batch **registered** at all (a missing line is a batch +/// whose loop never reached the registry — an exhausted table, a period-0 constant, a lane that +/// forgot to register); does its **loop** actually move (the four quarter-phase samples — a +/// flat row here is an asset that scrolls nowhere, not a renderer fault); is the **table row** +/// live (the value the shader will read — flat while the loop moves means the tick's draw gate +/// never picked this material up, which is 1375's failure mode); and is the part **drawn** (the +/// gate's own input). +/// +/// It samples the loop itself rather than watching the row over time, so it is a **one-shot that +/// works in a deterministic capture** — where [`tick_anim_materials`] is skipped entirely and the +/// rows stay zero by design. In a live run the `row` column is the tick's real output and the two +/// halves can be compared directly. +#[allow(clippy::too_many_arguments)] +pub(super) fn matanim_probe( + time: Res