diff --git a/crates/benilla-adt/src/combined_alpha.rs b/crates/benilla-adt/src/combined_alpha.rs index 05316818..fa3e39e0 100644 --- a/crates/benilla-adt/src/combined_alpha.rs +++ b/crates/benilla-adt/src/combined_alpha.rs @@ -1,7 +1,7 @@ //! MCAL → one 64×64 RGBA alpha map (R/G/B = the opacity of texture layers 1/2/3; A = 255). Ported -//! from wow-adt's `CombinedAlphaMap` so the output is byte-identical: layers are ingested in MCLY -//! order (skipping the opaque base), each decoded into the next channel; encodings are 4-bit packed -//! (×16), 8-bit raw, or Blizzard RLE (token bit 7 = fill, else copy; low 7 bits = count). `fix_alpha` +//! from wow-adt's `CombinedAlphaMap`: layers are ingested in MCLY order (skipping the opaque base), +//! each decoded into the next channel; encodings are 4-bit packed (×17 — `n / 15`, what the +//! reference's RGBA4444 texel reads as; wow-adt's ×16 topped out at 240), 8-bit raw, or Blizzard RLE (token bit 7 = fill, else copy; low 7 bits = count). `fix_alpha` //! reconstructs a 64×64 plane from 63×63 source by duplicating the previous pixel at the last row/col. use crate::McnkChunk; @@ -76,10 +76,10 @@ impl CombinedAlphaMap { const PACKED: usize = W * W / 2; if offset + PACKED <= raw.len() { for &p in &raw[offset..offset + PACKED] { - if !self.set_next((p & 0x0F) * 16) { + if !self.set_next((p & 0x0F) * 17) { break; } - if !self.set_next(((p >> 4) & 0x0F) * 16) { + if !self.set_next(((p >> 4) & 0x0F) * 17) { break; } } @@ -171,3 +171,35 @@ impl CombinedAlphaMap { self.y = 0; } } + +#[cfg(test)] +mod tests { + use super::*; + + fn blank() -> CombinedAlphaMap { + CombinedAlphaMap { + map: vec![0; W * W * 4], + x: 0, + y: 0, + layer: 0, + has_big_alpha: false, + fix_alpha: true, + } + } + + /// The reference packs a 4-bit layer weight into an RGBA4444 texel (wow-re `terrain.md`, + /// the `alpha_texel_*` packers), and a 4-bit unorm channel reads as `n / 15` — so a fully + /// painted nibble is full coverage. `n × 16` stopped at 240: every "fully painted" road or rock + /// let 6% of the layer beneath show through. + #[test] + fn four_bit_alpha_reads_as_n_over_15() { + let mut m = blank(); + // Low nibble first: texel 0 = 0xF, texel 1 = 0x8. + m.ingest_small(&[0x8F; W * W / 2], 0); + assert_eq!(m.get(0, 0, 0), 255, "nibble 15 is full coverage"); + assert_eq!(m.get(1, 0, 0), 136, "nibble 8 is 8/15"); + let mut z = blank(); + z.ingest_small(&[0x00; W * W / 2], 0); + assert_eq!(z.get(0, 0, 0), 0); + } +} diff --git a/crates/benilla-app/src/char_select/mod.rs b/crates/benilla-app/src/char_select/mod.rs index 45210095..3fa0ee1e 100644 --- a/crates/benilla-app/src/char_select/mod.rs +++ b/crates/benilla-app/src/char_select/mod.rs @@ -154,11 +154,14 @@ impl Plugin for CharSelectPlugin { input::rotate_model, debug_select_dialog, dialog::drive_delete_dialog, + // Ahead of the dialog driver, so a refused delete is on screen the frame + // its answer lands rather than the next. + delete_result, // The reference's one `GlueDialog` (`crate::glue::dialog`), which on this - // screen carries the refused character login. Both glue screens run the - // same system over the same resource; what a press *means* is answered - // per-screen, and an `Error` — the only kind reachable here — needs no - // answer at all. + // screen carries the refused character login and the refused delete. Both + // glue screens run the same system over the same resource; what a press + // *means* is answered per-screen, and an `Error` — the only kind reachable + // here — needs no answer at all. crate::glue::dialog::drive_glue_dialog, // Before the list refresh, and before `select_input` reads a click that // landed on the panel rather than the screen (decision 1196). @@ -168,7 +171,6 @@ impl Plugin for CharSelectPlugin { refresh::refresh_list, refresh::refresh_banner_and_buttons, refresh::feed_glue_preview, - delete_result, debug_select_shot, ) .chain() @@ -723,19 +725,68 @@ fn back_on_logout( } } -/// Surface a refused delete (the roster refresh already reflects a success — the row vanishes). -/// A refusal is realistically unreachable on vmangos (any enumerated character deletes), so a log -/// line honest-flags it rather than growing an error dialog nothing can trigger. -fn delete_result(mut msgs: MessageReader) { +/// Surface a refused delete in the glue dialog; a success needs nothing here, since `net::io` +/// re-enumerates before the result and the row is already gone. +/// +/// **A refusal is reachable, and was silent.** vmangos refuses to delete a guild master +/// (`HandleCharDeleteOpcode`: `GetGuildByLeader` → `CHAR_DELETE_FAILED`), and this used to be a +/// `warn!` under a comment calling the refusal unreachable — so the player pressed Delete, typed +/// DELETE, and watched nothing happen. +/// +/// The reference's shape, read off `WoW.exe`: `SMSG_CHAR_DELETE 0x5b45c0` hands the result byte to +/// `0x5ab0e0`, which passes it **straight through** as the operation status +/// (`SetStatus(byte == 0x39, byte)`); `CGlueMgr::Update`'s `screenState == 6` arm (`0x46c14e`) +/// then, on a completed-and-failed operation, fires `OPEN_STATUS_DIALOG("OKAY", text)` +/// (`0x46c177` pushes `"OKAY"` `0x837084`) with the status text the key table `0x85cae8` names — +/// the one-button dialog [`crate::glue::dialog::GlueDialog::open_error`] is. +fn delete_result( + mut msgs: MessageReader, + mut dialog: ResMut, + strings: Option>, +) { + let empty = GlueStrings::default(); + let strings = strings.as_deref().unwrap_or(&empty); for msg in msgs.read() { - if msg.action == CharAction::Delete - && msg.code != benilla_protocol::messages::CHAR_DELETE_SUCCESS - { - warn!("char select: delete refused (code {:#04x})", msg.code); + if msg.action != CharAction::Delete { + continue; + } + if let Some(text) = char_delete_refusal_text(strings, msg.code) { + info!( + "char select: delete refused (code {:#04x}) — {text}", + msg.code + ); + dialog.open_error(text); } } } +/// A `SMSG_CHAR_DELETE` result byte → the sentence the refusal dialog says, or `None` for the +/// success (`0x39`), which the refreshed roster says instead. +/// +/// The keys are the `CHAR_DELETE_*` block of the reference's status-key table `0x85cae8` +/// (`0x38..=0x3b`, wow-5875-re `system/glue/scratch/login-failure-dialogs.md` §3.1), numbered +/// exactly as vmangos's `ResponseCodes` (`SharedDefines.h`). vmangos sends only two of them: +/// `0x39` on success and `0x3a` for a guild master; its three other refusals (a character still +/// loaded, one not found, one on another account) send nothing at all. +/// +/// **The default arm is benilla's, not the reference's.** The reference indexes the whole 83-row +/// table with the byte, so an out-of-block byte would read some other family's sentence (and one +/// past `0x52` would read `"(%i)"`); that table is not transcribed here, and no server we speak to +/// sends such a byte, so anything unnamed says the block's own generic failure. +pub(crate) fn char_delete_refusal_text(strings: &GlueStrings, code: u8) -> Option<&str> { + let (key, fallback): (&str, &str) = match code { + benilla_protocol::messages::CHAR_DELETE_SUCCESS => return None, + 0x38 => ("CHAR_DELETE_IN_PROGRESS", "Deleting character"), + 0x3B => ( + "CHAR_DELETE_FAILED_LOCKED_FOR_TRANSFER", + "Your character is currently locked as part of the paid character transfer process.", + ), + // `0x3a` — the guild-master refusal — and the default arm above. + _ => ("CHAR_DELETE_FAILED", "Character deletion failed"), + }; + Some(strings.text(key, fallback)) +} + /// Glue-flow smoke (`WOW_GLUE_ROUNDTRIP=1`, decision 0423): once a real roster is up, bounce /// CharSelect → CharCreate → **Back** → CharSelect and exit — so the return-to-select rebuild is /// provable headlessly from the logs. Runs ungated (it crosses states); inert without the env. @@ -1388,6 +1439,73 @@ mod tests { } } + /// **A refused delete is said, not swallowed.** vmangos refuses to delete a guild master with + /// `CHAR_DELETE_FAILED` (0x3a); the screen used to `warn!` into the log and show nothing, so + /// the confirmed delete just silently did not happen. The reference raises its one-button + /// glue dialog with the `CHAR_DELETE_*` status text (`CGlueMgr::Update`'s `screenState == 6` + /// arm). A success raises nothing — the refreshed roster is the answer. + #[test] + fn a_refused_delete_raises_the_glue_dialog() { + use crate::glue::dialog::{DialogKind, GlueDialog}; + let mut app = App::new(); + app.add_plugins(MinimalPlugins) + .add_message::() + .init_resource::() + .add_systems(Update, delete_result); + + app.world_mut().write_message(CharActionResultMessage { + action: CharAction::Delete, + code: benilla_protocol::messages::CHAR_DELETE_SUCCESS, + }); + app.update(); + assert!( + !app.world().resource::().is_open(), + "a success is answered by the roster, not a dialog" + ); + + app.world_mut().write_message(CharActionResultMessage { + action: CharAction::Delete, + code: 0x3A, + }); + app.update(); + let dialog = app.world().resource::(); + assert_eq!(dialog.kind, Some(DialogKind::Error), "the OKAY dialog"); + // No GlueStrings in this App: the fallback literal. The shipped sentence is asserted + // against the real chain below. + assert_eq!(dialog.text, "Character deletion failed"); + } + + /// The delete refusals resolve to the sentences 1.12 ships, off the player's own chain — and + /// every key named is a REAL key, so a matching fallback cannot hide a missing one. Skips + /// without client data. + #[test] + fn every_delete_result_resolves_in_the_real_glue_strings() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let strings = crate::glue_strings::table_from_chain(&mut chain); + + assert_eq!(char_delete_refusal_text(&strings, 0x39), None); + assert_eq!( + char_delete_refusal_text(&strings, 0x3A), + Some("Character deletion failed") + ); + assert_eq!( + char_delete_refusal_text(&strings, 0x3B), + Some( + "Your character is currently locked as part of the paid character transfer \ + process." + ) + ); + let map = strings.into_map(); + for key in [ + "CHAR_DELETE_IN_PROGRESS", + "CHAR_DELETE_FAILED", + "CHAR_DELETE_FAILED_LOCKED_FOR_TRANSFER", + ] { + assert!(map.contains_key(key), "{key} is not in the shipped table"); + } + } + /// B119 — a created character is selected against the roster **already in hand**. `net::io` /// re-enumerates and emits the fresh roster BEFORE the create result, so by the time /// `note_created` runs the row exists and the roster message is spent; a version that only armed diff --git a/crates/benilla-app/src/death/mod.rs b/crates/benilla-app/src/death/mod.rs index 557aa6bb..68720661 100644 --- a/crates/benilla-app/src/death/mod.rs +++ b/crates/benilla-app/src/death/mod.rs @@ -65,6 +65,12 @@ pub(crate) struct DeathNet { pub(crate) corpse: Option, /// A pending resurrection offer; cleared when answered or when the popup times out. pub(crate) resurrect: Option, + /// Bumped on every `SMSG_RESURRECT_REQUEST` — the offer announces per MESSAGE (wow-re + /// death-ui.md §8: handler `0x5e7bc0` signals per arrival through `0x5ded50`), not per + /// `Some`-edge. `resurrect` is only cleared by an Accept/Decline, and the stock UI can hide + /// the popup without either (PLAYER_UNGHOST's `StaticPopup_Hide` runs no `OnCancel`), so an + /// edge latch there swallowed every later offer for the session. + pub(crate) resurrect_generation: u32, /// Our streamed corpse OBJECT's guid (a TYPEID_CORPSE create whose `CORPSE_FIELD_OWNER` is /// us) — what `CMSG_RECLAIM_CORPSE` carries, like the real client. `None` until the corpse /// streams into range (vmangos ignores the guid's content, so a 0 send still reclaims — @@ -141,9 +147,9 @@ struct DeathAnnounced { /// range events through the latch (the client's own re-announce, death-ui.md §4), so the /// RECOVER_CORPSE popup re-shows with the new delay. reclaim_generation: u32, - /// The pending resurrect offer has been announced to the UI (`RESURRECT_REQUEST` fired) — - /// held back while a player-caster's name is still resolving through the name cache. - offer_announced: bool, + /// The [`DeathNet::resurrect_generation`] this VM was last told about (`RESURRECT_REQUEST` + /// fired) — held back while a player-caster's name is still resolving through the name cache. + offer_generation: u32, /// The [`DeathNet::confirm_generation`] this latch last announced — a fresh /// `SMSG_SPIRIT_HEALER_CONFIRM` re-fires `CONFIRM_XP_LOSS` through the latch (decision 1068; /// the reclaim-latch pattern above). The old Some-edge latch here was B80's deadlock: it @@ -324,22 +330,16 @@ fn feed_death( } // ── The offer/confirm announcements (edge-fired, name-gated) ─────────────────────────────── - match &death_net.resurrect { - Some(offer) if !memo.offer_announced => { - // A player caster's wire name is empty — resolve through the ask-once name cache and - // hold the popup until it lands (the ref popup formats "%s wants to resurrect you"). - let name = if offer.name.is_empty() { - names.resolve(offer.caster, &net).map(str::to_owned) - } else { - Some(offer.name.clone()) - }; - if let Some(name) = name { - memo.offer_announced = true; - script.fire_event("RESURRECT_REQUEST", vec![ScriptValue::Str(name)]); - } + if let Some(name) = offer_announcement(memo, &death_net, dead || ghost, |offer| { + // A player caster's wire name is empty — resolve through the ask-once name cache and + // hold the popup until it lands (the ref popup formats "%s wants to resurrect you"). + if offer.name.is_empty() { + names.resolve(offer.caster, &net).map(str::to_owned) + } else { + Some(offer.name.clone()) } - None => memo.offer_announced = false, - _ => {} + }) { + script.fire_event("RESURRECT_REQUEST", vec![ScriptValue::Str(name)]); } // The confirm is message-fired, not state-edged (decision 1068): each SMSG bumps the // generation, so asking the healer again after a Cancel brings the dialog back. @@ -738,11 +738,86 @@ impl Plugin for DeathPlugin { } } +/// The name `RESURRECT_REQUEST` should fire with this frame, if it should — and the latch +/// advanced when it does. +/// +/// Per message, gated on the dead predicate: `0x5ded50` fires only while `0x605f30` holds (health +/// ≤ 0 or the ghost flag — `dead_or_ghost`), so a stale offer never reaches a living player's UI, +/// even through a fresh VM after a `/reload`. A caster whose name is still resolving holds the +/// announcement without consuming it. +fn offer_announcement( + memo: &mut DeathAnnounced, + death_net: &DeathNet, + dead_or_ghost: bool, + name_of: impl FnOnce(&ResurrectOffer) -> Option, +) -> Option { + if !dead_or_ghost || memo.offer_generation == death_net.resurrect_generation { + return None; + } + let name = name_of(death_net.resurrect.as_ref()?)?; + memo.offer_generation = death_net.resurrect_generation; + Some(name) +} + #[cfg(test)] mod tests { use super::*; use bevy::ecs::system::RunSystemOnce; + fn offer(caster: u64, name: &str) -> ResurrectOffer { + ResurrectOffer { + caster, + name: name.into(), + sickness: false, + has_timer: true, + } + } + + /// **An offer the UI hid without answering must not swallow the next one.** PLAYER_UNGHOST + /// hides the RESURRECT popups with `StaticPopup_Hide`, which runs no `OnCancel`, so no + /// Decline clears `resurrect`; the next death's `SMSG_RESURRECT_REQUEST` must still announce. + /// And a stale offer must never reach the UI of a living player (a `/reload` after the res). + #[test] + fn every_resurrect_message_announces_while_dead_and_never_while_alive() { + let mut net = DeathNet::default(); + let mut memo = DeathAnnounced::default(); + let name = |o: &ResurrectOffer| Some(o.name.clone()); + + net.resurrect = Some(offer(1, "Pone")); + net.resurrect_generation += 1; + assert_eq!( + offer_announcement(&mut memo, &net, true, name).as_deref(), + Some("Pone") + ); + assert_eq!( + offer_announcement(&mut memo, &net, true, name), + None, + "once per message" + ); + + // Ran back instead: alive, the popup hidden, the offer never answered. A /reload now + // (a fresh VM memo) must not show "Pone wants to resurrect you" to a living player. + let mut reloaded = DeathAnnounced::default(); + assert_eq!(offer_announcement(&mut reloaded, &net, false, name), None); + + // Dead again, a new priest offers. + net.resurrect = Some(offer(2, "Ptwo")); + net.resurrect_generation += 1; + assert_eq!( + offer_announcement(&mut memo, &net, true, name).as_deref(), + Some("Ptwo"), + "the second offer announces" + ); + + // A name still resolving holds the announcement without consuming it. + net.resurrect_generation += 1; + assert_eq!(offer_announcement(&mut memo, &net, true, |_| None), None); + assert_eq!( + offer_announcement(&mut memo, &net, true, name).as_deref(), + Some("Ptwo") + ); + } + /// **The world scope dies with the session, and only with the session** (decision 1732). /// /// Both halves matter and they pull opposite ways: a `/reload` must NOT restart the six diff --git a/crates/benilla-app/src/death/net.rs b/crates/benilla-app/src/death/net.rs index d35f2636..6a37c4dd 100644 --- a/crates/benilla-app/src/death/net.rs +++ b/crates/benilla-app/src/death/net.rs @@ -212,6 +212,7 @@ fn resurrect_request( sickness, has_timer, }); + death_net.resurrect_generation = death_net.resurrect_generation.wrapping_add(1); } /// `SMSG_SPIRIT_HEALER_CONFIRM` — the healer awaiting the XP-loss two-step's Accept. The message diff --git a/crates/benilla-app/src/debug_panel/inspect.rs b/crates/benilla-app/src/debug_panel/inspect.rs index 41a3b096..7afb780c 100644 --- a/crates/benilla-app/src/debug_panel/inspect.rs +++ b/crates/benilla-app/src/debug_panel/inspect.rs @@ -353,7 +353,7 @@ pub(super) fn inspect_ui( // (the same ask-once path the unit frames use). let name_line = net_entity .and_then(|p| guids.get(p).ok()) - .and_then(|g| names.resolve(g.0, &net_commands)) + .and_then(|g| names.resolve_unit(g.0, store, &net_commands)) .map(str::to_string); // Line gates go by the entity's KIND, not field presence: a create-seeded store answers every // field (absent = 0, the descriptor truth), so "is the health field there" stopped meaning diff --git a/crates/benilla-app/src/entities/spell_fx/mod.rs b/crates/benilla-app/src/entities/spell_fx/mod.rs index bb3ecceb..7cbe2083 100644 --- a/crates/benilla-app/src/entities/spell_fx/mod.rs +++ b/crates/benilla-app/src/entities/spell_fx/mod.rs @@ -972,6 +972,8 @@ pub(super) fn attach_spell_fx( // pose (`RigPose::anchor_for`, decision 1355). Option<&mut benilla_world::rig_anim::RigPose>, &GlobalTransform, + // The body is built (model or fallback cube) — see the wait below. + Has, )>, fx: Option>, asset_server: Res, @@ -989,7 +991,7 @@ pub(super) fn attach_spell_fx( return; }; let now = time.elapsed_secs(); - for (unit, mut att, bones, mut pose, unit_gt) in &mut units { + for (unit, mut att, bones, mut pose, unit_gt, body_built) in &mut units { att.instances.retain_mut(|inst| { // Self-termination (the cast-release flash ran its span). if let Some(expires) = inst.expires { @@ -1070,6 +1072,18 @@ pub(super) fn attach_spell_fx( // A root-aura spell's plant additionally re-plants on owner displacement // ([`tend_world_plants`] — the client's 0x4000 flag leg). let planted = inst.tag == benilla_formats::WORLD_EFFECT_TAG; + // **A bone-riding effect waits for the body.** The cascade below reads the unit's + // attach points, and before the body is built (a display swap still loading the new + // model, a player still settling its equipment) there are none — so it fell through + // to the unit root, ground-anchored, and a persistent instance is never re-seated + // after that: an Ice Barrier or a Power Word: Shield sat decaled at the feet for the + // aura's whole life. The reference re-arms the persistent kits after the rebuild has + // set the new model (`0x60abe0` → `0x5ff130`, wow-re `shapeshift-morph-cloud.md`); + // `VisualAttached` is that "the model is set" — the cube fallback carries it too, so + // a unit whose model never loads still gets its effect at the root. + if !planted && !body_built { + return true; + } // Ground-anchored: a world plant sits at the feet by construction; otherwise the // cascade landed on the model's BASE point (`0x13`) or fell through to the unit // root — the feet-level anchors. A hand/head/chest-anchored instance keeps its flat @@ -1327,6 +1341,38 @@ mod tests { ); } + /// **The re-create waits for the new body.** A display swap tears the visual down and the + /// new model may still be loading when the effect model is already cached: re-arming then + /// found no attach points and parked a chest shield at the feet, ground-decaled, for the + /// aura's whole life (nothing re-seats a live root). It waits until the body is built. + #[test] + fn a_re_armed_instance_waits_for_the_body_before_it_spawns() { + let (mut app, unit, roots) = standing(&[true]); + app.world_mut().entity_mut(roots[0]).despawn(); + app.world_mut().resource_mut::().models.insert( + "Spells\\IceShield_State.mdx".into(), + DisplayModel { + parts: Some(Vec::new()), + ..crate::entities::display::empty_shell() + }, + ); + app.update(); + app.update(); + assert_eq!( + instances_of(&app, unit), + vec![(true, None)], + "no body yet: the instance holds, it does not spawn at the root" + ); + app.world_mut() + .entity_mut(unit) + .insert(crate::entities::VisualAttached); + app.update(); + assert!( + matches!(instances_of(&app, unit)[..], [(true, Some(_))]), + "the body is built: the instance spawns" + ); + } + /// A decaying persistent instance was already ending — the drain takes it too (a re-spawn /// would replay its death rattle from nothing on the new body). #[test] diff --git a/crates/benilla-app/src/game_plugins.rs b/crates/benilla-app/src/game_plugins.rs index 25665a78..acaceeb9 100644 --- a/crates/benilla-app/src/game_plugins.rs +++ b/crates/benilla-app/src/game_plugins.rs @@ -1019,7 +1019,9 @@ pub(crate) mod schedule_tests { /// exclusive system on one side were read as "explained" by a class argued for the build's /// barriers, which the declared graph (2337) no longer has; 413 are `finish_colliders`', 143 /// the net drain's. Same tree, the class dropped, nothing else moved: 5,052 + 556. - const UPDATE_ACTIONABLE_CEILING: usize = 5_608; + /// **5,554 (decision 2345)** — lowered: the TAB scan stopped reading `Visibility` (the draw + /// election's verdict), which ended its pairs with every `Visibility` writer. + const UPDATE_ACTIONABLE_CEILING: usize = 5_554; const UPDATE_ACTIONABLE_SLACK: usize = 40; fn ratchet(what: &str, n: usize, ceiling: usize, slack: usize) { diff --git a/crates/benilla-app/src/go_anim.rs b/crates/benilla-app/src/go_anim.rs index d1e92739..15424ae1 100644 --- a/crates/benilla-app/src/go_anim.rs +++ b/crates/benilla-app/src/go_anim.rs @@ -74,7 +74,6 @@ use std::time::Duration; use crate::creature_anim::{advance_track, scan_events, AnimSoundEvent}; use crate::net::{GuidIndex, ObjectStore}; -use benilla_world::model_fade::DespawnFade; use benilla_world::schedule::WorldStage; /// `GO_STATE_ACTIVE` (vmangos `GOState`) — the **open** state (door swung, chest lid up). Passable. @@ -597,8 +596,11 @@ fn arm_despawn_anim(mut gos: Query<&mut GoAnim, Changed>) /// scheduler `0x672df0` on its way out — so the object stops existing while its model keeps /// drawing and ramps to zero. A looted chest whose static model authors no `Despawn` sequence is /// exactly this path with no animation in front of it, and popping it was the report that found -/// the missing hop. [`DespawnFade`] is that hand-off; it arms once and drives itself, so the pin -/// comes off with it and this query stops matching. +/// the missing hop. [`DespawnFade`](benilla_world::model_fade::DespawnFade) is that hand-off; it +/// arms once and drives itself, so the pin comes off with it and this query stops matching. The +/// hand-off is [`crate::net::tear_down`], the same one both wire routes take, so the object's +/// identity ends here too — a pinned object is still an object while it plays, and stops being +/// one when the pin drops. fn release_despawn_pin( mut commands: Commands, pinned: Query<(Entity, Option<&GoAnim>), With>, @@ -610,7 +612,7 @@ fn release_despawn_pin( commands .entity(e) .try_remove::() - .try_insert(DespawnFade::default()); + .queue_silenced(crate::net::tear_down); } } @@ -1132,6 +1134,7 @@ pub(crate) fn plugin(app: &mut App) { #[cfg(test)] mod tests { use super::*; + use benilla_world::model_fade::DespawnFade; /// **A ghost's mover drops DOOR GameObjects — and nothing else** (decision 1767). /// @@ -1677,9 +1680,11 @@ mod tests { assert_eq!(armed(&app, go), Some((0x93, RepeatAnimation::Never))); // The wire pair, in the order vmangos sends it and Commands apply it. - app.world_mut() - .entity_mut(go) - .insert((DespawnAnimAnnounced, PendingDestroy)); + app.world_mut().entity_mut(go).insert(( + crate::net::Guid(0xF110_0000_0000_0E66), + DespawnAnimAnnounced, + PendingDestroy, + )); app.update(); assert_eq!( armed(&app, go), @@ -1687,8 +1692,8 @@ mod tests { "the announcement arms 157 Despawn for ONE window" ); assert!( - app.world().get_entity(go).is_ok(), - "the pin holds the object alive across its own destroy" + app.world().get::(go).is_some(), + "the pin holds the OBJECT alive across its own destroy, not just its model" ); advance(&mut app, 2700); @@ -1697,6 +1702,10 @@ mod tests { app.world().get::(go).is_some(), "the window ended — the pin drops and the deferred destroy hands the model to the fade" ); + assert!( + app.world().get::(go).is_none(), + "…and the deferred destroy is the teardown: the object ends as the fade begins" + ); assert!( app.world().get::(go).is_none(), "and the pin comes off with it, so the release stops re-arming the fade" diff --git a/crates/benilla-app/src/nameplates.rs b/crates/benilla-app/src/nameplates.rs index 30227b49..5df73dbf 100644 --- a/crates/benilla-app/src/nameplates.rs +++ b/crates/benilla-app/src/nameplates.rs @@ -556,10 +556,10 @@ pub(crate) fn drive_nameplates( // resolve's `&mut`-tied return across the subname read below is a borrow conflict, and the // `str::to_owned` that used to break it was a per-unit-per-frame allocation made just to // compare against the cache (the steady arm below no longer builds anything). - if names.resolve(guid.0, &net_commands).is_none() { + if names.resolve_unit(guid.0, store, &net_commands).is_none() { continue; } - let Some(name) = names.peek(guid.0) else { + let Some(name) = names.peek_unit(guid.0, store) else { continue; }; // The player flag decorations (a1–a3): glued straight onto the name line, no space. diff --git a/crates/benilla-app/src/names.rs b/crates/benilla-app/src/names.rs index cb3c23fa..f880c0b5 100644 --- a/crates/benilla-app/src/names.rs +++ b/crates/benilla-app/src/names.rs @@ -1,11 +1,13 @@ //! Unit-name resolution — the query-cache seam of decision 0068 §3. //! //! The 1.12 wire carries **no names in descriptors**: a player's name answers `CMSG_NAME_QUERY` -//! (keyed by guid), a creature's answers `CMSG_CREATURE_QUERY` (keyed by the template *entry* -//! embedded in its guid, shared by every spawn of that template — exactly how the real client -//! recovers it). This module owns the cache and the **ask-once** discipline: a consumer calls -//! [`NameCache::resolve`], which returns the name when known and otherwise issues the query (deduped -//! while in flight) and reports "not yet". The net bridge ([`crate::net`]) fills the cache from the +//! (keyed by guid), a creature's answers `CMSG_CREATURE_QUERY` (keyed by the template *entry*, +//! shared by every spawn of that template), a pet's or charm's answers `CMSG_PET_NAME_QUERY` +//! (keyed by its pet number). Which of the three names a streamed unit is read off its +//! **descriptor**, as the reference's `GetUnitName` (`0x609210`) reads it — see [`NameKey`]. This +//! module owns the cache and the **ask-once** discipline: a consumer calls +//! [`NameCache::resolve_unit`] (or [`NameCache::resolve`] with only a guid), which returns the name +//! when known and otherwise issues the query (deduped while in flight) and reports "not yet". The net bridge ([`crate::net`]) fills the cache from the //! decoded `PlayerName`/`CreatureName` events and clears the in-flight sets on disconnect (a query //! dropped by a dead writer must be re-askable after reconnect). //! @@ -171,29 +173,96 @@ pub(crate) fn gated_rank(rec: Option<&CreatureRecord>, store: Option<&ObjectStor } } -impl NameCache { - /// The name for `guid`, if known. On a miss, sends the right query (once per guid/entry per - /// connection) and returns `None` — call again after the answer lands. A guid family that has no - /// name on the 1.12 wire (GameObjects resolve via their own query, not modeled yet) is `None` - /// without a query. - pub(crate) fn resolve(&self, guid_val: u64, commands: &NetCommands) -> Option<&str> { +/// Which of the three caches names a unit, and under which key — the branch of the reference's +/// `CGUnit_C::GetUnitName` (`0x609210`), which is also what the combat log's `GetObjectName` +/// (`0x6264e0`) calls for any streamed unit. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum NameKey { + /// A player — the `OBJECT_FIELD_TYPE` player bit (`0x609232 shr ecx,4; test cl,1`): the + /// name cache by guid. + Player(u64), + /// `UNIT_FIELD_PETNUMBER` non-zero (`0x609295`): the pet-name cache under that number. + Pet(u32), + /// `UNIT_FIELD_PETNUMBER` zero (`0x60929d je 0x60934b`): the creature record at `[unit+0xb30]`, + /// which is registered under the descriptor's `OBJECT_FIELD_ENTRY` (`0x60b160`). + Creature(u32), +} + +impl NameKey { + /// The key for `guid`, read off its descriptor when one is in hand. + /// + /// **With a descriptor this is `0x609210` exactly**: the pet number and the template entry are + /// the unit's *fields*, never the guid's bits. The two disagree more often than they agree + /// outside a hunter's or warlock's own pet — vmangos gives a companion pet a `HIGHGUID_PET` + /// guid around a fresh pet number but never files it in the charm info (so `PETNUMBER` is 0 and + /// the server would ignore a pet-name query for it); a guardian's `SetPetNumber(n, false)` + /// writes 0; a mind-controlled creature keeps a `HIGHGUID_UNIT` guid and gains a non-zero + /// number. + /// + /// **Without one it is the best the guid can say**, and says less: a `HIGHGUID_PET` guid's + /// entry slot is taken as the pet number, which is right for a permanent pet (`Pet::Create` + /// feeds the same number to both — `SetPetNumber(n, IsPermanentPetFor(owner))`) and cannot name + /// a companion or a guardian. A caller that holds the unit's store must pass it. + fn of(guid_val: u64, unit: Option<&ObjectStore>) -> Option { if guid::is_player(guid_val) { - self.players + return Some(Self::Player(guid_val)); + } + if !guid::is_creature_or_pet(guid_val) { + return None; + } + match unit { + Some(unit) => match unit.0.unit_pet_number() { + 0 => unit + .0 + .object_entry() + .filter(|&e| e != 0) + .or_else(|| guid::entry(guid_val)) + .map(Self::Creature), + n => Some(Self::Pet(n)), + }, + None => guid::pet_number(guid_val) + .map(Self::Pet) + .or_else(|| guid::entry(guid_val).map(Self::Creature)), + } + } +} + +impl NameCache { + /// The name for `guid` **when no descriptor for it is in hand**, if known — a roster member, a + /// mail sender, a chat speaker, a unit that has left. On a miss, sends the right query (once + /// per key per connection) and returns `None` — call again after the answer lands. A guid + /// family that has no name on the 1.12 wire (GameObjects resolve via their own query, not + /// modeled yet) is `None` without a query. + /// + /// **A caller holding the unit's [`ObjectStore`] uses [`Self::resolve_unit`]** — the guid + /// alone cannot name a companion pet or a guardian ([`NameKey::of`]). + pub(crate) fn resolve(&self, guid_val: u64, commands: &NetCommands) -> Option<&str> { + self.resolve_unit(guid_val, None, commands) + } + + /// The name for a unit, keyed off its descriptor — the reference's `GetUnitName` + /// (`0x609210`, see [`NameKey`]). `unit: None` degrades to [`Self::resolve`]'s guid-only key. + /// Same ask-once discipline on every branch. + pub(crate) fn resolve_unit( + &self, + guid_val: u64, + unit: Option<&ObjectStore>, + commands: &NetCommands, + ) -> Option<&str> { + match NameKey::of(guid_val, unit)? { + NameKey::Player(guid_val) => self + .players .get_or_ask(guid_val, || { debug!("names: asking player name (guid {guid_val})"); let _ = commands.0.send(ClientCommand::NameQuery { guid: guid_val }); }) - .map(String::as_str) - } else if let Some(pet_number) = guid::pet_number(guid_val) { - // A pet is a `TYPEID_UNIT` like any creature, but it carries a pet number where a - // creature carries its template entry, so it has its own query. Asking the creature - // query for that number is what left every summoned pet nameless. - self.resolve_pet(pet_number, guid_val, commands) - } else if guid::is_creature_or_pet(guid_val) { - let entry = guid::entry(guid_val)?; - self.resolve_creature(entry, guid_val, commands) - } else { - None + .map(String::as_str), + // A pet is a `TYPEID_UNIT` like any creature, but one with a pet number is named + // through its own query. Asking the creature query for the number is what left every + // summoned pet nameless; asking the pet query for a number the server never filed is + // what left every companion pet nameless. + NameKey::Pet(pet_number) => self.resolve_pet(pet_number, guid_val, commands), + NameKey::Creature(entry) => self.resolve_creature(entry, guid_val, commands), } } @@ -237,18 +306,19 @@ impl NameCache { } /// The cached name for `guid`, read-only — no query on a miss (the trace/diagnostic twin of - /// [`Self::resolve`], for callers that must not mutate the ask-once state). + /// [`Self::resolve`], for callers that must not mutate the ask-once state). Guid-only, with + /// [`Self::resolve`]'s limits; [`Self::peek_unit`] is the descriptor-keyed twin. pub(crate) fn peek(&self, guid_val: u64) -> Option<&str> { - if guid::is_player(guid_val) { - self.players.get(guid_val).map(String::as_str) - } else if let Some(pet_number) = guid::pet_number(guid_val) { - self.pets.get(pet_number).map(String::as_str) - } else if guid::is_creature_or_pet(guid_val) { - self.creatures - .get(guid::entry(guid_val)?) - .map(|r| r.name.as_str()) - } else { - None + self.peek_unit(guid_val, None) + } + + /// The cached name for a unit keyed off its descriptor, read-only — [`Self::resolve_unit`]'s + /// no-query twin. + pub(crate) fn peek_unit(&self, guid_val: u64, unit: Option<&ObjectStore>) -> Option<&str> { + match NameKey::of(guid_val, unit)? { + NameKey::Player(guid_val) => self.players.get(guid_val).map(String::as_str), + NameKey::Pet(pet_number) => self.pets.get(pet_number).map(String::as_str), + NameKey::Creature(entry) => self.creatures.get(entry).map(|r| r.name.as_str()), } } @@ -1100,4 +1170,108 @@ mod tests { Ok(ClientCommand::CreatureQuery { entry: 100, .. }) )); } + + /// `OBJECT_FIELD_ENTRY` and `UNIT_FIELD_PETNUMBER`, absolute descriptor indices. + const ENTRY: u16 = 3; + const PETNUMBER: u16 = 139; + + fn unit(fields: &[(u16, u32)]) -> ObjectStore { + ObjectStore(benilla_protocol::ObjectFields::from_pairs(fields)) + } + + fn record(name: &str) -> CreatureRecord { + CreatureRecord { + name: name.into(), + subname: None, + creature_type: 0, + pet_family: 0, + rank: 0, + type_flags: 0, + civilian: false, + racial_leader: false, + display_id: 0, + } + } + + /// **A companion pet is named by its template, not by the pet number in its guid** — + /// `0x609210`'s `UNIT_FIELD_PETNUMBER == 0` leg. vmangos builds a mini-pet as a + /// `HIGHGUID_PET` guid around a fresh pet number but never files that number in its charm + /// info, so a `CMSG_PET_NAME_QUERY` for it is never answered (`PetHandler.cpp:190-192`) — the + /// guid-keyed resolve left every Mechanical Squirrel "Unknown" forever. Its descriptor says + /// PETNUMBER 0, and the reference reads the creature record under `OBJECT_FIELD_ENTRY`. + #[test] + fn a_companion_pet_is_named_by_its_descriptor_entry() { + let (cmds, rx) = commands(); + let mut cache = NameCache::default(); + let squirrel = compose(guid::HIGH_PET, 5_021, 3); + let store = unit(&[(ENTRY, 2_671), (PETNUMBER, 0)]); + cache.insert_creature(2_671, Some(record("Mechanical Squirrel"))); + + assert_eq!( + cache.resolve_unit(squirrel, Some(&store), &cmds), + Some("Mechanical Squirrel") + ); + assert_eq!( + cache.peek_unit(squirrel, Some(&store)), + Some("Mechanical Squirrel") + ); + assert!( + matches!(rx.try_recv(), Err(TryRecvError::Empty)), + "no pet-name query for a unit whose PETNUMBER is 0" + ); + } + + /// The cold companion asks the CREATURE query for its descriptor entry — once. + #[test] + fn a_cold_companion_pet_asks_the_creature_query_for_its_entry() { + let (cmds, rx) = commands(); + let cache = NameCache::default(); + let squirrel = compose(guid::HIGH_PET, 5_021, 3); + let store = unit(&[(ENTRY, 2_671)]); + + assert_eq!(cache.resolve_unit(squirrel, Some(&store), &cmds), None); + assert!(matches!( + rx.try_recv(), + Ok(ClientCommand::CreatureQuery { entry: 2_671, guid }) if guid == squirrel + )); + assert_eq!(cache.resolve_unit(squirrel, Some(&store), &cmds), None); + assert!(matches!(rx.try_recv(), Err(TryRecvError::Empty))); + } + + /// A hunter/warlock pet carries its pet number in the descriptor (`SetPetNumber(n, true)`), + /// and that is the key the pet-name query goes out under — not the creature template. + #[test] + fn a_permanent_pet_asks_the_pet_query_for_its_descriptor_number() { + let (cmds, rx) = commands(); + let mut cache = NameCache::default(); + let rex = compose(guid::HIGH_PET, 7, 42); + let store = unit(&[(ENTRY, 3_122), (PETNUMBER, 7)]); + cache.insert_creature(3_122, Some(record("Bloodtalon Taillasher"))); + + assert_eq!(cache.resolve_unit(rex, Some(&store), &cmds), None); + assert!(matches!( + rx.try_recv(), + Ok(ClientCommand::PetNameQuery { pet_number: 7, guid }) if guid == rex + )); + cache.insert_pet(7, "Rex".into()); + assert_eq!(cache.resolve_unit(rex, Some(&store), &cmds), Some("Rex")); + assert_eq!(cache.peek_unit(rex, Some(&store)), Some("Rex")); + } + + /// A mind-controlled creature (`HIGHGUID_UNIT` guid) carries a pet number too + /// (`SpellAuras.cpp:3248`, `SetPetNumber(GeneratePetNumber(), true)`), and the reference names + /// it through the pet-name cache under that number for as long as the charm holds. + #[test] + fn a_charmed_creature_is_named_by_its_pet_number() { + let (cmds, rx) = commands(); + let cache = NameCache::default(); + let ogre = compose(guid::HIGH_UNIT, 1_000, 9); + let store = unit(&[(ENTRY, 1_000), (PETNUMBER, 55)]); + + assert_eq!(cache.resolve_unit(ogre, Some(&store), &cmds), None); + assert!(matches!( + rx.try_recv(), + Ok(ClientCommand::PetNameQuery { pet_number: 55, .. }) + )); + } } diff --git a/crates/benilla-app/src/net.rs b/crates/benilla-app/src/net.rs index b43caeab..6753ecc9 100644 --- a/crates/benilla-app/src/net.rs +++ b/crates/benilla-app/src/net.rs @@ -32,6 +32,7 @@ pub(crate) mod handlers; pub(crate) mod io; mod motion; mod objects; +pub(crate) use objects::tear_down; mod session; mod world; diff --git a/crates/benilla-app/src/net/objects.rs b/crates/benilla-app/src/net/objects.rs index e07493ad..ebebd7dd 100644 --- a/crates/benilla-app/src/net/objects.rs +++ b/crates/benilla-app/src/net/objects.rs @@ -891,17 +891,20 @@ fn object_destroyed(guid: u64, commands: &mut Commands, index: &mut GuidIndex) { // fade is not the animation; it is what every teardown does underneath it. // // The guid leaves the index either way — to the server it no longer exists, and a respawn - // streams in as a fresh entity that fades in over the top. If it was the target, the ring's - // gone-entity branch clears the selection next frame, the same as the reference's teardown - // (which sends the same `CMSG_SET_SELECTION 0`). + // streams in as a fresh entity that fades in over the top. **The object ends here, not when + // its model finishes fading** ([`tear_down`]): the entity sheds its [`Guid`] in this same + // handler's command flush, so every consumer that means "a live object" stops seeing it at + // once — and if it was the target, the ring's gone-object branch clears the selection and + // sends `CMSG_SET_SELECTION 0` on its next pass, this frame, as the reference's OnDeactivate + // does at the teardown itself (`0x5fbb60` → `0x493910`, wow-re `selection-death-clear.md` Q2). // // …*unless the object is pinned* — `0x464920` on a still-pinned object only sets the // pending-destroy bit and returns, and the real free waits for the last pin to drop (wow-re // `go-display-sound-events.md` §6d). The one pin benilla takes is the despawn animation // announced a moment earlier by `SMSG_GAMEOBJECT_DESPAWN_ANIM`, which is the whole of how an // object gets to play its own despawn after the server says it is gone (decision 1404); the - // fade then follows the animation, where the deferred destroy runs - // ([`crate::go_anim::release_despawn_pin`]). + // fade then follows the animation, where the deferred destroy — and so the teardown — runs + // ([`crate::go_anim::release_despawn_pin`]). A pinned object is still an object until then. if let Some(e) = index.0.remove(&guid) { // An item has no model to hand the fadeout, so it goes now (decision 2334) — what the // scheduler does with a modelless entity anyway, one frame later — and its countdown @@ -918,8 +921,8 @@ fn object_destroyed(guid: u64, commands: &mut Commands, index: &mut GuidIndex) { if let Ok(mut ent) = world.get_entity_mut(e) { ent.insert(crate::go_anim::PendingDestroy); } - } else if let Ok(mut ent) = world.get_entity_mut(e) { - ent.insert(DespawnFade::default()); + } else if let Ok(ent) = world.get_entity_mut(e) { + tear_down(ent); } }); } @@ -934,14 +937,41 @@ fn objects_removed(guids: Vec, commands: &mut Commands, index: &mut GuidInd // eyes). The mechanism behind it is the same one [`object_destroyed`] above now takes — the // OUT_OF_RANGE block and DESTROY reach `0x464920` alike, and its OnDeactivate hands the model // to the `SWModelFadeout` scheduler either way ([`DespawnFade`], decision 2198). That the two - // routes agree is not a convenience here; it is the reference's own shape. + // routes agree is not a convenience here; it is the reference's own shape — and so is the + // object ending at the stream-out rather than when the fade does ([`tear_down`]): a unit that + // walks out of range, vanishes or stealths stops being targetable, TAB-able and hoverable now. for g in guids { if let Some(e) = index.0.remove(&g) { - commands.entity(e).insert(DespawnFade::default()); + commands.entity(e).queue(tear_down); } } } +/// **An object's teardown** — the reference's `0x464920`, which both `SMSG_DESTROY_OBJECT` and +/// the OUT_OF_RANGE block reach: the OBJECT is freed on the spot, and only its detached MODEL +/// survives, handed to the `SWModelFadeout` scheduler to ramp out ([`DespawnFade`], decision +/// 2198). The OnDeactivate on the way (`0x5fbb60` → `0x493910`) clears the selection if it was +/// this unit and sends `CMSG_SET_SELECTION 0` — at the teardown, not two seconds later (wow-re +/// `object-layer/scratch/selection-death-clear.md` Q2). +/// +/// benilla keeps the model on the same entity rather than re-parenting it onto a fresh one, so +/// "the object is gone" is said by taking away the one component that makes an entity an object: +/// its [`Guid`], the server identity every object consumer keys on — the TAB scan, `/target`, +/// the mouseover pick, the name and nameplate walks, the minimap blips, the quest markers, the +/// chat bubbles, the selection ring's gone-object branch, the unit feed's "left the manager" +/// gate. What the model needs to finish drawing stays: the [`NetEntity`] the renderer reads, and +/// the [`ObjectStore`] the animation driver reads — a corpse that decays keeps lying dead while +/// it fades instead of reading as alive and standing up. The guid has already left the +/// [`GuidIndex`] (the caller's job), so a same-tick re-create of it (corpse → respawn) spawns a +/// fresh object beside the fading model, and nothing ever sees two objects with one guid. +/// +/// Every teardown goes through here: both wire routes, and a pinned object's deferred destroy +/// ([`crate::go_anim::release_despawn_pin`]). +pub(crate) fn tear_down(mut ent: EntityWorldMut) { + ent.remove::(); + ent.insert(DespawnFade::default()); +} + /// A creature path packet (`SMSG_MONSTER_MOVE`, or its deck twin `SMSG_MONSTER_MOVE_TRANSPORT`): /// settle the unit's transport membership, apply the dictated facing snap, then attach or clear /// the travel spline. diff --git a/crates/benilla-app/src/npc_text.rs b/crates/benilla-app/src/npc_text.rs index e81bb3f0..addc125e 100644 --- a/crates/benilla-app/src/npc_text.rs +++ b/crates/benilla-app/src/npc_text.rs @@ -239,8 +239,9 @@ pub(crate) fn subject_for_guid( if guid == 0 { return None; } - let name = names.resolve(guid, commands)?.to_string(); - if let Some(store) = index.0.get(&guid).and_then(|e| stores.get(*e).ok()) { + let store = index.0.get(&guid).and_then(|e| stores.get(*e).ok()); + let name = names.resolve_unit(guid, store, commands)?.to_string(); + if let Some(store) = store { return Some(Subject { name, race: store.0.unit_race().unwrap_or(0), diff --git a/crates/benilla-app/src/pending_item_ops.rs b/crates/benilla-app/src/pending_item_ops.rs index 04b8db64..4bf14a7d 100644 --- a/crates/benilla-app/src/pending_item_ops.rs +++ b/crates/benilla-app/src/pending_item_ops.rs @@ -148,6 +148,16 @@ impl PendingItemOps { unlocked } + /// The session end: drop every outstanding entry and report nothing. The lock is item-object + /// state in the reference (`item+0x314`), and those objects do not outlive the session; an op + /// still in flight when the socket died never gets the field update or failure that would + /// settle it, so without this its slots stayed locked for the whole next session. The epoch + /// steps so a feed that pushed `locked: true` runs once more to correct it. + pub(crate) fn clear_session(&mut self) { + self.entries.clear(); + self.epoch += 1; + } + /// Drop every outstanding entry, unconditionally — [`Self::clear_by_failure`]'s /// unmatched/zero fallback. fn clear_all(&mut self) -> Vec<(i64, u32)> { diff --git a/crates/benilla-app/src/player.rs b/crates/benilla-app/src/player.rs index 2ace39a6..a5c22329 100644 --- a/crates/benilla-app/src/player.rs +++ b/crates/benilla-app/src/player.rs @@ -208,9 +208,9 @@ pub(crate) const UNIT_FLAG_IN_COMBAT: u32 = 0x0008_0000; /// `UnitDefines.h:511`; the client reads it as `shr reg,0x14; test rl,1` — wow-re counted 20 /// independent sites of that idiom with no shared gate, `unit-flags-movement-gates.md` §4). /// -/// Beside its neighbour for the same reason that one is here: two readers today that have nothing -/// to do with each other — `SetStandState`'s own guard #3 and the idle handler's auto-AFK gate — -/// and a bit that is spelled out twice is a bit that eventually drifts. +/// Beside its neighbour for the same reason that one is here: readers that have nothing to do with +/// each other — `SetStandState`'s own guard #3, the idle handler's auto-AFK gate and `UnitOnTaxi` +/// — and a bit that is spelled out more than once is a bit that eventually drifts. pub(crate) const UNIT_FLAG_TAXI_FLIGHT: u32 = 0x0010_0000; /// Ask for a **stand state** — the client's `SetStandState(newState)` (`0x5ed430`: send diff --git a/crates/benilla-app/src/player/camera_saved.rs b/crates/benilla-app/src/player/camera_saved.rs index c535160d..cb70ce72 100644 --- a/crates/benilla-app/src/player/camera_saved.rs +++ b/crates/benilla-app/src/player/camera_saved.rs @@ -34,9 +34,11 @@ //! back (1138). benilla has no UI reload, so there is no edge here to miss; when one lands it joins //! this list. //! -//! **Read once**, when the roster names the character we are entering the world as — the macro/ -//! binding load's own seam ([`crate::ui_macro::identity`]). Absent file = the shipped defaults, which -//! is the normal first run. +//! **Read once per session**, when the roster names the character we are entering the world as — +//! the macro/binding load's own seam ([`crate::ui_macro::identity`]). Absent file = the shipped +//! defaults, which is the normal first run: the rig is re-seated at them first, so a fresh alt does +//! not open at the zoom of whoever played last. The pitch reaches the camera through the login +//! seize ([`super::Player::login_pitch`]), which is the one place the opening pitch is seated. use std::path::PathBuf; @@ -44,7 +46,10 @@ use bevy::prelude::*; use crate::char_select::{ClientState, InWorldGated}; -use super::camera::{CameraControl, FlyCam, CAM_DIST_MAX, CAM_DIST_MIN, CAM_PITCH_LIMIT}; +use super::camera::{ + CameraControl, FlyCam, CAM_DIST_DEFAULT, CAM_DIST_MAX, CAM_DIST_MIN, CAM_PITCH_LIMIT, +}; +use super::Player; use benilla_world::view::WorldCamera; /// The persisted pose's file keys — the reference's own spellings, in the reference's own order. @@ -144,6 +149,7 @@ fn load_camera_pose( mut file: ResMut, mut rig: ResMut, mut cam: Query<&mut FlyCam, With>, + mut player: ResMut, ) { let Some(id) = crate::ui_macro::identity(&roster) else { return; @@ -156,6 +162,11 @@ fn load_camera_pose( }; file.path = crate::local_state::camera_character_path(&id.0, &id.1); file.identity = Some(id); + // This character's pose starts from the shipped defaults, not from the last one's. + rig.distance = CAM_DIST_DEFAULT; + rig.target_distance = CAM_DIST_DEFAULT; + rig.collision_distance = CAM_DIST_DEFAULT; + player.login_pitch = None; let Some(path) = file.path.clone() else { return; // hermetic capture, or no install — session-only, defaults stand @@ -177,6 +188,7 @@ fn load_camera_pose( } if let Some(p) = pitch { cam.pitch = p; + player.login_pitch = Some(p); } info!("camera pose: restored from {}", path.display()); } @@ -198,13 +210,19 @@ fn save(file: &CameraPoseFile, rig: &CameraControl, pitch: f32) { /// `OnExit(InWorld)`: `/logout` back to the glue, or a disconnect. fn save_on_session_end( - file: Res, + mut file: ResMut, rig: Res, cam: Query<&FlyCam, With>, ) { if let Ok(cam) = cam.single() { save(&file, &rig, cam.pitch); } + // The next login reads the file again, whoever it is — a reconnect or a relog of this same + // character included, whose seize would otherwise open at the shipped pitch. The path goes + // too: a quit from the character screen after this has no character's pose to write, and + // would otherwise save the glue camera's pitch over this one's. + file.identity = None; + file.path = None; } /// `AppExit`: quitting the client. Reads the message rather than a state edge because a quit from @@ -296,3 +314,97 @@ mod tests { assert!((p.unwrap() - pitch_from_file(10.0)).abs() < 1e-6); } } + +#[cfg(test)] +mod login_tests { + use bevy::ecs::system::RunSystemOnce; + + use super::*; + use crate::char_select::Roster; + use crate::local_state::test_env::{EnvGuard, ENV_LOCK}; + + fn roster(name: &str, guid: u64) -> Roster { + Roster::with_pending_pick( + vec![benilla_protocol::Character { + guid, + name: name.into(), + race: 1, + class: 1, + gender: 0, + skin: 0, + face: 0, + hair_style: 0, + hair_color: 0, + facial_hair: 0, + level: 1, + zone: 0, + map: 0, + position: benilla_protocol::wire::Vector3d { + x: 0.0, + y: 0.0, + z: 0.0, + }, + flags: 0, + equipment: [benilla_protocol::CharEnumItem::default(); 19], + pet_display_id: 0, + pet_level: 0, + pet_family: 0, + }], + guid, + ) + } + + /// **The restored pitch reaches the login seize, and an alt starts from the defaults.** The + /// seize seated its own constant over the pitch this load had just restored, so only the + /// distance half of the remembered pose ever survived a login; and a character with no file + /// kept whatever zoom the previous one had left in the rig. + #[test] + fn the_saved_pitch_is_the_login_pitch_and_a_fresh_alt_gets_the_defaults() { + let _l = ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let tmp = std::env::temp_dir().join(format!("benilla-cam-{}", std::process::id())); + std::fs::remove_dir_all(&tmp).ok(); + let _c = EnvGuard::unset("WOW_CAPTURE"); + let _h = EnvGuard::set("BENILLA_HOME", tmp.to_str().unwrap()); + let path = crate::local_state::camera_character_path("Realm", "Tank").unwrap(); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, render(8.0, -0.3)).unwrap(); + + let mut app = App::new(); + app.init_resource::() + .init_resource::() + .init_resource::() + .insert_resource(roster("Tank", 7)); + app.world_mut().spawn(( + FlyCam { + yaw: 0.0, + pitch: 0.0, + speed: 1.0, + }, + WorldCamera, + )); + app.world_mut().run_system_once(load_camera_pose).unwrap(); + let restored = app.world().resource::().login_pitch; + assert!( + restored.is_some_and(|p| (p + 0.3).abs() < 1e-3), + "the file's pitch is what the seize will seat: {restored:?}" + ); + assert_eq!(app.world().resource::().target_distance, 8.0); + + // The session ends; an alt with no file logs in. + app.world_mut() + .run_system_once(save_on_session_end) + .unwrap(); + *app.world_mut().resource_mut::() = Player::default(); + app.insert_resource(roster("Healer", 8)); + app.world_mut().run_system_once(load_camera_pose).unwrap(); + assert_eq!(app.world().resource::().login_pitch, None); + assert_eq!( + app.world().resource::().target_distance, + CAM_DIST_DEFAULT, + "a fresh alt opens at the shipped zoom, not the tank's" + ); + std::fs::remove_dir_all(&tmp).ok(); + } +} diff --git a/crates/benilla-app/src/player/state.rs b/crates/benilla-app/src/player/state.rs index fce1ad5b..28cecc70 100644 --- a/crates/benilla-app/src/player/state.rs +++ b/crates/benilla-app/src/player/state.rs @@ -876,6 +876,11 @@ pub(crate) struct Player { /// judgement and pushes [`Player::settle_deadline`] forward, so the timeout budget measures /// time waiting for the *destination's* world (decisions 0710 + 0737). pub(crate) world_stale: bool, + /// The pitch the login seize seats the camera at — the saved pose's `cameraPitch` when + /// [`super::camera_saved`] restored one for this session, else the shipped opening pitch. Set + /// by the pose load, read once by the seize: the seize used to seat its own constant over the + /// restored value, so the pitch half of the remembered pose never survived a login. + pub(super) login_pitch: Option, /// A same-map teleport landed: the server relocated the mover, so any in-progress self /// server-ride (charge/taxi) is **void** — vmangos teleports at ITS flight end (its own spline /// finishes ~latency before ours) and its spline-done handler ignores acks while the teleport @@ -1323,8 +1328,9 @@ impl Player { } /// A server-authored spline currently owns the avatar (Charge/knockback/taxi — the - /// [`super::server_ride`] state). For instruments (the taxi probe watches the flight run) and - /// the UI's `UnitOnTaxi` feed. + /// [`super::server_ride`] state). For instruments (the taxi probe watches the flight run). + /// **Not** `UnitOnTaxi` — a fear or a charge is a server ride too; that verb reads + /// [`crate::player::UNIT_FLAG_TAXI_FLIGHT`] off the descriptor (`0x517a86`). pub(crate) fn server_riding(&self) -> bool { self.server_riding } diff --git a/crates/benilla-app/src/player/wire_in.rs b/crates/benilla-app/src/player/wire_in.rs index e6b2afaf..023ee129 100644 --- a/crates/benilla-app/src/player/wire_in.rs +++ b/crates/benilla-app/src/player/wire_in.rs @@ -72,6 +72,9 @@ enum ControlVerdict { /// /// Skipped while a server-authored spline owns the body: the reference returns early on that edge /// (`0x619d50`), and vmangos would discard it anyway (`HasPendingSplineDone`). +/// The pitch a login opens at when no saved camera pose names one. +const LOGIN_PITCH: f32 = -0.45; + fn yield_own_body(net_cmds: &NetCommands, player: &mut Player, self_guid: Option) { movement_net::park_mover(&net_cmds.0, player); let Some(me) = self_guid else { return }; @@ -452,7 +455,7 @@ pub(super) fn apply_server_moves( // orientation on a fresh login) — adopt it whole, camera seated behind, like // the reference. Zeroing here made every login face due north regardless. cam.yaw = yaw; - cam.pitch = -0.45; + cam.pitch = player.login_pitch.unwrap_or(LOGIN_PITCH); } // The camera is deliberately NOT re-seated on a mover change: it is already on the new // body (Mind Control sets `PLAYER_FARSIGHT` to the victim alongside the handoff, so diff --git a/crates/benilla-app/src/target/ring.rs b/crates/benilla-app/src/target/ring.rs index 595a0458..8c90b6a3 100644 --- a/crates/benilla-app/src/target/ring.rs +++ b/crates/benilla-app/src/target/ring.rs @@ -42,7 +42,7 @@ use benilla_formats::{load_faction_catalog, reputation_rank, FactionCatalog, Rea use benilla_protocol::EntityKind; use bevy::prelude::*; -use crate::net::{NetEntity, ObjectStore, Reputations, SelfPlayer}; +use crate::net::{Guid, NetEntity, ObjectStore, Reputations, SelfPlayer}; use benilla_assets::{LockRecover, WorldAssets}; use benilla_world::decal::{DecalFrame, WorldDecal}; use benilla_world::particles::buffer::EffectVertex; @@ -256,8 +256,9 @@ pub(super) fn load_factions(mut commands: Commands, world_assets: Option, @@ -287,14 +288,22 @@ pub(super) fn update_ring( // box, `0x60ce70` tail → `0x60aee0`; the scale law is B3 — // `SCALE_X × CreatureDisplayInfo.creatureModelScale`, and the child's `NetEntity.scale` // carries exactly the CDI column). + // + // `.0` is filtered on [`Guid`] — **a live object**, not merely a drawn model: a torn-down + // unit keeps its model (and its `Transform`) for the two-second fadeout, but sheds its guid + // at the teardown ([`crate::net::tear_down`]), so it lands in the gone-object branch below on + // the teardown's own frame rather than when the fade despawns it. targets: ( - Query<( - &Transform, - Option<&SelectionRadius>, - Option<&ObjectStore>, - Option<&NetEntity>, - Option<&crate::entities::mount::MountChild>, - )>, + Query< + ( + &Transform, + Option<&SelectionRadius>, + Option<&ObjectStore>, + Option<&NetEntity>, + Option<&crate::entities::mount::MountChild>, + ), + With, + >, Query<(&NetEntity, Option<&SelectionRadius>), With>, // The party roster — the selector's 4-slot guid table (the party ring colours). Res, @@ -414,9 +423,10 @@ pub(super) fn update_ring( // own no-ground gate (`0x6d74b5`: the whole draw is skipped). !projected } - // The target entity no longer exists (destroyed or streamed out): clear, informing the - // server — the reference's teardown does exactly this for both removal paths (object - // deactivate → the selection clear + `CMSG_SET_SELECTION 0`, byte-verified — wow-re + // The target is no longer an object (destroyed or streamed out — torn down, its model + // perhaps still fading; or despawned outright): clear, informing the server — the + // reference's teardown does exactly this for both removal paths (object deactivate → + // the selection clear + `CMSG_SET_SELECTION 0`, byte-verified — wow-re // selection-death-clear RE; this is also what drops a selected corpse at respawn, when // the server destroys it ahead of the fresh create). Err(_) => { @@ -1345,4 +1355,186 @@ mod tests { false )); } + + /// **A torn-down object stops being an object at the teardown, not when its model is done + /// fading.** The reference's two teardown routes (`SMSG_DESTROY_OBJECT`, the OUT_OF_RANGE + /// block) both reach `0x464920` → OnDeactivate `0x5fbb60` → `0x493910`, which clears a + /// matching selection and sends `CMSG_SET_SELECTION 0` on the spot; only the detached model + /// survives into the `SWModelFadeout` scheduler (wow-re `selection-death-clear.md` Q2, + /// decision 2198). Driven through the real handler table on the built client, then the real + /// ring and the real nearest-enemy scan, one pass each — no fade time elapses. + mod teardown { + use benilla_protocol::field::{ + FIELD_UNIT_FLAGS, FIELD_UNIT_HEALTH, FIELD_UNIT_LEVEL, FIELD_UNIT_MAXHEALTH, + }; + use benilla_protocol::messages::ObjectType; + use benilla_protocol::{EntityKind, ObjectFields, SessionEvent}; + use bevy::ecs::system::RunSystemOnce; + use bevy::prelude::*; + use crossbeam_channel::Receiver; + + use crate::net::{ClientCommand, Guid, GuidIndex, NetCommands, ObjectStore, SelfPlayer}; + use crate::target::{attack_order_target, Selection, TargetScan}; + use benilla_world::model_fade::DespawnFade; + + const ME: u64 = 0x0000_0000_0000_0007; + const MOB: u64 = 0xF130_0000_1234_0001; + + fn create(guid: u64) -> SessionEvent { + SessionEvent::ObjectCreate { + guid, + kind: EntityKind::Unit, + display_id: None, + position: [3.0, 0.0, 0.0], + orientation: 0.0, + scale: 1.0, + speeds: None, + mover: None, + transport_progress: None, + transport: None, + spline: None, + fields: ObjectFields::from_pairs(&[ + (FIELD_UNIT_HEALTH, 100), + (FIELD_UNIT_MAXHEALTH, 100), + (FIELD_UNIT_LEVEL, 9), + ]) + .into_created(ObjectType::Unit), + } + } + + /// The built client with our own avatar and one live mob in the index, the mob + /// selected, and the write channel captured. + fn client_with_selected_mob() -> (App, Entity, Receiver) { + let mut app = crate::game_plugins::schedule_tests::headless_client(); + let (tx, rx) = crossbeam_channel::unbounded(); + app.insert_resource(NetCommands(tx)); + let world = app.world_mut(); + // Inserted by a Startup system on a real boot; no schedule runs here. + world.init_resource::(); + let me = world + .spawn(( + SelfPlayer, + Guid(ME), + Transform::default(), + ObjectStore( + // Player-controlled (`UNIT_FLAG_PVP_ATTACKABLE`), so a neutral mob is + // attackable with no faction catalog loaded — the scan's control half. + ObjectFields::from_pairs(&[ + (FIELD_UNIT_HEALTH, 100), + (FIELD_UNIT_MAXHEALTH, 100), + (FIELD_UNIT_FLAGS, 0x8), + ]) + .into_created(ObjectType::Player), + ), + )) + .id(); + world.resource_mut::().0.insert(ME, me); + crate::net::handlers::dispatch(world, vec![create(MOB)]); + let mob = world.resource::().0[&MOB]; + *world.resource_mut::() = Selection { + target: Some(mob), + guid: Some(MOB), + }; + // The control: a live selected unit keeps its selection through a ring pass. + world + .run_system_once(super::super::update_ring) + .expect("the ring runs on the built client"); + assert_eq!(world.resource::().guid, Some(MOB)); + // (The create's own name query is on the channel; only a selection send matters.) + assert!( + !rx.try_iter() + .any(|c| matches!(c, ClientCommand::SetSelection { .. })), + "a live target is not deselected" + ); + (app, mob, rx) + } + + fn selection_cleared_at(teardown: SessionEvent) { + let (mut app, mob, rx) = client_with_selected_mob(); + let world = app.world_mut(); + crate::net::handlers::dispatch(world, vec![teardown]); + // The model is still there, fading — the scheduler's half of the teardown. + assert!( + world.get_entity(mob).is_ok(), + "the model outlives the object" + ); + assert!(world.get::(mob).is_some(), "…and fades"); + world + .run_system_once(super::super::update_ring) + .expect("the ring runs on the built client"); + let sel = world.resource::(); + assert_eq!( + (sel.target, sel.guid), + (None, None), + "the selection ends with the object, not 2 s later with its model" + ); + let sent: Vec<_> = rx.try_iter().collect(); + assert!( + sent.iter() + .any(|c| matches!(c, ClientCommand::SetSelection { guid: 0 })), + "the server is told at the teardown: {sent:?}" + ); + } + + #[test] + fn a_destroyed_target_clears_the_selection_at_the_destroy() { + selection_cleared_at(SessionEvent::ObjectDestroyed(MOB)); + } + + #[test] + fn a_streamed_out_target_clears_the_selection_at_the_stream_out() { + selection_cleared_at(SessionEvent::ObjectsRemoved(vec![MOB])); + } + + /// The nearest-enemy acquire (the TAB core) cannot pick a fading model: it is not an + /// object any more. The control half proves the scan does find the mob while it lives. + #[test] + fn the_nearest_enemy_scan_never_picks_a_torn_down_object() { + let (mut app, _mob, _rx) = client_with_selected_mob(); + let world = app.world_mut(); + let acquire = |world: &mut World| { + *world.resource_mut::() = Selection::default(); + world + .run_system_once( + |scan: TargetScan, + mut sel: ResMut, + mut seam: crate::creature_anim::AttackSeam, + mut errors: ResMut| { + attack_order_target(&scan, &mut sel, &mut seam, &mut errors) + }, + ) + .expect("the scan runs on the built client") + }; + assert_eq!( + acquire(world), + Some(MOB), + "control: the live mob is acquired" + ); + crate::net::handlers::dispatch(world, vec![SessionEvent::ObjectsRemoved(vec![MOB])]); + assert_eq!(acquire(world), None, "the fading model is not a candidate"); + } + + /// The corpse → respawn shape: the server destroys the old object and creates the same + /// guid again in one tick. The fresh create is a new live object, and the old model + /// fading beside it no longer answers to the guid. + #[test] + fn a_same_tick_recreate_is_a_fresh_object_and_the_old_model_is_nobody() { + let (mut app, old, _rx) = client_with_selected_mob(); + let world = app.world_mut(); + crate::net::handlers::dispatch( + world, + vec![SessionEvent::ObjectDestroyed(MOB), create(MOB)], + ); + let fresh = world.resource::().0[&MOB]; + assert_ne!(fresh, old, "a fresh entity for the fresh object"); + assert!(world.get::(fresh).is_none()); + let answering: Vec = world + .query::<(Entity, &Guid)>() + .iter(world) + .filter(|(_, g)| g.0 == MOB) + .map(|(e, _)| e) + .collect(); + assert_eq!(answering, [fresh], "one object answers to the guid"); + } + } } diff --git a/crates/benilla-app/src/target/scan.rs b/crates/benilla-app/src/target/scan.rs index 1d0e8e0b..f834acdb 100644 --- a/crates/benilla-app/src/target/scan.rs +++ b/crates/benilla-app/src/target/scan.rs @@ -283,7 +283,6 @@ pub(crate) struct TargetScan<'w, 's> { &'static Guid, &'static Transform, Option<&'static ObjectStore>, - Option<&'static Visibility>, ), Without, >, @@ -410,8 +409,8 @@ impl TargetScan<'_, '_> { let self_store = self.self_store(); self.units .iter() - .find(|(_, _, g, _, _, _)| g.0 == guid) - .is_some_and(|(_, _, _, _, store, _)| { + .find(|(_, _, g, _, _)| g.0 == guid) + .is_some_and(|(_, _, _, _, store)| { reaction_from_player( self.factions.as_deref(), &self.reputations, @@ -425,8 +424,8 @@ impl TargetScan<'_, '_> { fn unit_by_guid(&self, guid: u64) -> Option<(Entity, Option<&ObjectStore>)> { self.units .iter() - .find(|(_, _, g, _, _, _)| g.0 == guid) - .map(|(e, _, _, _, store, _)| (e, store)) + .find(|(_, _, g, _, _)| g.0 == guid) + .map(|(e, _, _, _, store)| (e, store)) } /// The full build: walk every known unit, filter (the kept 1.12 legality laws), project @@ -478,7 +477,7 @@ impl TargetScan<'_, '_> { ); }; let mut out = Vec::new(); - for (entity, net, guid_c, tf, store, vis) in &self.units { + for (entity, net, guid_c, tf, store) in &self.units { if !matches!(net.kind, EntityKind::Unit | EntityKind::Player) { continue; } @@ -507,11 +506,12 @@ impl TargetScan<'_, '_> { } } } - // The scene-attach/hide notion (kept): an explicitly hidden root is out. - if vis == Some(&Visibility::Hidden) { - trace_unit(guid_c.0, tf, "REJECT hidden"); - continue; - } + // **No scene-attach gate.** The reference rejects a not-currently-rendered object + // here (`0x6704c0`), and this scan kept that while nothing hid a unit root. Since the + // outdoor draw election (1270/1475) hides every body outside the frustum, reading its + // `Visibility` would be the frustum by another name — and would empty the off-screen + // tier the Classic design (0567) keeps as the fallback. Tiering by the camera is the + // `project` step below; stealth and the like leave through the server's out-of-range. let dist = (tf.translation - self_tf.translation).length(); if dist > TAB_RANGE { trace_unit(guid_c.0, tf, "REJECT range (41 yd)"); @@ -1699,6 +1699,53 @@ mod tests { assert_eq!(pool(&mut world, ScanSide::Friend), [ALLY, ALLY_FEIGN]); } + /// **A unit the draw election culled is still a TAB candidate.** The outdoor election + /// (decisions 1270/1475) writes `Visibility::Hidden` on every body root outside the camera + /// frustum, every frame — so a scan gate on `Hidden` emptied the off-screen tier 0567 keeps + /// as the fallback, and TAB / attack-with-no-target / pet Attack could never reach a mob + /// hitting you from behind while nothing was on screen. + #[test] + fn a_body_the_draw_election_culled_is_still_a_candidate() { + use bevy::ecs::system::RunSystemOnce; + + const MOB: u64 = 0xBEEF; + let mut world = World::new(); + world.init_resource::(); + world.init_resource::(); + world.spawn(( + SelfPlayer, + Transform::default(), + Guid(1), + store(&[ + (F_TYPE, TYPE_PLAYER), + (F_FLAGS, CONTROLLED), + (F_HEALTH, 100), + (F_MAXHEALTH, 100), + ]), + )); + world.spawn(( + NetEntity { + kind: EntityKind::Unit, + display_id: None, + scale: 1.0, + }, + Guid(MOB), + Transform::from_xyz(0.0, 0.0, 5.0), + store(&[(F_HEALTH, 100), (F_MAXHEALTH, 100)]), + // The election's out-of-view verdict (`exterior_cull`'s body leg). + Visibility::Hidden, + )); + let pool = world + .run_system_once(|scan: TargetScan| { + scan.build(ScanSide::Enemy) + .iter() + .map(|c| c.guid) + .collect::>() + }) + .expect("the scan runs as a one-shot system"); + assert_eq!(pool, [MOB]); + } + /// [`LastEnemy`] tracks the last **attackable** selection and nothing else — `TargetLastEnemy`'s /// whole memory (`[0xb4e2e8]/[0xb4e2ec]`, `0x49377d`). /// diff --git a/crates/benilla-app/src/ui_auction/mod.rs b/crates/benilla-app/src/ui_auction/mod.rs index b1bdb55b..7373f447 100644 --- a/crates/benilla-app/src/ui_auction/mod.rs +++ b/crates/benilla-app/src/ui_auction/mod.rs @@ -104,19 +104,27 @@ const SUBCLASS_HIDDEN_FROM_AUCTIONS: u32 = 0x2; /// real `ItemClass.dbc` row. **Every string the filter displays comes from the player's own DBC**, /// which is what keeps this feature clear of decisions 1234/1260 — we ship the structure, the /// install supplies the words. -const AUCTION_CLASSES: [u32; 10] = [ - 2, // Weapon - 4, // Armor - 1, // Container - 0, // Consumable - 7, // Trade Goods - 6, // Projectile - 11, // Quiver - 9, // Recipe - 5, // Reagent - 15, // Miscellaneous +/// +/// Each entry is the reference's `0x807060` row `{itemClassId, hasSubclassFilter}` (wow-re +/// auction-house §9.1): a class whose flag is 0 offers no subclass rows and no inventory-slot rows. +const AUCTION_CLASSES: [(u32, bool); 10] = [ + (2, true), // Weapon + (4, true), // Armor + (1, true), // Container + (0, false), // Consumable + (7, false), // Trade Goods + (6, true), // Projectile + (11, true), // Quiver + (9, true), // Recipe + (5, false), // Reagent + (15, false), // Miscellaneous ]; +/// `ItemSubClass.Flags` bit `0x200`: the subclass offers the fourteen inventory-slot rows beneath +/// it (`GetAuctionInvTypes`, `0x4cfb63 test ah,2`). In the shipped file it is set on exactly Armor +/// 0..4 (Miscellaneous, Cloth, Leather, Mail, Plate) — not on Shield, Libram, Idol or Totem. +const SUBCLASS_OFFERS_INV_TYPES: u32 = 0x200; + /// The `AuctionHouse.dbc` catalog, loaded with the other item DBCs ([`crate::ui_items`]). Optional /// resource — absent, the sell pane shows no deposit rather than inventing one. #[derive(Resource)] @@ -514,7 +522,7 @@ pub(crate) fn categories( }; AUCTION_CLASSES .iter() - .filter_map(|&class_id| { + .filter_map(|&(class_id, has_subclass_filter)| { let name = classes.0.name(class_id)?.to_string(); let subs = subclasses .0 @@ -527,18 +535,16 @@ pub(crate) fn categories( Some(AuctionSubCategory { sub_id: sub, name: subclasses.0.name(class_id, sub)?.to_string(), - // INTERIM (decision 1511, §5 TU-4): which class/subclass pairs offer the - // fourteen inventory-slot rows is not yet derived from the binary. Armor - // is the one the reference visibly offers them under, and it is the only - // class where an equip slot narrows anything. Data, not logic — a - // correction is this line. - has_inv_types: class_id == 4, + has_inv_types: subclasses.0.flags(class_id, sub) + & SUBCLASS_OFFERS_INV_TYPES + != 0, }) }) .collect(); Some(AuctionCategory { class_id, name, + has_subclass_filter, subclasses: subs, }) }) @@ -820,6 +826,7 @@ fn drain_auction( level_min: u8::try_from(q.min_level).unwrap_or(u8::MAX), level_max: u8::try_from(q.max_level).unwrap_or(u8::MAX), slot_id: q.inv_type.unwrap_or(auction_filter::ANY), + // Already ids — the binding maps menu positions the way `0x4ce980` does. main_category: q.class.unwrap_or(auction_filter::ANY), sub_category: q.sub_class.unwrap_or(auction_filter::ANY), quality: q.quality.unwrap_or(auction_filter::ANY), @@ -944,6 +951,54 @@ fn drain_auction( mod tests { use super::*; + /// The tree off the player's own DBCs carries the reference's two gates (wow-re + /// auction-house §9.1/§9.2): the `0x807060` class flag, and `ItemSubClass.Flags & 0x200` for + /// the inventory-slot rows — which Shield does NOT carry, though it is Armor. + #[test] + fn the_tree_carries_the_reference_gates() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).unwrap(); + let classes = + crate::ui_items::ItemClasses(benilla_formats::load_item_classes(&mut chain).unwrap()); + let subclasses = crate::ui_items::ItemSubClasses( + benilla_formats::load_item_sub_classes(&mut chain).unwrap(), + ); + let tree = categories(Some(&classes), Some(&subclasses)); + let ids: Vec<(u32, bool)> = tree + .iter() + .map(|c| (c.class_id, c.has_subclass_filter)) + .collect(); + assert_eq!(ids, AUCTION_CLASSES.to_vec()); + + let weapon = &tree[0]; + assert_eq!(weapon.subclasses[0].sub_id, 0, "Axe leads, in file order"); + assert!( + !weapon + .subclasses + .iter() + .any(|s| [9, 11, 12, 17].contains(&s.sub_id)), + "the excluded weapon rows are not offered" + ); + assert!(weapon.subclasses.iter().all(|s| !s.has_inv_types)); + + let armor = &tree[1]; + let offering: Vec = armor + .subclasses + .iter() + .filter(|s| s.has_inv_types) + .map(|s| s.sub_id) + .collect(); + assert_eq!( + offering, + vec![0, 1, 2, 3, 4], + "not Shield, Libram, Idol, Totem" + ); + + // Consumable's flag is 0, yet its subclass list stays for the query's own scan. + assert!(!tree[3].has_subclass_filter); + assert!(!tree[3].subclasses.is_empty()); + } + /// The four buckets, and the one that matters: an expired auction the server has not swept yet /// arrives as an underflowed u32 and must read as Short, never as "Very Long". #[test] diff --git a/crates/benilla-app/src/ui_battlefield_score.rs b/crates/benilla-app/src/ui_battlefield_score.rs index b6fed6ce..435d5fe0 100644 --- a/crates/benilla-app/src/ui_battlefield_score.rs +++ b/crates/benilla-app/src/ui_battlefield_score.rs @@ -32,7 +32,8 @@ use crate::world_state_ui::WorldStateUiRes; /// `RequestBattlefieldScoreData`'s throttle — `0x4aa170`: `now + 0x1388`. const REQUEST_THROTTLE: Duration = Duration::from_millis(5000); -/// The last `MSG_PVP_LOG_DATA`, and the request stamp. +/// The last `MSG_PVP_LOG_DATA`, and the request stamp. Both die with the session +/// (`net::on_session_end`). #[derive(Resource, Default)] pub(crate) struct BattlefieldScoreboard { log: Option, @@ -181,7 +182,15 @@ mod net { /// Register the handler — called from [`super::BattlefieldScorePlugin`]. pub(super) fn register(app: &mut App) { - app.net_handler(SessionEventKind::PvpLogData, on_pvp_log_data); + app.net_handler(SessionEventKind::PvpLogData, on_pvp_log_data) + .net_handler(SessionEventKind::Disconnected, on_session_end); + } + + /// The board and the request stamp are zeroed at every login (wow-re + /// `battlefield-verb-family.md` §8: module init `0x4a9c40` zeroes the score scalars) — a + /// listener on the session end (a second handler on the kind, after the bridge's own teardown). + fn on_session_end(In(_): In, mut board: ResMut) { + *board = BattlefieldScoreboard::default(); } fn on_pvp_log_data(In(ev): In, mut board: ResMut) { @@ -255,4 +264,36 @@ mod tests { .collect(); assert_eq!(cols, ["Flags Returned"]); } + + /// **The board is zeroed at every login** (wow-re `battlefield-verb-family.md` §8: module + /// init `0x4a9c40`, from `InitializeGame`, zeroes the score scalars) — the last session's + /// `MSG_PVP_LOG_DATA` must not come back as the next character's scoreboard, and its request + /// stamp must not throttle that character's first ask. + #[test] + fn the_session_end_zeroes_the_scoreboard() { + let mut app = App::new(); + app.init_resource::(); + net::register(&mut app); + { + let mut board = app.world_mut().resource_mut::(); + board.apply(PvpLogData { + ended: true, + winner: Some(1), + rows: Vec::new(), + }); + board.last_request = Some(Instant::now()); + } + + crate::net::handlers::dispatch( + app.world_mut(), + vec![benilla_protocol::SessionEvent::Disconnected { + reason: "socket".into(), + end: benilla_protocol::SessionEnd::Lost, + }], + ); + + let board = app.world().resource::(); + assert!(board.log.is_none()); + assert!(board.last_request.is_none()); + } } diff --git a/crates/benilla-app/src/ui_chat/combat.rs b/crates/benilla-app/src/ui_chat/combat.rs index 664e5ff8..880396f9 100644 --- a/crates/benilla-app/src/ui_chat/combat.rs +++ b/crates/benilla-app/src/ui_chat/combat.rs @@ -1072,13 +1072,22 @@ pub(crate) fn power_word(script: &benilla_ui::script::UiScript, power: u32) -> O /// same ask-once name cache every other client-composed chat line waits on. `None` = not yet /// answered; the caller re-tries next frame, exactly as the reference's deferred-name queue /// (`DAT_00c4e208`, drained by the name-ready callback `0x6294b0`) replays its message. -pub(crate) fn object_name(guid: u64, names: &NameCache, commands: &NetCommands) -> Option { +/// +/// `unit` is the endpoint's descriptor when it is streamed: `0x6264e0` looks the guid up in the +/// object manager first and, for a unit, calls `GetUnitName` (`0x609210`) — which keys a pet's or a +/// companion's name off its descriptor, not its guid ([`NameCache::resolve_unit`]). +pub(crate) fn object_name( + guid: u64, + unit: Option<&ObjectStore>, + names: &NameCache, + commands: &NetCommands, +) -> Option { // Guid 0 = "the name is already in the fills" — no wire endpoint is ever guid 0, so the // sentinel costs nothing and is what lets `/chattest` drive the real drain with literal names. if guid == 0 { return None; } - names.resolve(guid, commands).map(str::to_owned) + names.resolve_unit(guid, unit, commands).map(str::to_owned) } // ────────────────────────────────── the families ────────────────────────────────────── diff --git a/crates/benilla-app/src/ui_chat/feed.rs b/crates/benilla-app/src/ui_chat/feed.rs index 91e428b7..f02926f3 100644 --- a/crates/benilla-app/src/ui_chat/feed.rs +++ b/crates/benilla-app/src/ui_chat/feed.rs @@ -1260,7 +1260,12 @@ pub(super) fn feed_chat( if guid == 0 { continue; } - match super::combat::object_name(guid, &names, &commands) { + match super::combat::object_name( + guid, + guids.0.get(&guid).and_then(|e| stores.get(*e).ok()), + &names, + &commands, + ) { Some(name) if slot == 0 => line.fills.attacker = name, Some(name) => line.fills.victim = name, None => wait = true, @@ -1281,7 +1286,12 @@ pub(super) fn feed_chat( } } super::combat::Named::Unit(guid) => { - match super::combat::object_name(guid, &names, &commands) { + match super::combat::object_name( + guid, + guids.0.get(&guid).and_then(|e| stores.get(*e).ok()), + &names, + &commands, + ) { Some(name) => line.fills.named = name, None => wait = true, } diff --git a/crates/benilla-app/src/ui_craft.rs b/crates/benilla-app/src/ui_craft.rs index 8d10832c..3b6b24b3 100644 --- a/crates/benilla-app/src/ui_craft.rs +++ b/crates/benilla-app/src/ui_craft.rs @@ -24,11 +24,12 @@ use benilla_formats::{ SPELL_EFFECT_ENCHANT_ITEM_TEMPORARY, SPELL_EFFECT_LEARN_SPELL, }; use benilla_protocol::messages::PLAYER_SKILL_SLOTS; +use benilla_protocol::{SessionEvent, SessionEventKind}; use benilla_ui::script::{CraftReagent, CraftRecipe, CraftState, CraftTooltip, UiScript}; use crate::entities::ItemDisplays; use crate::items::Items; -use crate::net::{NetCommands, ObjectStore, Objects, SelfPlayer}; +use crate::net::{NetCommands, NetHandlerApp, ObjectStore, Objects, SelfPlayer}; use crate::spell::{cast_target, CastCommit, CastLadder}; use crate::ui_action::{PlayerActions, Spells}; use crate::ui_items::{count_of, item_icon, InventoryScope}; @@ -47,7 +48,9 @@ use crate::ui_unit::UnitFeed; /// type** that opened it. Routed here by the opener's `EffectMiscValue[0] != 0` (byte-VERIFIED — /// wow-re `tradeskill` TU-A); that same misc value *is* the craft type (1 Beast Training · /// 3 Enchanting), which the client keeps at `ds:0xbdcfb8` and reads for both the window's admission -/// filter and its row comparator (decision 1124). Client-local state, no wire. +/// filter and its row comparator (decision 1124). Client-local state, no wire. Cleared by the Lua +/// close and by the session end ([`on_session_end`]) — a logout's fresh VM never runs the old one's +/// `OnHide`, so the close alone would carry the window into the next login. #[derive(Resource, Default)] pub(crate) struct CraftOpen { pub(crate) line: Option, @@ -58,6 +61,7 @@ pub(crate) struct UiCraftPlugin; impl Plugin for UiCraftPlugin { fn build(&self, app: &mut App) { + app.net_handler(SessionEventKind::Disconnected, on_session_end); app.init_resource::().add_systems( Update, (feed_craft.in_set(UnitFeed), drain_craft.after(UiInput)), @@ -65,6 +69,12 @@ impl Plugin for UiCraftPlugin { } } +/// The Craft window dies with the session — a listener on the session end (a second handler on +/// the kind, after the bridge's own teardown). +fn on_session_end(In(_): In, mut open: ResMut) { + *open = CraftOpen::default(); +} + /// The line's `(rank, max, bonus)` off the skill block — the Craft window bands difficulty on /// the EFFECTIVE skill (rank + bonuses), byte-VERIFIED unlike the TradeSkill window's raw rank /// (wow-re `tradeskill` TU-C). @@ -402,4 +412,29 @@ mod tests { teacher.effect_trigger_spell[1] = 999_999; assert_eq!(craft_tooltip(5149, &teacher), CraftTooltip::Spell(999_999)); } + + /// **The Craft window dies with the session** — the TradeSkill book's twin: only `CloseCraft` + /// cleared it, and a logout's fresh VM never runs the old one's `OnHide`, so the next login + /// fired `CRAFT_SHOW` into the new character's UI. + #[test] + fn the_session_end_closes_the_craft_window() { + let mut app = App::new(); + app.add_plugins(UiCraftPlugin); + { + let mut open = app.world_mut().resource_mut::(); + open.line = Some(333); + open.craft_type = 3; + } + + crate::net::handlers::dispatch( + app.world_mut(), + vec![benilla_protocol::SessionEvent::Disconnected { + reason: "socket".into(), + end: benilla_protocol::SessionEnd::Lost, + }], + ); + + let open = app.world().resource::(); + assert_eq!((open.line, open.craft_type), (None, 0)); + } } diff --git a/crates/benilla-app/src/ui_dialog_verbs.rs b/crates/benilla-app/src/ui_dialog_verbs.rs index 95f336d0..0cbc5d90 100644 --- a/crates/benilla-app/src/ui_dialog_verbs.rs +++ b/crates/benilla-app/src/ui_dialog_verbs.rs @@ -243,7 +243,9 @@ impl AreaSpiritHealer { } /// The three battleground queue slots (`0xb6e9d0`, stride `0x20`), each with the moment its -/// status landed — the clock every stamp in the slot is relative to. +/// status landed — the clock every stamp in the slot is relative to. Kept across an in-session +/// world enter (§8, `0x4a9db0`); zeroed whole at the session end, as the reference's module init +/// zeroes it at every login (`net::on_session_end`). #[derive(Resource, Default)] pub(crate) struct BattlefieldQueue { slots: [Option<(BattlefieldStatus, Instant)>; 3], @@ -1095,7 +1097,18 @@ mod net { .net_handler(K::AreaSpiritHealerTime, on_area_spirit_healer_time) .net_handler(K::BattlefieldStatus, on_battlefield_status) .net_handler(K::MeetingStoneSetQueue, on_meeting_stone) - .net_handler(K::MeetingStoneNotice, on_meeting_stone); + .net_handler(K::MeetingStoneNotice, on_meeting_stone) + .net_handler(K::Disconnected, on_session_end); + } + + /// The battleground queue is zeroed at every login (wow-re `battlefield-verb-family.md` §8: + /// module init `0x4a9c40`, from `InitializeGame` — the three slots, `[0x8457cc] = -1`, the + /// scalars), and vmangos never sends a clear for the queue of a player who logged out. A + /// listener on the session end (a second handler on the kind, after the bridge's own + /// teardown). The in-session world enter (`0x4a9db0`) is a different edge that KEEPS the slots, + /// and it does not come through here. + fn on_session_end(In(_): In, mut queue: ResMut) { + *queue = BattlefieldQueue::default(); } /// The pet trainer's question (decision 1963) — the talent wipe's twin @@ -2060,4 +2073,65 @@ mod tests { assert_eq!(pet.pending(), None); assert_eq!(pet.cost, 0); } + + /// **The queue is zeroed at every login, not kept across it** (wow-re + /// `battlefield-verb-family.md` §8): module init `0x4a9c40`, run from `InitializeGame`, + /// zeroes the three slots, the active index (`[0x8457cc] = -1`) and the scalars, and vmangos + /// never sends a clear for a queue whose player logged out. So a slot, the active map and the + /// instance clocks from the last session must not reach the next — only an in-session world + /// enter (`0x4a9db0`) keeps the slots, and that is `ui_battlefield`'s, untouched here. + #[test] + fn the_session_end_zeroes_the_battlefield_queue() { + let mut app = App::new(); + app.init_resource::(); + net::register(&mut app); + let now = Instant::now(); + { + let mut q = app.world_mut().resource_mut::(); + q.apply_at( + BattlefieldStatus { + slot: 0, + map_id: 489, + bracket: 0, + instance_id: 3, + status: 3, + time_ms: None, + in_progress: Some((90_000, 30_000)), + queued: None, + }, + now, + ); + q.apply_at( + BattlefieldStatus { + slot: 1, + map_id: 30, + bracket: 0, + instance_id: 0, + status: 1, + time_ms: Some(0), + in_progress: None, + queued: None, + }, + now, + ); + } + + crate::net::handlers::dispatch( + app.world_mut(), + vec![benilla_protocol::SessionEvent::Disconnected { + reason: "logged out".into(), + end: benilla_protocol::SessionEnd::LoggedOut, + }], + ); + + let mut q = app.world_mut().resource_mut::(); + assert!( + q.slots().iter().all(Option::is_none), + "all three slots empty" + ); + assert_eq!(q.active_map(), None); + assert_eq!(q.run_time_ms(now), 0); + assert_eq!(q.instance_expiration_ms(now), 0); + assert!(!q.take_score_dirty()); + } } diff --git a/crates/benilla-app/src/ui_items/net.rs b/crates/benilla-app/src/ui_items/net.rs index 9d7f6708..3a01b00c 100644 --- a/crates/benilla-app/src/ui_items/net.rs +++ b/crates/benilla-app/src/ui_items/net.rs @@ -14,7 +14,20 @@ use crate::ui_loot::LootLatch; /// Register the handlers — called from [`super::UiItemsPlugin`]. pub(super) fn register(app: &mut App) { app.net_handler(SessionEventKind::InventoryFailure, on_inventory_failure) - .net_handler(SessionEventKind::OpenContainer, on_open_container); + .net_handler(SessionEventKind::OpenContainer, on_open_container) + .net_handler(SessionEventKind::Disconnected, on_session_end); +} + +/// The pending item locks die with the socket — a listener on the session end (a second handler on +/// the kind, after the bridge's own teardown). The unlock transitions still queued for the feed go +/// too: they name the old session's slots, and the next session's VM starts with nothing locked. +fn on_session_end( + In(_): In, + mut pending: ResMut, + mut lock_cleared: ResMut, +) { + pending.clear_session(); + lock_cleared.0.clear(); } /// The containers the server told us to open (`SMSG_OPEN_CONTAINER`), by container id, waiting @@ -165,6 +178,46 @@ mod tests { ); assert_eq!(latch.0, None); } + /// **The item locks die with the session.** The lock is item-object state in the reference + /// (`item+0x314`), and the objects are rebuilt at every world entry — but ours is a resource, + /// and only a resolving field update, a failure or the loot close cleared it. A lockbox opened + /// in the frame the socket died never saw any of those, so it stayed greyed and locked for + /// the whole next session. Driven through the real registration. + #[test] + fn the_session_end_drops_every_item_lock() { + let mut app = App::new(); + app.init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::(); + register(&mut app); + app.world_mut() + .resource_mut::() + .add([(0, 3, 0x4000_0000_0000_0007, 1)]); + app.world_mut() + .resource_mut::() + .0 + .push((0, 5)); + let epoch = app.world().resource::().epoch(); + + crate::net::handlers::dispatch( + app.world_mut(), + vec![SessionEvent::Disconnected { + reason: "socket".into(), + end: benilla_protocol::SessionEnd::Lost, + }], + ); + + let pending = app.world().resource::(); + assert!(pending.is_empty() && !pending.contains(0, 3)); + assert_ne!(pending.epoch(), epoch, "the feed's gate sees the set move"); + assert!( + app.world().resource::().0.is_empty(), + "no ITEM_LOCK_CHANGED for the old session's slots" + ); + } } #[cfg(test)] diff --git a/crates/benilla-app/src/ui_party/feed.rs b/crates/benilla-app/src/ui_party/feed.rs index 15da2830..ff999620 100644 --- a/crates/benilla-app/src/ui_party/feed.rs +++ b/crates/benilla-app/src/ui_party/feed.rs @@ -1126,9 +1126,10 @@ fn test_apply_local( ) -> bool { match req { PartyRequest::Leave => { - // A local disband: the real path's all-zero list resets everything, sandbox - // flag included — `/partytest` starts a fresh one. - *group = GroupState::default(); + // A local disband: exactly what the real path's all-zero list resets — the group + // facts, sandbox flag included (`/partytest` starts a fresh one), and not the + // session's tickets. + group.leave_group(); true } PartyRequest::UninviteUnit(token) => { diff --git a/crates/benilla-app/src/ui_party/mod.rs b/crates/benilla-app/src/ui_party/mod.rs index e6926d42..f62e5bdc 100644 --- a/crates/benilla-app/src/ui_party/mod.rs +++ b/crates/benilla-app/src/ui_party/mod.rs @@ -95,7 +95,8 @@ pub struct GroupState { /// A lockout is per-CHARACTER, not per-group, so this is the one field here that is not a /// group fact. It lives here anyway for the reason that matters: it is per-SESSION state that /// must die with the socket, and [`Self::clear_session`] is that guarantee — a second - /// resource would be a second thing to remember to clear. + /// resource would be a second thing to remember to clear. Leaving the group does NOT clear it + /// ([`Self::leave_group`]); nor the four ticket fields below. pub saved_instances: Vec, /// **How many times the server has answered** — one per `SMSG_RAID_INSTANCE_INFO`. A TICKET, /// not a flag (the [`Self::ready_check`] shape), and the difference is load-bearing: the @@ -164,7 +165,7 @@ impl GroupState { } } if leaving { - *self = GroupState::default(); + self.leave_group(); return lines; } // Our own party→raid conversion (or joining straight into a raid). INTERIM like the @@ -179,8 +180,8 @@ impl GroupState { // stale marks on screen: the icons are drawn from this board, and nothing else clears it // short of a `0x321` for each slot, which the server does not send. // - // A full disband needs no arm here — `leaving` above replaces the whole state, and - // `[u64; 8]::default()` is already the empty board. 1820 recorded this as "stale icons + // A full disband needs no arm here — `leaving` above resets every group fact + // ([`Self::leave_group`]), and the board is one. 1820 recorded this as "stale icons // after a disband", which was wider than the truth; the conversion is the real gap. if group_type != 1 && self.group_type == 1 { self.raid_targets = [0; 8]; @@ -395,6 +396,54 @@ impl GroupState { .map_or(0, |i| i as u8 + 1) } + /// The all-zero `SMSG_GROUP_LIST` — we are no longer in a group. Resets the **group facts** + /// and nothing else. + /// + /// This used to be `*self = GroupState::default()`, which is [`Self::clear_session`]'s job, + /// and it took the per-SESSION tickets down with the roster: the feed fires on edges of those + /// tickets, so every leave, kick or disband fired a `READY_CHECK` (a popup naming no leader, + /// and a 30 s request armed for a check nobody started) and an `UPDATE_INSTANCE_INFO` over an + /// emptied lockout list. In the reference `READY_CHECK` has one fire site, the + /// `MSG_RAID_READY_CHECK` open handler (decision 1989), and a lockout belongs to the character. + /// + /// **The destructure is exhaustive on purpose** — no `..`: a field added to [`GroupState`] + /// does not compile until someone decides here which side of that line it is on. The defaulted + /// reset that caused this bug classified every new field silently. + pub(super) fn leave_group(&mut self) { + let GroupState { + // Group facts: the roster, its side-state, the sandbox (the wire always wins). + in_group, + group_type, + own_flags, + members, + leader, + loot, + stats, + raid_targets, + test, + // An invite is an offer into a group we are not in yet; it is cleared with the group + // as it always was here (UNVERIFIED either way — vmangos sends invitees nothing on a + // disband, so the case needs a leave to race an invite popup). + pending_invite, + // Session tickets — the feed's edges. Kept; `clear_session` is where they die. + saved_instances: _, + saved_instances_answers: _, + ready_check: _, + ready_check_requests: _, + ready_check_answers: _, + } = self; + *in_group = false; + *group_type = 0; + *own_flags = 0; + members.clear(); + *leader = 0; + *loot = None; + stats.clear(); + *raid_targets = [0; 8]; + *test = false; + *pending_invite = None; + } + /// Session teardown (decision 0065's lifecycle): everything resets with the socket. pub fn clear_session(&mut self) { *self = GroupState::default(); @@ -491,6 +540,59 @@ mod tests { assert!(g.saved_instances.is_empty()); } + /// **Leaving the group is not the socket dying.** The all-zero `SMSG_GROUP_LIST` clears the + /// roster — and ONLY the roster. The ready-check and saved-instance fields are per-session + /// tickets the feed fires events on edges of: resetting them here made every leave, kick or + /// disband look like a new `READY_CHECK` (a popup with no leader, and a stale 30 s request + /// armed) and a fresh `UPDATE_INSTANCE_INFO` carrying an emptied lockout list — a blank Raid + /// Info panel. In the reference `READY_CHECK` has one fire site, the `MSG_RAID_READY_CHECK` + /// open handler (decision 1989); a lockout is the character's, not the group's. + #[test] + fn leaving_the_group_keeps_the_session_tickets() { + let lockout = benilla_protocol::messages::RaidInstanceEntry { + map: 409, + reset: 86_400, + instance: 7, + }; + let mut g = GroupState { + in_group: true, + leader: 0xA11CE, + members: vec![member("Alice", 0xA11CE)], + ..Default::default() + }; + g.raid_targets[0] = 0xA11CE; + g.apply_ready_check_request(false); + g.apply_raid_instance_info(Vec::new()); + g.apply_raid_instance_info(vec![lockout]); + assert_eq!((g.ready_check, g.ready_check_requests), (1, 1)); + assert_eq!(g.saved_instances_answers, 2); + + g.apply_list(0, 0, vec![], 0, None); + + // The group facts are gone … + assert!(!g.in_group); + assert!(g.members.is_empty()); + assert_eq!(g.leader, 0); + assert_eq!(g.raid_targets, [0; 8]); + // … the session's tickets are not: no edge for the feed to fire on. + assert_eq!( + (g.ready_check, g.ready_check_requests), + (1, 1), + "no READY_CHECK edge" + ); + assert_eq!(g.saved_instances_answers, 2, "no UPDATE_INSTANCE_INFO edge"); + assert_eq!( + g.saved_instances, + vec![lockout], + "the lockouts are the character's" + ); + + // The socket's end is still the one place they die. + g.clear_session(); + assert_eq!((g.ready_check, g.saved_instances_answers), (0, 0)); + assert!(g.saved_instances.is_empty()); + } + /// The ungated diff (0440 byte law): a FIRST roster prints joins for everyone already /// there; later rosters diff both ways. /// The open form's two arms (decision 1989): the leader's own echo neither prints nor pops; diff --git a/crates/benilla-app/src/ui_pet/drain.rs b/crates/benilla-app/src/ui_pet/drain.rs index 300c4ab8..56823c9f 100644 --- a/crates/benilla-app/src/ui_pet/drain.rs +++ b/crates/benilla-app/src/ui_pet/drain.rs @@ -172,17 +172,13 @@ pub(super) fn drain_pet_actions( }); } for slot in toggles { - let Some(entry) = slot_entry(&bar, slot).filter(|e| e.autocast_allowed()) else { + let Some(flipped) = toggle_slot_autocast(&mut bar, slot) else { continue; }; - // The client flips bit 30 in the slot word IN PLACE and sends the whole new word — it is - // not a "set autocast to X for spell Y" verb (wow-re §10.2, `0x4bcbff`/`0x4bcc17`). The - // server reads the direction back out of the type byte it arrives in. - let flipped = entry.with_autocast(!entry.autocast_on()); debug!( "ui_pet: autocast {} for spell {} (slot {slot})", flipped.autocast_on(), - entry.action() + flipped.action() ); let _ = commands.0.send(ClientCommand::PetSetAction { pet_guid, @@ -191,9 +187,6 @@ pub(super) fn drain_pet_actions( // `slot_entry` already rejected 0. entries: vec![(slot - 1, flipped.packed)], }); - if let Some(e) = slot_entry_mut(&mut bar, slot) { - *e = flipped; - } } for _ in 0..stops { if stop_pet_attack(&mut bar, &commands) { @@ -218,6 +211,38 @@ pub(super) fn drain_pet_actions( } } +/// `TogglePetAutocast`'s local half, `0x4bcbb0` — flip the slot's autocast bit and write it back, +/// returning the new word for the send, or `None` when the reference no-ops silently (a slot out +/// of range, or a word that is not autocast-ALLOWED, bit 31 — `0x4bcbf1`). +/// +/// The client flips bit 30 in the slot word IN PLACE and sends the whole new word — it is not a +/// "set autocast to X for spell Y" verb (wow-re §10.2, `0x4bcbff`/`0x4bcc17`). The server reads the +/// direction back out of the type byte it arrives in. +/// +/// Then `0x4bcc19` calls `0x4bd190(&bar[slot])` — the propagation into the **spellbook**: for a +/// spell-type slot (type 1), scan the raw pet-spell array **backwards** for the entry equal to the +/// slot under `& 0x3FFFFFFF` and copy the slot's FULL word into it. The pet book renders from that +/// array, so without the copy a bar toggle left the Pet tab's ring stale until the next +/// `SMSG_PET_SPELLS`. The book→bar direction is `ui_pet_book::flip_autocast` (decision 1032). +pub(super) fn toggle_slot_autocast(bar: &mut PetBar, slot: u32) -> Option { + let entry = slot_entry(bar, slot).filter(|e| e.autocast_allowed())?; + let flipped = entry.with_autocast(!entry.autocast_on()); + *slot_entry_mut(bar, slot)? = flipped; + if flipped.kind() == 1 { + let key = flipped.packed & 0x3FFF_FFFF; + if let Some(book) = bar + .spells + .spells + .iter_mut() + .rev() + .find(|w| w.packed & 0x3FFF_FFFF == key) + { + *book = flipped; + } + } + Some(flipped) +} + /// `PetStopAttack`'s **core**, `0x4bd650` — call the pet off, and the only thing besides a new pet /// that puts the Attack button out. /// diff --git a/crates/benilla-app/src/ui_pet/mod.rs b/crates/benilla-app/src/ui_pet/mod.rs index fdf10334..5a2a292a 100644 --- a/crates/benilla-app/src/ui_pet/mod.rs +++ b/crates/benilla-app/src/ui_pet/mod.rs @@ -65,6 +65,8 @@ mod tests; /// /// `spells.pet_guid == 0` is "there is no pet bar", and it is the single test: the teardown packet /// carries exactly that and nothing else. +/// +/// The session end resets it the same way (`net::on_session_end`): a dropped socket sends no teardown. #[derive(Resource, Default)] pub(crate) struct PetBar { /// The last `SMSG_PET_SPELLS` in full, with `SMSG_PET_MODE`'s state edits folded in. diff --git a/crates/benilla-app/src/ui_pet/net.rs b/crates/benilla-app/src/ui_pet/net.rs index d1e0b4b5..ba71a87f 100644 --- a/crates/benilla-app/src/ui_pet/net.rs +++ b/crates/benilla-app/src/ui_pet/net.rs @@ -42,7 +42,17 @@ pub(super) fn register(app: &mut App) { .net_handler(K::PetNameInvalid, on_refusal_line) .net_handler(K::PetBroken, on_refusal_line) .net_handler(K::PetActionSound, on_action_sound) - .net_handler(K::PetDismissSound, on_dismiss_sound); + .net_handler(K::PetDismissSound, on_dismiss_sound) + .net_handler(K::Disconnected, on_session_end); +} + +/// The pet bar dies with the socket — a listener on the session end (a second handler on the kind, +/// after the bridge's own teardown). The zero-guid `SMSG_PET_SPELLS` is the only in-session +/// teardown, and a dropped socket never sends one: without this the next session kept the old +/// pet's bar, pressed `CMSG_PET_ACTION` at a guid that no longer exists, and the stable read a live +/// pet. The same reset as the zero guid's, cooldown store included. +fn on_session_end(In(_): In, mut bar: ResMut) { + *bar = PetBar::default(); } /// The pet action bar (decision 0982) — server-authoritative, so PET_SPELLS is a wholesale diff --git a/crates/benilla-app/src/ui_pet/tests.rs b/crates/benilla-app/src/ui_pet/tests.rs index 27af3220..cd9aeb3e 100644 --- a/crates/benilla-app/src/ui_pet/tests.rs +++ b/crates/benilla-app/src/ui_pet/tests.rs @@ -959,3 +959,79 @@ fn a_cooldown_alone_fires_the_cooldown_event_and_not_the_bar_update() { ); assert_eq!(enable, 1); } + +/// **A bar toggle reaches the spellbook** (wow-re `pet-action-bar-api.md` §10.2): after the +/// local write-back, `0x4bcc19` calls `0x4bd190(&bar[slot])`, which scans the raw pet-spell +/// array backwards for the entry equal to the slot under `& 0x3FFFFFFF` and copies the slot's +/// FULL word into it. The book renders from that array, so without the copy the Pet tab kept +/// the old autocast ring until the next `SMSG_PET_SPELLS`. The book→bar direction is +/// `ui_pet_book::flip_autocast` (decision 1032); this is its twin. +#[test] +fn a_bar_autocast_toggle_reaches_the_pet_spellbook() { + const CLAW: u32 = 16827; + const GROWL: u32 = 2649; + let mut bar = PetBar { + spells: PetSpells { + pet_guid: 0x2A, + ..Default::default() + }, + ..Default::default() + }; + bar.spells.bar[3] = packed(CLAW, PET_ACT_ENABLED); // autocast allowed + on + bar.spells.spells = vec![ + packed(GROWL, PET_ACT_ENABLED), + packed(CLAW, PET_ACT_ENABLED), + ]; + + let flipped = toggle_slot_autocast(&mut bar, 4).expect("an autocastable slot toggles"); + assert!(!flipped.autocast_on()); + assert!(!bar.spells.bar[3].autocast_on(), "the bar slot flipped"); + assert!( + !bar.spells.spells[1].autocast_on(), + "…and the book's CLAW entry got the slot's whole word" + ); + assert_eq!(bar.spells.spells[1].packed, flipped.packed); + assert!( + bar.spells.spells[0].autocast_on(), + "a different spell's book entry is untouched" + ); + + // And back on: the copy is the full word, both directions. + toggle_slot_autocast(&mut bar, 4); + assert!(bar.spells.spells[1].autocast_on()); + + // `0x4bcbf1`: a word without bit 31 aborts before anything is written, book included. + bar.spells.bar[5] = packed(3025, PET_ACT_PASSIVE); + bar.spells.spells.push(packed(3025, PET_ACT_PASSIVE)); + assert!(toggle_slot_autocast(&mut bar, 6).is_none()); + assert_eq!(bar.spells.spells[2], packed(3025, PET_ACT_PASSIVE)); +} + +/// **The pet bar dies with the session.** Only the zero-guid `SMSG_PET_SPELLS` cleared it, and a +/// dropped socket never sends one — so the next session kept the old pet's bar: presses went out +/// as `CMSG_PET_ACTION` to a guid that no longer exists, and the stable read `has_live_pet`. +#[test] +fn the_session_end_tears_the_pet_bar_down() { + let mut app = bevy::prelude::App::new(); + app.add_plugins(UiPetPlugin); + { + let mut bar = app.world_mut().resource_mut::(); + bar.spells.pet_guid = 0x2A; + bar.spells.bar[3] = packed(16827, PET_ACT_ENABLED); + } + + crate::net::handlers::dispatch( + app.world_mut(), + vec![benilla_protocol::SessionEvent::Disconnected { + reason: "socket".into(), + end: benilla_protocol::SessionEnd::Lost, + }], + ); + + let bar = app.world().resource::(); + assert_eq!( + bar.spells.pet_guid, 0, + "no pet bar carried into the next session" + ); + assert!(bar.spells.bar[3].is_empty()); +} diff --git a/crates/benilla-app/src/ui_pet/unit.rs b/crates/benilla-app/src/ui_pet/unit.rs index ca449b95..abdd650d 100644 --- a/crates/benilla-app/src/ui_pet/unit.rs +++ b/crates/benilla-app/src/ui_pet/unit.rs @@ -106,7 +106,9 @@ pub(super) fn feed_pet_unit( .then(|| pet.store(pet_guid)) .flatten() .map(|store| { - let name = names.resolve(pet_guid, &commands).map(str::to_string); + let name = names + .resolve_unit(pet_guid, Some(store), &commands) + .map(str::to_string); // No `ChrClasses.dbc` here, and none needed: a pet is a UNIT, so the reference's // own TYPEMASK_PLAYER gate refuses it before the class byte is ever read. Passing // the table could not change the answer. diff --git a/crates/benilla-app/src/ui_petition/mod.rs b/crates/benilla-app/src/ui_petition/mod.rs index 92740f2b..29292e8a 100644 --- a/crates/benilla-app/src/ui_petition/mod.rs +++ b/crates/benilla-app/src/ui_petition/mod.rs @@ -402,6 +402,16 @@ impl PetitionState { self.open = None; } + /// The session end: the open charter, the in-flight sign and the undrained lines are the old + /// session's and go with it — silently, with **no** decline (the socket is gone, and a + /// decline is the close verb's, not the teardown's). The record cache stays: it is keyed by + /// the server's petition id, and the world-enter release already re-arms its asks. + fn clear_session(&mut self) { + self.open = None; + self.signing = false; + self.lines.clear(); + } + /// The open charter's item guid — what `SignPetition` / `OfferPetition` / `RenamePetition` /// address. `None` = nothing open, and the verb is dropped rather than sent against a guess. fn open_item(&self) -> Option { @@ -456,7 +466,22 @@ pub(crate) mod net { .net_handler(K::PetitionSignResults, on_sign_results) .net_handler(K::TurnInPetitionResults, on_turn_in_results) .net_handler(K::PetitionDeclined, on_declined) - .net_handler(K::PetitionRenamed, on_renamed); + .net_handler(K::PetitionRenamed, on_renamed) + .net_handler(K::Disconnected, on_session_end); + } + + /// The charter window and the registrar die with the socket — a listener on the session end + /// (a second handler on the kind, after the bridge's own teardown). Without it the next + /// login's fresh VM saw `None → Some` and fired `PETITION_SHOW` for the old session's charter, + /// and closing that ghost declined it on the wire. The registrar's walk-away guard cannot + /// stand in: it measures from a self player, and there is none until the next world entry. + fn on_session_end( + In(_): In, + mut petition: ResMut, + mut registrar: ResMut, + ) { + petition.clear_session(); + registrar.close(); } /// The registrar's two `UNIT_NPC_FLAGS` gates are on LIVE NPC state rather than on the @@ -847,4 +872,44 @@ mod tests { ); assert_eq!(open_title(&petition), Some("Second")); } + + /// **The charter window and the in-flight sign die with the session.** Only the Lua close, + /// a sign result or a turn-in cleared `open`, and a logout's fresh VM never runs the old + /// one's `OnHide` — so the next login's feed saw `None → Some` and fired `PETITION_SHOW` into + /// the new character's UI, and closing that ghost put `MSG_PETITION_DECLINE` on the wire for + /// the old session's charter. Driven through the real registration. + #[test] + fn the_session_end_closes_the_charter_and_forgets_the_sign() { + let (commands, _rx) = commands(); + let mut app = App::new(); + app.add_plugins(UiPetitionPlugin); + { + let mut petition = app.world_mut().resource_mut::(); + petition.show(signatures(0x99, 0xaa, 7, &[0xbb]), &commands); + petition.signing = true; + } + app.world_mut() + .resource_mut::() + .open(&show_list(0x2a1f, &[(1000, 1)]), Some(REGISTRAR_NPC_FLAGS)); + + crate::net::handlers::dispatch( + app.world_mut(), + vec![benilla_protocol::SessionEvent::Disconnected { + reason: "socket".into(), + end: benilla_protocol::SessionEnd::Lost, + }], + ); + + assert_eq!(app.world().resource::().npc(), None); + let petition = app.world().resource::(); + assert_eq!( + petition.open_item(), + None, + "no charter carried into the next login" + ); + assert!( + !petition.signing, + "no sign of the old session still in flight" + ); + } } diff --git a/crates/benilla-app/src/ui_quest/mod.rs b/crates/benilla-app/src/ui_quest/mod.rs index 69a5b8bf..0fec4ab6 100644 --- a/crates/benilla-app/src/ui_quest/mod.rs +++ b/crates/benilla-app/src/ui_quest/mod.rs @@ -55,12 +55,24 @@ pub(crate) struct QuestGiver { pub(crate) npc: Option, /// The open panel's wire view. pub(crate) view: Option, - /// The trailing `u32` of the last `SMSG_QUESTGIVER_QUEST_DETAILS` - /// ([`benilla_protocol::messages::QuestDetails::auto_finish`]) — the reference's `0xbe0824`, - /// a **latch** written by the DETAILS handler rather than a field of the open view, because - /// that is the shape the binary has and because its one reader ([`end_quest_session`]) runs - /// whichever panel is up. Non-zero suppresses the giver re-open on close (decision 1738). - pub(crate) detail_flag: u32, + /// The reference's `0xbe0824` — a **latch** written by THREE panel packets, not a field of + /// the open view: `SMSG_QUESTGIVER_QUEST_DETAILS`' trailing `u32` (publisher `0x500ef0`, + /// `0x50101a`), `SMSG_QUESTGIVER_OFFER_REWARD`'s `u32` after the reward text (the same + /// publisher), and `SMSG_QUESTGIVER_REQUEST_ITEMS`' `closeOnCancel` (`0x501070`, `0x50111a`); + /// zeroed only by the world-enter reset `0x500af0` (wow-re `quest-share-flow.md` §5, + /// `quest-material-reward-spell-bindings.md` §3.2). The greeting packet leaves it alone. Its + /// one reader is `DeclineQuest`'s fork ([`decline_leg`]), where non-zero diverts a plain + /// unit's decline from the giver re-open to the silent teardown — how the server says "this + /// panel was not reached through a menu, there is nothing to go back to" (vmangos sends 1 on + /// every DETAILS and OFFER_REWARD, and `closeOnCancel` = 1 on a single-quest auto-open, + /// 0 on the `COMPLETE_QUEST` path, `QuestHandler.cpp:390-395`). + pub(crate) close_on_cancel: u32, + /// The reference's "already acted" latch `0xbe0844`: set by every acting leg of + /// `AcceptQuest` (`0x5013c9`) and `DeclineQuest` (`0x50147a`/`0x5014d5`/`0x501549`), refusing + /// both verbs while set (`0x5013a8`, `0x5013fe`); cleared by each panel publish (`0x500bd0`, + /// `0x500d91` — our [`Self::open`]). It matters because a re-opening decline does not tear + /// the window down: the panel stays up until the server's answer replaces it. + pub(crate) acted: bool, /// Per-guid dialog status (`SMSG_QUESTGIVER_STATUS`) — the `!`/`?` marker's value, stored for a /// later world-marker slice. statuses: HashMap, @@ -94,6 +106,8 @@ impl QuestGiver { pub(crate) fn open(&mut self, npc: u64, view: QuestView) { self.npc = Some(npc); self.view = Some(view); + // `0x500bd0` clears the already-acted latch on every panel publish (`0x500d91`). + self.acted = false; } /// Whether a quest window is currently open (a predicate for callers + the module tests). @@ -212,6 +226,8 @@ impl QuestGiver { self.clear(); self.statuses.clear(); self.messages.clear(); + self.close_on_cancel = 0; + self.acted = false; } } @@ -616,60 +632,84 @@ fn feed_quest( *last_npc = giver.npc; } -/// Drain the Lua intents: the greeting-row selects (map to the row's quest id → QUERY_QUEST for an -/// available quest, COMPLETE_QUEST for an active one) and the button actions (Accept/Continue/Reward -/// → the matching CMSG; Close → a local clear, no packet). -/// **The quest session's one end** — the reference's `0x501130`, which every way out of the -/// questgiver window funnels through: `DeclineQuest()`, `CloseQuest()` (ESC, the window's own -/// OnHide), the walk-away watchdog and the leave-world teardown are four of its eleven callers, and -/// they all do the same thing. That is why benilla models one [`QuestAction::Close`] and not the -/// `Decline`/`Close` pair 1733 briefly split it into: two Lua verbs, one routine (decision 1738, -/// VERIFIED by the wow-re §5 dispatched for 1733). +/// `CloseQuest()` — the teardown `0x501130(0,1)` its binding `0x501a10` calls and nothing else +/// (wow-re `system/ui/scratch/quest-share-flow.md` §4.1): a PLAYER source — a party member whose +/// shared quest we are walking away from — is answered `MSG_QUEST_PUSH_RESULT{DECLINE_QUEST}`; +/// every other source is closed **network-silently**. There is no giver re-open here: that lives +/// only in `DeclineQuest`'s fork ([`decline_leg`]). 1738 put the two verbs on one routine and so +/// re-opened an NPC's list on ESC, which is the reason a multi-quest greeting could not be closed. /// -/// It sends, and what it sends depends on **the giver's object type**, not on which button was -/// pressed: -/// -/// - **A player** — a party member whose shared quest we are turning down. The answer they are -/// waiting on: `MSG_QUEST_PUSH_RESULT{sharerGuid, DECLINE_QUEST}`. This is the only verdict the -/// client ever originates besides `BUSY`. -/// - **A unit** — an ordinary questgiver, and the reference **re-opens its list**: -/// `CMSG_GOSSIP_HELLO` for a gossip-flagged NPC, `CMSG_QUESTGIVER_HELLO` otherwise. Declining a -/// quest putting you back in the NPC's menu is not a courtesy the server does; it is this send. -/// benilla asserted the opposite in `QuestFrame.xml` ("vanilla's client-side decline sends no -/// packet") from 0088 until 1738 refuted it at the bytes. -/// -/// `detail_flag` suppresses the unit re-open when non-zero — the trailing `u32` of -/// `SMSG_QUESTGIVER_QUEST_DETAILS` ([`QuestDetails::auto_finish`]), whose only reader in the whole -/// image is this routine. `npc_flags` is `None` when the giver is not a streamed unit, which is -/// also how an item giver (0664) and a player fall out of the unit branch. -fn end_quest_session(npc: u64, detail_flag: u32, npc_flags: Option, commands: &NetCommands) { +/// The PLAYER test is the guid's shape, as in [`QuestGiver::walk_away_send`]; the reference's +/// `0x468460(typemask 0x10)` also requires the sharer to resolve, which a sharer out of view does +/// not — a case the range guard closes on its own first. +fn close_quest(npc: u64, commands: &NetCommands) { if benilla_protocol::guid::is_player(npc) { - debug!("ui_quest: declining {npc:#x}'s shared quest"); + debug!("ui_quest: closing {npc:#x}'s shared quest — declining it"); let _ = commands.0.send(ClientCommand::QuestPushResult { sharer: npc, msg: QuestShareMsg::DECLINE_QUEST, }); - return; - } - let Some(flags) = npc_flags else { - // Not a streamed unit: an item giver, or an NPC that left view under the open window. - // Nothing to re-open and nobody to answer. - debug!("ui_quest: close on non-unit giver {npc:#x} (no packet)"); - return; - }; - if detail_flag != 0 { - debug!("ui_quest: close on {npc:#x} — detail flag {detail_flag} suppresses the re-open"); - return; - } - if flags & crate::target::cursor_mode::npc_flags::GOSSIP != 0 { - debug!("ui_quest: close on {npc:#x} — re-opening the gossip menu"); - let _ = commands.0.send(ClientCommand::GossipHello { guid: npc }); - } else { - debug!("ui_quest: close on {npc:#x} — re-opening the quest list"); - let _ = commands.0.send(ClientCommand::QuestgiverHello { npc }); } } +/// What `DeclineQuest()` does — the leg of its core `0x5013f0` a source takes. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum DeclineLeg { + /// The already-acted latch is set (`0x5013fe`), or the source does not resolve (`0x50142c`): + /// nothing at all — no send, no teardown. + Nothing, + /// The teardown `0x501130(0,1)` (`0x5014aa`): answers a PLAYER sharer, closes the window. + Teardown, + /// A gossip-flagged unit (`0x50143b`-`0x501474`): `CMSG_GOSSIP_HELLO`; the window stays. + GossipHello, + /// A plain unit (`0x5014e4`-`0x501526`): `CMSG_QUESTGIVER_HELLO`; the window stays. + QuestgiverHello, +} + +/// **`DeclineQuest`'s fork** — its binding `0x501d30` calls `0x5013f0`, the one routine that +/// re-opens a giver (wow-re `quest-share-flow.md` §6, read at the bytes `0x5013f0`-`0x501553`), +/// tested in this order: +/// +/// 1. the already-acted latch `0xbe0844` set, or the source unresolvable → nothing; +/// 2. a UNIT whose `UNIT_NPC_FLAGS` carries GOSSIP → `CMSG_GOSSIP_HELLO` — **before** the +/// `0xbe0824` test, so a gossip NPC is re-opened whatever the latch says; +/// 3. an ITEM, a PLAYER, or `0xbe0824 != 0` → the teardown `0x501130(0,1)`, which answers a +/// sharer and is silent otherwise; +/// 4. a GAMEOBJECT → its interact virtual `[vtbl+0x60]` (`0x5014d0`); +/// 5. any other unit → `CMSG_QUESTGIVER_HELLO`, which re-opens the quest list. +/// +/// Legs 2, 4 and 5 set `0xbe0844` and do NOT tear the window down: the server's answer replaces +/// the panel. **Leg 4 is modelled as the teardown**: wow-re pinned the call site as the generic +/// interact virtual but not `CGGameObject_C`'s override (its §9), so what goes on the wire is +/// unknown; a silent close is the one outcome that cannot leave a dead window up. +/// +/// `npc_flags` is the giver's live `UNIT_NPC_FLAGS`, `None` when it does not resolve to a streamed +/// object. An item giver (0664) is taken by its guid's shape: it lives in our bags, where the +/// reference always resolves it. A player is taken by shape too, as in [`close_quest`]. +fn decline_leg(npc: u64, acted: bool, close_on_cancel: u32, npc_flags: Option) -> DeclineLeg { + use benilla_protocol::guid; + if acted { + return DeclineLeg::Nothing; + } + if guid::is_player(npc) || guid::is_item(npc) { + return DeclineLeg::Teardown; + } + let Some(flags) = npc_flags else { + return DeclineLeg::Nothing; + }; + let unit = guid::is_creature_or_pet(npc); + if unit && flags & crate::target::cursor_mode::npc_flags::GOSSIP != 0 { + return DeclineLeg::GossipHello; + } + if close_on_cancel != 0 || !unit { + return DeclineLeg::Teardown; + } + DeclineLeg::QuestgiverHello +} + +/// Drain the Lua intents: the greeting-row selects (map to the row's quest id → QUERY_QUEST for an +/// available quest, COMPLETE_QUEST for an active one) and the button actions (Accept/Continue/Reward +/// → the matching CMSG; Decline → [`decline_leg`]; Close → [`close_quest`]). fn drain_quest( script: Option>, mut giver: ResMut, @@ -687,7 +727,7 @@ fn drain_quest( script.take_quest_actions(); return; }; - // The giver's live `UNIT_NPC_FLAGS`, for [`end_quest_session`]'s gossip/quest-list fork. + // The giver's live `UNIT_NPC_FLAGS`, for [`decline_leg`]'s gossip/quest-list fork. // `None` for anything that is not a streamed unit — an item giver, a player, a despawn. let npc_flags = index .0 @@ -736,11 +776,35 @@ fn drain_quest( for action in script.take_quest_actions() { match action { QuestAction::Close => { - end_quest_session(npc, giver.detail_flag, npc_flags, &commands); + close_quest(npc, &commands); giver.clear(); } + QuestAction::Decline => { + let leg = decline_leg(npc, giver.acted, giver.close_on_cancel, npc_flags); + debug!("ui_quest: decline on {npc:#x} — {leg:?}"); + match leg { + DeclineLeg::Nothing => {} + DeclineLeg::Teardown => { + close_quest(npc, &commands); + giver.clear(); + } + DeclineLeg::GossipHello => { + let _ = commands.0.send(ClientCommand::GossipHello { guid: npc }); + giver.acted = true; + } + DeclineLeg::QuestgiverHello => { + let _ = commands.0.send(ClientCommand::QuestgiverHello { npc }); + giver.acted = true; + } + } + } QuestAction::Accept => { + // `0x501380` refuses while the already-acted latch is set (`0x5013a8`). + if giver.acted { + continue; + } if let Some(quest) = view_quest { + giver.acted = true; let _ = commands .0 .send(ClientCommand::QuestgiverAccept { npc, quest }); @@ -1083,15 +1147,26 @@ mod tests { // ── The party share's one client-originated verdict (decision 1733) ────────────────────────── - /// Run `lua` against a quest window open on `giver`, and return what the drain sent. + /// Run `lua` against a quest window open on `giver` with the `0xbe0824` latch at + /// `close_on_cancel`, and return what the drain sent. /// `npc_flags` seats the giver as a streamed unit with those `UNIT_NPC_FLAGS`; `None` leaves it /// unstreamed (an item giver, or an NPC that left view). fn drain_after( giver: u64, npc_flags: Option, - detail_flag: u32, + close_on_cancel: u32, lua: &str, ) -> Vec { + drain_world(giver, npc_flags, close_on_cancel, lua).1 + } + + /// [`drain_after`], keeping the world so a test can read the [`QuestGiver`] afterwards. + fn drain_world( + giver: u64, + npc_flags: Option, + close_on_cancel: u32, + lua: &str, + ) -> (App, Vec) { let (tx, rx) = crossbeam_channel::unbounded(); let mut app = App::new(); app.insert_resource(NetCommands(tx)); @@ -1105,10 +1180,7 @@ mod tests { .0 .insert(giver, e); } - let mut quest = QuestGiver { - detail_flag, - ..Default::default() - }; + let mut quest = QuestGiver::default(); quest.open( giver, QuestView::Detail(QuestDetails { @@ -1117,29 +1189,32 @@ mod tests { title: "A Threat Within".into(), details: String::new(), objectives: String::new(), - auto_finish: detail_flag, + auto_finish: close_on_cancel, choices: Vec::new(), rewards: Vec::new(), money: 0, reward_spell: 0, }), ); + // The latch is written by the packet, after the open (as `net::quest_detail` does). + quest.close_on_cancel = close_on_cancel; app.insert_resource(quest); let script = UiScript::new().unwrap(); script.run(lua).unwrap(); app.insert_non_send_resource(script); app.add_systems(Update, drain_quest); app.update(); - rx.try_iter().collect() + let sent = rx.try_iter().collect(); + (app, sent) } const SHARER: u64 = 0x0000_0000_0000_002A; // HIGHGUID_PLAYER: a zero high word const NPC: u64 = 0xF130_0000_0000_0007; // HIGHGUID_UNIT /// **Ending the session on a SHARED quest answers the sharer — and `CloseQuest` does it too.** - /// 1733 shipped these as two actions on the assumption that only `DeclineQuest` answered; the - /// §5 found both Lua verbs calling one routine (`0x501130`), so ESC-ing a share panel reports - /// the decline exactly as the button does. This test is the corrected form of 1733's + /// 1733 shipped these as two actions on the assumption that only `DeclineQuest` answered; both + /// Lua verbs reach the teardown `0x501130(0,1)` (`CloseQuest` directly, `DeclineQuest` through + /// its PLAYER leg), so ESC-ing a share panel reports the decline exactly as the button does. This test is the corrected form of 1733's /// `closing_a_shared_quest_panel_is_not_a_decline`, which asserted the opposite. #[test] fn every_way_out_of_a_shared_quest_answers_the_sharer() { @@ -1158,8 +1233,8 @@ mod tests { } } - /// **Ending it on an NPC RE-OPENS the giver**, which benilla asserted for years that it did not - /// ("vanilla's client-side decline sends no packet"). The fork is on the NPC's own gossip flag: + /// **`DeclineQuest` on an NPC RE-OPENS the giver**, which benilla asserted for years that it did + /// not ("vanilla's client-side decline sends no packet"). The fork is on the NPC's own gossip flag: /// `CMSG_GOSSIP_HELLO` for a gossip-flagged NPC, `CMSG_QUESTGIVER_HELLO` otherwise. This is the /// mechanism behind the reference putting you back in the questgiver's menu after a decline. #[test] @@ -1182,20 +1257,103 @@ mod tests { ); } - /// The DETAILS packet's trailing `u32` **suppresses** that re-open when non-zero — the field + /// The `0xbe0824` latch **suppresses** a plain unit's re-open when non-zero — the field /// benilla parsed and ignored until the §5 found its one reader. Ignoring it meant a /// suppressed giver was re-opened anyway. #[test] - fn a_non_zero_detail_flag_suppresses_the_reopen() { + fn a_non_zero_latch_suppresses_a_plain_units_reopen() { let sent = drain_after(NPC, Some(0), 1, "DeclineQuest()"); assert!(sent.is_empty(), "flag 1 suppresses the re-open: {sent:?}"); } - /// A giver that is not a streamed unit sends nothing at all: an item giver (0664) has no list - /// to re-open, and neither does an NPC that walked out of view under the open window. + /// A giver that does not resolve sends nothing at all. `CloseQuest` still tears the window + /// down; `DeclineQuest` bails before its fork (`0x50142c`) and leaves it — the range guard + /// closes a window whose giver has gone. #[test] fn an_unstreamed_giver_ends_the_session_silently() { - let sent = drain_after(NPC, None, 0, "CloseQuest()"); + let (app, sent) = drain_world(NPC, None, 0, "CloseQuest()"); assert!(sent.is_empty(), "no unit, no re-open: {sent:?}"); + assert!(!app.world().resource::().is_open()); + + let (app, sent) = drain_world(NPC, None, 0, "DeclineQuest()"); + assert!(sent.is_empty(), "{sent:?}"); + assert!(app.world().resource::().is_open()); + } + + /// An item giver (0664) takes the teardown leg: silent, and the window closes. + #[test] + fn declining_an_item_quest_closes_silently() { + const ITEM: u64 = 0x4000_0000_0000_0099; // HIGHGUID_ITEM + let (app, sent) = drain_world(ITEM, None, 0, "DeclineQuest()"); + assert!(sent.is_empty(), "{sent:?}"); + assert!(!app.world().resource::().is_open()); + } + + /// **`CloseQuest()` is network-silent on an NPC** — ESC, the X, the greeting's Goodbye + /// (`HideUIPanel` → `QuestFrame_OnHide` → `CloseQuest`). Its binding `0x501a10` calls the + /// teardown `0x501130(0,1)` and nothing else, whose one send is the PLAYER-filtered push + /// result. It used to share `DeclineQuest`'s action and so re-open the list the player was + /// trying to close (a multi-quest NPC's greeting could not be dismissed). + #[test] + fn close_quest_on_an_npc_is_network_silent() { + use crate::target::cursor_mode::npc_flags; + for flags in [0, npc_flags::GOSSIP] { + for latch in [0, 1] { + let (app, sent) = drain_world(NPC, Some(flags), latch, "CloseQuest()"); + assert!( + sent.is_empty(), + "CloseQuest on an NPC (flags {flags:#x}, latch {latch}) sends nothing: {sent:?}" + ); + assert!( + !app.world().resource::().is_open(), + "…and tears the window down" + ); + } + } + } + + /// `DeclineQuest`'s gossip leg (`0x50143b`-`0x501474`) is tested BEFORE the `0xbe0824` latch + /// (`0x5014a2`), so a gossip-flagged NPC is re-opened whatever the latch says. + #[test] + fn the_gossip_leg_precedes_the_latch() { + use crate::target::cursor_mode::npc_flags; + let sent = drain_after(NPC, Some(npc_flags::GOSSIP), 1, "DeclineQuest()"); + assert!( + matches!(sent.as_slice(), [ClientCommand::GossipHello { guid: NPC }]), + "a latched gossip NPC still gets GOSSIP_HELLO: {sent:?}" + ); + } + + /// The re-open legs do NOT tear the window down (no `0x501130` on them) — the server's answer + /// replaces the panel — and they set the already-acted latch `0xbe0844`, which refuses a + /// second `DeclineQuest` and an `AcceptQuest` until the next panel arrives. + #[test] + fn a_reopening_decline_keeps_the_window_and_latches_acted() { + let (app, sent) = drain_world( + NPC, + Some(0), + 0, + "DeclineQuest() DeclineQuest() AcceptQuest()", + ); + assert!( + matches!( + sent.as_slice(), + [ClientCommand::QuestgiverHello { npc: NPC }] + ), + "one re-open, and nothing after it: {sent:?}" + ); + let giver = app.world().resource::(); + assert!( + giver.is_open(), + "the window stays up for the server's answer" + ); + } + + /// The latch-diverted leg (`0x5014aa` → `0x501130(0,1)`) DOES tear the window down. + #[test] + fn a_latched_decline_closes_the_window() { + let (app, sent) = drain_world(NPC, Some(0), 1, "DeclineQuest()"); + assert!(sent.is_empty(), "{sent:?}"); + assert!(!app.world().resource::().is_open()); } } diff --git a/crates/benilla-app/src/ui_quest/net.rs b/crates/benilla-app/src/ui_quest/net.rs index e45383fe..de8e0726 100644 --- a/crates/benilla-app/src/ui_quest/net.rs +++ b/crates/benilla-app/src/ui_quest/net.rs @@ -33,7 +33,29 @@ pub(super) fn register(app: &mut App) { .net_handler(K::QuestLogFull, on_log_full) .net_handler(K::QuestGiverInvalid, on_giver_invalid) .net_handler(K::QuestGiverFailed, on_giver_failed) - .net_handler(K::Disconnected, on_session_end); + .net_handler(K::Disconnected, on_session_end) + .net_handler(K::Connected, on_login) + .net_handler(K::Worldport, on_worldport); +} + +/// The world-enter reset `0x500af0` (sole caller `0x49099c`, inside the enter-world cascade +/// `0x4908c0` that runs on a fresh login and on every worldport) zeroes the `0xbe0824` latch — +/// its only clear (wow-re ledger `0x500af0`). A second listener on each kind, after the bridge's. +fn on_login(In(ev): In, mut quest: ResMut) { + if let SessionEvent::Connected { .. } = ev { + world_enter_reset(&mut quest); + } +} + +/// [`on_login`]'s twin for the other entry into the cascade, the worldport. +fn on_worldport(In(ev): In, mut quest: ResMut) { + if let SessionEvent::Worldport { .. } = ev { + world_enter_reset(&mut quest); + } +} + +fn world_enter_reset(quest: &mut QuestGiver) { + quest.close_on_cancel = 0; } fn on_giver_status(In(ev): In, mut quest: ResMut) { @@ -202,8 +224,8 @@ fn quest_detail(d: QuestDetails, quest: &mut QuestGiver, commands: &NetCommands) }); return; } - // The trailing flag is a latch, not a field of the view (see `QuestGiver::detail_flag`). - quest.detail_flag = d.auto_finish; + // The trailing flag is a latch, not a field of the view (see `QuestGiver::close_on_cancel`). + quest.close_on_cancel = d.auto_finish; quest.open(d.npc, crate::ui_quest::QuestView::Detail(d)); } @@ -214,6 +236,8 @@ fn quest_progress(p: QuestRequestItems, quest: &mut QuestGiver) { "net: quest progress — quest {} on {:#x} (complete: {})", p.quest_id, p.npc, p.is_complete ); + // The progress publisher `0x501070` writes `closeOnCancel` into the same latch (`0x50111a`). + quest.close_on_cancel = p.close_on_cancel; quest.open(p.npc, crate::ui_quest::QuestView::Progress(p)); } @@ -223,6 +247,9 @@ fn quest_offer(o: QuestOfferReward, quest: &mut QuestGiver) { "net: quest reward offer — quest {} on {:#x}", o.quest_id, o.npc ); + // The `u32` after the reward text lands in the same latch as DETAILS' trailing one — one + // publisher, `0x500ef0` (`0x50101a`). + quest.close_on_cancel = o.auto_finish; quest.open(o.npc, crate::ui_quest::QuestView::Reward(o)); } @@ -583,17 +610,79 @@ mod tests { assert!(rx.try_iter().next().is_none(), "no verdict, no refusal"); } - /// The DETAILS trailing flag is LATCHED on the packet, not read off the open view — the - /// reference's `0xbe0824`, whose one reader runs whichever panel is up. + /// The `0xbe0824` latch is written by all THREE panel packets — DETAILS' trailing `u32`, + /// OFFER_REWARD's `u32` after the reward text (both through `0x500ef0`, `0x50101a`) and + /// REQUEST_ITEMS' `closeOnCancel` (`0x501070`, `0x50111a`) — and zeroed by the world-enter + /// reset `0x500af0`. It is a latch, not a field of the open view: its one reader runs + /// whichever panel is up. benilla used to write it from DETAILS alone and never clear it, so + /// a single-quest auto-open's progress panel (`closeOnCancel` = 1) re-opened the NPC on + /// Cancel, which answers with the same panel again. #[test] - fn the_detail_flag_latches_from_the_packet() { + fn the_latch_is_written_by_every_panel_packet_and_cleared_on_world_enter() { + const NPC: u64 = 0xF130_0000_0000_0007; let (tx, _rx) = crossbeam_channel::unbounded(); let commands = NetCommands(tx); let mut quest = QuestGiver::default(); - let mut d = detail(0xF130_0000_0000_0007, 100); + let mut d = detail(NPC, 100); d.auto_finish = 3; quest_detail(d, &mut quest, &commands); - assert_eq!(quest.detail_flag, 3); + assert_eq!(quest.close_on_cancel, 3, "DETAILS writes it"); + + quest_progress(progress(NPC, 100, 1), &mut quest); + assert_eq!( + quest.close_on_cancel, 1, + "REQUEST_ITEMS' closeOnCancel writes it" + ); + quest_progress(progress(NPC, 100, 0), &mut quest); + assert_eq!(quest.close_on_cancel, 0, "…including a zero"); + + quest_offer(offer(NPC, 100, 1), &mut quest); + assert_eq!(quest.close_on_cancel, 1, "OFFER_REWARD writes it"); + + world_enter_reset(&mut quest); + assert_eq!(quest.close_on_cancel, 0, "the world-enter reset zeroes it"); + } + + /// Every panel publish clears the already-acted latch (`0x500bd0`, `0x500d91`). + #[test] + fn a_panel_packet_clears_the_acted_latch() { + let (tx, _rx) = crossbeam_channel::unbounded(); + let commands = NetCommands(tx); + let mut quest = QuestGiver { + acted: true, + ..Default::default() + }; + quest_detail(detail(0xF130_0000_0000_0007, 100), &mut quest, &commands); + assert!(!quest.acted); + } + + fn progress(npc: u64, quest_id: u32, close_on_cancel: u32) -> QuestRequestItems { + QuestRequestItems { + npc, + quest_id, + title: "A Threat Within".into(), + request_text: String::new(), + emote: 0, + close_on_cancel, + required_money: 0, + required_items: Vec::new(), + is_complete: false, + } + } + + fn offer(npc: u64, quest_id: u32, auto_finish: u32) -> QuestOfferReward { + QuestOfferReward { + npc, + quest_id, + title: "A Threat Within".into(), + offer_text: String::new(), + auto_finish, + choices: Vec::new(), + rewards: Vec::new(), + money: 0, + quest_flags: 0, + reward_spell: 0, + } } } diff --git a/crates/benilla-app/src/ui_quest_log.rs b/crates/benilla-app/src/ui_quest_log.rs index e0c1d9be..d89a5a26 100644 --- a/crates/benilla-app/src/ui_quest_log.rs +++ b/crates/benilla-app/src/ui_quest_log.rs @@ -23,9 +23,8 @@ //! divergence note at the fire site / decision 0340) — diffed against a `Local`, exactly like //! every other feed in this crate. //! -//! [`drain_quest_log_abandons`] maps the confirmed abandon's 1-based Lua entry index back to its -//! descriptor slot (this frame's push order, kept on [`QuestLog`]) and sends -//! `CMSG_QUESTLOG_REMOVE_QUEST`. +//! [`drain_quest_log_abandons`] maps the confirmed abandon's quest id to its descriptor slot (this +//! frame's rows, kept on [`QuestLog`]) and sends `CMSG_QUESTLOG_REMOVE_QUEST`. //! //! This module used to also answer the questgiver greeting's active/available split, via a //! `contains` membership test over the live descriptor slots. **It doesn't any more, and must not @@ -76,20 +75,20 @@ struct Row { /// /// - `templates` — the `SMSG_QUEST_QUERY_RESPONSE` cache, ask-once by quest id through /// [`QueryCache`] (the exact twin of [`Items`]'s item-template cache). -/// - `entry_slots` — this frame's pushed entry order → descriptor slot, so -/// [`drain_quest_log_abandons`] can turn a confirmed abandon's 1-based Lua index back into the -/// `CMSG_QUESTLOG_REMOVE_QUEST` slot it came from (slots aren't contiguous — an abandoned/turned-in -/// quest leaves a gap). +/// - `row_slots` — this frame's quest rows as `(quest id, descriptor slot)`, folded ones included: +/// the reference's row table `0xbb71c0`, which [`drain_quest_log_abandons`] searches by the +/// confirmed abandon's quest id for the `CMSG_QUESTLOG_REMOVE_QUEST` slot (slots aren't +/// contiguous — an abandoned/turned-in quest leaves a gap). #[derive(Resource, Default)] pub(crate) struct QuestLog { templates: QueryCache, - entry_slots: Vec>, + row_slots: Vec<(u32, u8)>, /// Collapsed section headers, keyed by header TITLE (two zones sharing a name share a header /// row, so the fold state naturally shares too). Owned here — the engine only reports the /// flag and drains toggle intents ([`drain_quest_log_collapses`]). collapsed: HashSet, /// This frame's pushed entry order → the header title for header rows (`None` for quests) — - /// the collapse drain's index→identity map, the fold twin of `entry_slots`. + /// the collapse drain's index→identity map. header_keys: Vec>, } @@ -123,7 +122,7 @@ impl QuestLog { /// to re-ask that there's no reason to risk it. pub(crate) fn clear_session(&mut self) { self.templates.clear(); - self.entry_slots.clear(); + self.row_slots.clear(); self.collapsed.clear(); self.header_keys.clear(); } @@ -461,15 +460,15 @@ fn build_detail( } } -/// Map a 1-based Lua entry index to its descriptor slot via this frame's push order (`entry_slots`) -/// — the abandon drain's half of the flow; `None` for a header row (no slot to remove). Pure, so -/// it's testable without a live [`QuestLog`]/ECS. -fn abandon_slot(entry: u32, entry_slots: &[Option]) -> Option { - (entry as usize) - .checked_sub(1) - .and_then(|i| entry_slots.get(i)) - .copied() - .flatten() +/// The descriptor slot of the row carrying `quest_id` — `AbandonQuest`'s search (`0x4df070`: walk +/// the 40-row table `0xbb71c0`, skip headers, match the row's quest id, send its slot `+0x4`); +/// `None` when no row carries it, which the reference answers by doing nothing. Pure, so it's +/// testable without a live [`QuestLog`]/ECS. +fn abandon_slot(quest_id: u32, row_slots: &[(u32, u8)]) -> Option { + row_slots + .iter() + .find(|&&(id, _)| id == quest_id) + .map(|&(_, slot)| slot) } /// Re-point the engine's quest-log selection (a 1-based entry INDEX) across a snapshot rebuild — @@ -576,8 +575,7 @@ fn order_groups(rows: &[GroupRow]) -> Vec<(i32, String, Vec)> { /// Read the self player's `PLAYER_QUEST_LOG` descriptor slots each frame, resolve entries/detail, /// and push a [`QuestLogState`] snapshot on change (diffed against a `Local`, the crate's standard -/// feed shape). Also refreshes [`QuestLog::active_quest_ids`]/`entry_slots` for the greeting split -/// and the abandon drain. +/// feed shape). Also refreshes [`QuestLog`]'s `row_slots` for the abandon drain. fn feed_quest_log( script: Option>, self_q: Query<(&ObjectStore, &Guid), With>, @@ -734,7 +732,6 @@ fn feed_quest_log( }; let mut entries: Vec = Vec::new(); - let mut entry_slots: Vec> = Vec::new(); let mut header_keys: Vec> = Vec::new(); // The quests folded under a collapsed header: out of the visible list, still in the log. The // engine's watch prune counts them, as the reference's does (`0x4de7a7`–`0x4de80f` scans the @@ -756,7 +753,6 @@ fn feed_quest_log( objectives: Vec::new(), detail: None, // a header names a zone/sort; there is no quest to describe }); - entry_slots.push(None); header_keys.push(Some(name.clone())); for &ri in row_idxs { let r = &rows[ri]; @@ -836,7 +832,6 @@ fn feed_quest_log( objectives, detail, }); - entry_slots.push(Some(r.slot)); header_keys.push(None); } } @@ -847,7 +842,8 @@ fn feed_quest_log( if new_sel != sel { script.set_quest_log_selection(new_sel); } - quest_log.entry_slots = entry_slots; + // Every cached row, folded or not — the table `AbandonQuest` searches by quest id. + quest_log.row_slots = rows.iter().map(|r| (r.quest_id, r.slot)).collect(); quest_log.header_keys = header_keys; let fresh = QuestLogState { @@ -1004,9 +1000,9 @@ fn quests_with_progressed_objectives( .collect() } -/// Drain the confirmed abandons (1-based Lua entry index, pinned at click time — see +/// Drain the confirmed abandons (the quest id marked at click time — see /// `benilla_ui::script::quest_log`'s module doc) and map each to `CMSG_QUESTLOG_REMOVE_QUEST` via -/// this frame's `entry_slots`. +/// this frame's `row_slots`. /// The `ZoneOrSort → header name` lookup ([`benilla_formats::QuestHeaderNames`]), loaded once at /// startup. Absent when the client data didn't load — the feed then buckets everything under /// "Quests". @@ -1102,13 +1098,13 @@ fn drain_quest_log_abandons( let Some(mut script) = script else { return; }; - for entry in script.take_quest_log_abandons() { - match abandon_slot(entry, &quest_log.entry_slots) { + for quest in script.take_quest_log_abandons() { + match abandon_slot(quest, &quest_log.row_slots) { Some(slot) => { - debug!("ui_quest_log: abandon entry {entry} → slot {slot}"); + debug!("ui_quest_log: abandon quest {quest} → slot {slot}"); let _ = commands.0.send(ClientCommand::QuestlogRemove { slot }); } - None => debug!("ui_quest_log: abandon entry {entry} out of range — ignored"), + None => debug!("ui_quest_log: abandon quest {quest} is not in the log — ignored"), } } } @@ -1690,18 +1686,103 @@ mod tests { assert_eq!(money_split(0), (0, 0)); } - // ── abandon_slot: entry → descriptor slot across gaps ────────────────────────────────────────── + // ── abandon_slot: quest id → descriptor slot across gaps ─────────────────────────────────────── #[test] - fn abandon_maps_entry_to_slot_across_a_gap_and_skips_headers() { - // A header row (None), then slots 0 and 2 (slot 1 empty in the descriptor array) — the - // push order skips the gap; entry 1 is the header, which maps to NO slot. - let entry_slots = vec![None, Some(0u8), Some(2u8)]; - assert_eq!(abandon_slot(1, &entry_slots), None); // header row: nothing to remove - assert_eq!(abandon_slot(2, &entry_slots), Some(0)); - assert_eq!(abandon_slot(3, &entry_slots), Some(2)); - assert_eq!(abandon_slot(4, &entry_slots), None); - assert_eq!(abandon_slot(0, &entry_slots), None); // 1-based; 0 is never a valid entry + fn abandon_maps_quest_id_to_slot_across_a_gap() { + // Slots 0 and 2 (slot 1 empty in the descriptor array) — the id finds its own slot. + let row_slots = vec![(783u32, 0u8), (7, 2)]; + assert_eq!(abandon_slot(783, &row_slots), Some(0)); + assert_eq!(abandon_slot(7, &row_slots), Some(2)); + assert_eq!(abandon_slot(8, &row_slots), None); // not in the log: nothing to remove + assert_eq!(abandon_slot(0, &row_slots), None); // 0 is never a quest + } + + // ── The abandon mark is a QUEST ID (0x4dfb50 / 0x4df070) ───────────────────────────────────── + + /// **An abandon names a quest, not a row.** `SetAbandonQuest` (`0x4dfb50`) copies the + /// selection `0xbb7480` — which holds the selected row's QUEST ID (`0x4def30`, `0x4def5d`) — + /// into the mark `0xbb7484`; `AbandonQuest` (`0x4dfe00` → `0x4df070`) searches the row table + /// `0xbb71c0` for that id and removes that row's slot. So a log that re-indexes while the + /// confirm popup is up — a fold, a quest arriving or leaving — cannot move the abandon onto a + /// neighbour. Ours pinned the 1-based row index and resolved it against the rows current at + /// confirm time: fold the group above and the popup's Yes abandoned the quest below. + #[test] + fn a_fold_between_mark_and_confirm_does_not_retarget_the_abandon() { + use benilla_protocol::messages::field::FIELD_PLAYER_QUEST_LOG_1_1; + + // Z1 = zone 0 ("Missing header!", forced first) holding A; Z2 = zone 7 holding B and C. + // Entries: [Z1, A, Z2, B, C] — B is row 4. + const A: u32 = 101; + const B: u32 = 201; + const C: u32 = 202; + let quests = [(0u8, A, 0i32, "A"), (1, B, 7, "B"), (2, C, 7, "C")]; + + let (tx, rx) = crossbeam_channel::unbounded(); + let mut app = App::new(); + app.insert_resource(NetCommands(tx)); + app.init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::(); + let mut log = QuestLog::default(); + let mut pairs = Vec::new(); + for (slot, id, zos, title) in quests { + let mut t = quest_template(std::array::from_fn(|_| obj(0, 0, 0, 0, ""))); + t.quest_id = id; + t.zone_or_sort = zos; + t.title = title.into(); + log.insert_template(t); + pairs.push((FIELD_PLAYER_QUEST_LOG_1_1 + 3 * u16::from(slot), id)); + } + app.insert_resource(log); + app.world_mut().spawn(( + ObjectStore(ObjectFields::from_pairs(&pairs)), + Guid(0x2A), + SelfPlayer, + )); + app.insert_non_send_resource(UiScript::new().unwrap()); + app.add_systems( + Update, + ( + drain_quest_log_collapses, + feed_quest_log, + drain_quest_log_abandons, + ) + .chain(), + ); + let run = |app: &mut App, lua: &str| { + app.world_mut() + .non_send_resource_mut::() + .run(lua) + .unwrap(); + }; + + app.update(); + run(&mut app, "SelectQuestLogEntry(4); SetAbandonQuest()"); + assert_eq!( + app.world() + .non_send_resource::() + .eval::("return GetAbandonQuestName()") + .unwrap(), + "B" + ); + // The popup is up; the player folds Z1 — B moves from row 4 to row 3, C takes row 4. + run(&mut app, "CollapseQuestHeader(1)"); + app.update(); + run(&mut app, "AbandonQuest()"); + app.update(); + + let sent: Vec<_> = rx + .try_iter() + .filter(|c| matches!(c, ClientCommand::QuestlogRemove { .. })) + .collect(); + assert!( + matches!(sent.as_slice(), [ClientCommand::QuestlogRemove { slot: 1 }]), + "the abandon removes B's slot (1), never C's (2): {sent:?}" + ); } // ── quests_with_progressed_objectives: the QUEST_WATCH_UPDATE per-quest trigger ───────────────── diff --git a/crates/benilla-app/src/ui_stable/mod.rs b/crates/benilla-app/src/ui_stable/mod.rs index d6c2d362..2a2499aa 100644 --- a/crates/benilla-app/src/ui_stable/mod.rs +++ b/crates/benilla-app/src/ui_stable/mod.rs @@ -527,4 +527,32 @@ mod tests { "the template ships no display" ); } + + /// **The stable window dies with the session.** The walk-away guard does not cover it: it + /// needs a self player to measure from, and between the socket dying and the next world + /// entry there is none — so a stable open at the drop rode into the next login's fresh VM. + #[test] + fn the_session_end_closes_the_stable() { + let mut app = App::new(); + app.init_resource::() + .init_resource::() + .init_resource::(); + net::register(&mut app); + { + let mut open = app.world_mut().resource_mut::(); + open.npc = Some(0xF130_0000_0000_0042); + open.num_stable_slots = 2; + } + + crate::net::handlers::dispatch( + app.world_mut(), + vec![benilla_protocol::SessionEvent::Disconnected { + reason: "socket".into(), + end: benilla_protocol::SessionEnd::Lost, + }], + ); + + let open = app.world().resource::(); + assert_eq!((open.npc, open.num_stable_slots), (None, 0)); + } } diff --git a/crates/benilla-app/src/ui_stable/net.rs b/crates/benilla-app/src/ui_stable/net.rs index 35b84f86..3a4c596a 100644 --- a/crates/benilla-app/src/ui_stable/net.rs +++ b/crates/benilla-app/src/ui_stable/net.rs @@ -14,7 +14,15 @@ use crate::net::{ClientCommand, NetCommands, NetHandlerApp}; pub(super) fn register(app: &mut App) { use SessionEventKind as K; app.net_handler(K::ListStabledPets, on_list) - .net_handler(K::StableResult, on_result); + .net_handler(K::StableResult, on_result) + .net_handler(K::Disconnected, on_session_end); +} + +/// The stable window dies with the socket — a listener on the session end (a second handler on the +/// kind, after the bridge's own teardown). The walk-away guard cannot stand in: it measures from a +/// self player, and there is none between the drop and the next world entry. +fn on_session_end(In(_): In, mut stable_open: ResMut) { + stable_open.clear(); } fn on_list( diff --git a/crates/benilla-app/src/ui_taxi/mod.rs b/crates/benilla-app/src/ui_taxi/mod.rs index ff9cf6f1..bf1867d3 100644 --- a/crates/benilla-app/src/ui_taxi/mod.rs +++ b/crates/benilla-app/src/ui_taxi/mod.rs @@ -34,8 +34,8 @@ use bevy::prelude::*; use benilla_ui::script::{TaxiUiState, UiScript}; use crate::names::NameCache; -use crate::net::{ClientCommand, NetCommands}; -use crate::player::Player; +use crate::net::{ClientCommand, NetCommands, ObjectStore, SelfPlayer}; +use crate::player::UNIT_FLAG_TAXI_FLIGHT; use crate::ui_script::{UiFeed, UiInput}; use crate::ui_session::{close_npc_session_out_of_range, NpcSession}; @@ -128,19 +128,17 @@ impl NpcSession for TaxiState { /// Push the current taxi map into the VM, fire `TAXIMAP_OPENED`/`TAXIMAP_CLOSED` on a transition /// (or a content/name change), surface an activate refusal on the red error line (closing the map /// on `OK` instead — the flight starts and the map has nothing left to show), present a -/// first-visit discovery, and push the `UnitOnTaxi` ride flag off [`Player::server_riding`]. -/// Diffed against `Local` memory, the trainer/merchant feed shape. +/// first-visit discovery. Diffed against `Local` memory, the trainer/merchant feed shape. (The +/// `UnitOnTaxi` flag is [`feed_on_taxi`]'s — it reads the descriptor, not the taxi window.) fn feed_taxi( script: Option>, mut state: ResMut, catalogs: Option>, - player: Res, names: Res, commands: Res, mut cache: ResMut, mut last: Local>>, mut last_name: Local>>, - mut last_riding: Local>>, mut sink: crate::ui_action::MessageSink, ) { let Some(mut script) = script else { @@ -148,7 +146,6 @@ fn feed_taxi( }; let last = last.get(&script); let last_name = last_name.get(&script); - let last_riding = last_riding.get(&script); // The activate verdict (SMSG_ACTIVATETAXIREPLY), staged by the net bridge: a refusal goes to // the surface its message record names ([`taxi_error_key`] — seven of the twelve are the @@ -251,14 +248,38 @@ fn feed_taxi( *last = fresh; *last_name = flightmaster_name; } +} - // UnitOnTaxi: a server-authored spline currently owns the avatar (Charge/knockback/taxi — - // 0260's rails); taxi is one of its callers, so this is the faithful signal without any - // taxi-specific movement state. Diffed like every other single-value push. - let riding = player.server_riding(); - if *last_riding != Some(riding) { - script.set_on_taxi(riding); - *last_riding = Some(riding); +/// Push `UnitOnTaxi("player")` — **our own descriptor's `UNIT_FLAG_TAXI_FLIGHT`, and nothing +/// else**. The reference's verb (`0x517a40`) resolves the token, then reads `UNIT_FIELD_FLAGS` +/// (`[[obj+0x110]+0xa0]`) and answers `1` iff bit 20 is set (`0x517a86 shr ecx,0x14; test cl,1`; +/// wow-re `unit-verbs-controlled-charmed-creaturetype.md`). vmangos sets and clears that bit exactly +/// around a flight (`WaypointMovementGenerator.cpp`, the `FlightPathMovementGenerator` +/// initialize/finalize). +/// +/// It used to read [`crate::player::Player::server_riding`] — "a server spline owns the avatar" — +/// which is also true under a fear's flee path, a Charge and a knockback. That was not a cosmetic +/// over-answer: stock `UIParent.lua`'s `PLAYER_CONTROL_LOST` handler returns early on +/// `UnitOnTaxi("player")`, so a feared player kept every window open that the reference closes. +/// +/// No self store streamed yet reads as not on a taxi — the verb's own absent-object `nil`. +/// Diffed like every other single-value push. +fn feed_on_taxi( + script: Option>, + self_q: Query<&ObjectStore, With>, + mut last: Local>>, +) { + let Some(mut script) = script else { + return; + }; + let last = last.get(&script); + let on_taxi = self_q + .iter() + .next() + .is_some_and(|s| s.0.unit_flags() & UNIT_FLAG_TAXI_FLIGHT != 0); + if *last != Some(on_taxi) { + script.set_on_taxi(on_taxi); + *last = Some(on_taxi); } } @@ -341,6 +362,7 @@ impl Plugin for UiTaxiPlugin { // frame; drain after it (mirrors ui_merchant/ui_trainer). close_npc_session_out_of_range::.before(feed_taxi), feed_taxi.in_set(UiFeed), + feed_on_taxi.in_set(UiFeed), drain_taxi.after(UiInput), ), ); @@ -350,6 +372,50 @@ impl Plugin for UiTaxiPlugin { #[cfg(test)] mod tests { use super::*; + use benilla_protocol::field::FIELD_UNIT_FLAGS; + use benilla_protocol::ObjectFields; + + /// `UnitOnTaxi("player")` answers our descriptor's `UNIT_FLAG_TAXI_FLIGHT` — `1` with the + /// bit, `nil` without it — whatever else owns the avatar's movement. (A fear's flee path, a + /// Charge or a knockback are server splines too, and the feed reads nothing that knows about + /// them: it takes no `Player` at all.) + #[test] + fn unit_on_taxi_reads_the_taxi_flight_flag() { + let mut app = App::new(); + app.add_systems(Update, feed_on_taxi); + app.insert_non_send_resource(UiScript::new().unwrap()); + let me = app + .world_mut() + .spawn(( + SelfPlayer, + ObjectStore(ObjectFields::from_pairs(&[(FIELD_UNIT_FLAGS, 0x1000)])), + )) + .id(); + let on_taxi = |app: &mut App| -> bool { + app.update(); + let s = app.world_mut().non_send_resource_mut::(); + s.eval::>(r#"return UnitOnTaxi("player")"#) + .unwrap() + .is_some() + }; + assert!(!on_taxi(&mut app), "no TAXI_FLIGHT bit: nil"); + + app.world_mut() + .entity_mut(me) + .insert(ObjectStore(ObjectFields::from_pairs(&[( + FIELD_UNIT_FLAGS, + 0x1000 | UNIT_FLAG_TAXI_FLIGHT, + )]))); + assert!(on_taxi(&mut app), "the bit set: 1"); + + app.world_mut() + .entity_mut(me) + .insert(ObjectStore(ObjectFields::from_pairs(&[( + FIELD_UNIT_FLAGS, + 0, + )]))); + assert!(!on_taxi(&mut app), "landed: nil again"); + } #[test] fn open_close_and_session_clear() { diff --git a/crates/benilla-app/src/ui_tooltip/mod.rs b/crates/benilla-app/src/ui_tooltip/mod.rs index 5ee255e9..89fd068a 100644 --- a/crates/benilla-app/src/ui_tooltip/mod.rs +++ b/crates/benilla-app/src/ui_tooltip/mod.rs @@ -760,7 +760,9 @@ fn drive_mouseover_tooltip( // The hovered UNIT's snapshot (a hovered non-unit resolves no store here). let unit = hovered.target.zip(hovered.guid).and_then(|(entity, guid)| { let store = stores.get(entity).ok()?; - let name = names.resolve(guid, &commands).map(str::to_string); + let name = names + .resolve_unit(guid, Some(store), &commands) + .map(str::to_string); let reaction = ring_reaction( rx.factions.as_deref(), &rx.reputations, diff --git a/crates/benilla-app/src/ui_tradeskill.rs b/crates/benilla-app/src/ui_tradeskill.rs index 90cf0e83..55d143c1 100644 --- a/crates/benilla-app/src/ui_tradeskill.rs +++ b/crates/benilla-app/src/ui_tradeskill.rs @@ -31,6 +31,7 @@ use bevy::prelude::*; use benilla_formats::{SpellFocusCatalog, SPELL_ATTR_IS_TRADESKILL, SPELL_EFFECT_CREATE_ITEM}; use benilla_protocol::messages::PLAYER_SKILL_SLOTS; +use benilla_protocol::{SessionEvent, SessionEventKind}; use benilla_ui::script::{ TradeSkillDifficulty, TradeSkillReagent, TradeSkillRecipe, TradeSkillState, UiScript, }; @@ -38,7 +39,7 @@ use benilla_ui::script::{ use crate::creature_anim::{CastEvent, CastEventKind}; use crate::entities::ItemDisplays; use crate::items::Items; -use crate::net::{NetCommands, ObjectStore, Objects, SelfPlayer}; +use crate::net::{NetCommands, NetHandlerApp, ObjectStore, Objects, SelfPlayer}; use crate::spell::{cast_target, CastCommit, CastLadder}; use crate::ui_action::{PlayerActions, Spells}; use crate::ui_items::{count_of, item_icon, InventoryScope}; @@ -54,7 +55,10 @@ use benilla_assets::{AssetSet, LockRecover, WorldAssets}; pub(crate) struct TradeSkillOpens(pub(crate) Vec); /// The open crafting book: the skill line whose recipes the window shows. `None` = closed. -/// Client-local state — no wire owns it (0437). +/// Client-local state — no wire owns it (0437). Cleared by the Lua close and by the session end +/// ([`on_session_end`]): a logout installs a fresh VM without running the old one's `OnHide`, so +/// the close alone never comes, and the next login's feed would fire `TRADE_SKILL_SHOW` into the +/// new character's UI. #[derive(Resource, Default)] pub(crate) struct TradeSkillOpen { pub(crate) line: Option, @@ -62,7 +66,8 @@ pub(crate) struct TradeSkillOpen { /// The Create/Create All repeat machine (TU-D, byte-confirmed — decision 0446): /// `DoTradeSkill(spell, n)` latches `n`, casts once, and each of our own `SMSG_SPELL_GO`s for -/// that spell decrements and re-casts until dry; any cast failure or a window close stops it cold. +/// that spell decrements and re-casts until dry; any cast failure, a window close or the session +/// end ([`on_session_end`]) stops it cold. #[derive(Resource, Default)] pub(crate) struct TradeSkillRepeat { pub(crate) spell_id: u32, @@ -86,6 +91,7 @@ pub(crate) struct UiTradeSkillPlugin; impl Plugin for UiTradeSkillPlugin { fn build(&self, app: &mut App) { + app.net_handler(SessionEventKind::Disconnected, on_session_end); app.init_resource::() .init_resource::() .init_resource::() @@ -104,6 +110,20 @@ impl Plugin for UiTradeSkillPlugin { } } +/// The book and its repeat die with the session — a listener on the session end (a second handler +/// on the kind, after the bridge's own teardown). Queued opener casts go too: they were the old +/// character's. +fn on_session_end( + In(_): In, + mut opens: ResMut, + mut open: ResMut, + mut repeat: ResMut, +) { + opens.0.clear(); + open.line = None; + repeat.clear(); +} + fn load_spell_focus(mut commands: Commands, assets: Option>) { let Some(assets) = assets else { return }; let loaded = { @@ -638,4 +658,33 @@ mod tests { let d = recipe(SPELL_EFFECT_CREATE_ITEM, 777); assert_eq!(recipe_icon(&d, None, &deps.items, &deps.commands), None); } + + /// **The book and its repeat die with the session.** Both are client-local and only the Lua + /// close cleared them — but a logout installs a fresh VM without running the old one's + /// `OnHide`, so the close never comes: the next login's feed saw `None → Some` and fired + /// `TRADE_SKILL_SHOW` into the new character's UI, and a latched "Create All" re-cast on that + /// character's first own `SMSG_SPELL_GO` of the same id. Driven through the real registration. + #[test] + fn the_session_end_closes_the_book_and_drops_the_repeat() { + let mut app = App::new(); + app.add_plugins(UiTradeSkillPlugin); + app.world_mut().resource_mut::().line = Some(197); + { + let mut repeat = app.world_mut().resource_mut::(); + repeat.spell_id = 3915; + repeat.remaining = 4; + } + + crate::net::handlers::dispatch( + app.world_mut(), + vec![benilla_protocol::SessionEvent::Disconnected { + reason: "logged out".into(), + end: benilla_protocol::SessionEnd::LoggedOut, + }], + ); + + assert_eq!(app.world().resource::().line, None); + let repeat = app.world().resource::(); + assert_eq!((repeat.spell_id, repeat.remaining), (0, 0)); + } } diff --git a/crates/benilla-app/src/ui_unit.rs b/crates/benilla-app/src/ui_unit.rs index 5181881b..11397044 100644 --- a/crates/benilla-app/src/ui_unit.rs +++ b/crates/benilla-app/src/ui_unit.rs @@ -1049,8 +1049,11 @@ pub(crate) struct UnitStores<'w, 's> { /// `ObjectStore` since 2334, and a durability tick or a stack count naming no unit token /// would open the gate for nothing (2343). changed: Query<'w, 's, (), (Changed, Without)>, - /// Whose object left the manager — the other half of that gate, and cleared every run. - removed: RemovedComponents<'w, 's, ObjectStore>, + /// Whose object left the manager — the other half of that gate, and cleared every run. The + /// [`Guid`], not the store: a torn-down object sheds its guid at the teardown while its + /// fading model keeps the store another two seconds ([`crate::net::tear_down`]), and a + /// despawn removes both. + removed: RemovedComponents<'w, 's, Guid>, /// The per-field edges this run (decision 2297) — [`fire_transitions`]' three mirror-diff /// arms fire off these, per token naming the moved unit. edges: MessageReader<'w, 's, FieldChanged>, @@ -1712,7 +1715,9 @@ fn feed_units( // resolve through the cache — a miss queries the server once and lands on a later frame. let self_pair = self_q.iter().next(); let player = self_pair.map(|(store, guid)| { - let name = names.resolve(guid.0, &commands).map(str::to_string); + let name = names + .resolve_unit(guid.0, Some(store), &commands) + .map(str::to_string); let mut s = snapshot(store, name, 0, chr); s.is_player = true; // The stated `is_connected` gap, closed for every token this feed pushes — the field's own @@ -1768,7 +1773,9 @@ fn feed_units( // stays server memory outside it, which the disjoint field borrows above preserve.) let target = selection.target.zip(selection.guid).and_then(|(e, guid)| { let store = stores.all.get(e).ok()?; - let name = names.resolve(guid, &commands).map(str::to_string); + let name = names + .resolve_unit(guid, Some(store), &commands) + .map(str::to_string); // The target's reaction toward us, on the `UnitReaction` 1..8 scale — the same decode the // selection ring runs (reputation-first, else the faction-template comparator). `ring_reaction` // returns the raw 0..7 rank (neutral its no-data fallback), which is `UnitReaction − 1`; +1 @@ -1826,7 +1833,9 @@ fn feed_units( .and_then(|guid| Some((*index.as_ref()?.0.get(&guid)?, guid))) .and_then(|(entity, guid)| { let store = stores.all.get(entity).ok()?; - let name = names.resolve(guid, &commands).map(str::to_string); + let name = names + .resolve_unit(guid, Some(store), &commands) + .map(str::to_string); let reaction = ring_reaction( factions.as_deref(), &reputations, @@ -1914,7 +1923,9 @@ fn feed_units( .and_then(|i| Some((i.0?, i.1?))) .and_then(|(entity, guid)| { let store = stores.all.get(entity).ok()?; - let name = names.resolve(guid, &commands).map(str::to_string); + let name = names + .resolve_unit(guid, Some(store), &commands) + .map(str::to_string); let reaction = ring_reaction( factions.as_deref(), &reputations, diff --git a/crates/benilla-app/src/vplates.rs b/crates/benilla-app/src/vplates.rs index 32bd6cc0..004229cd 100644 --- a/crates/benilla-app/src/vplates.rs +++ b/crates/benilla-app/src/vplates.rs @@ -814,7 +814,7 @@ fn drive_vplates( .max(1) as f32, bar_colour: [tint[0], tint[1], tint[2]], name: names - .resolve(guid.0, &net_commands) + .resolve_unit(guid.0, store, &net_commands) .map(str::to_owned) .unwrap_or_default(), // The skull's two legs (`0x7cbb40`, §7-VERIFIED): a WORLD BOSS — creature- @@ -830,7 +830,9 @@ fn drive_vplates( level: store.and_then(|s| s.0.unit_level()), skull: store.and_then(|s| s.0.unit_level()).is_some_and(|level| { crate::names::gated_rank( - benilla_protocol::guid::entry(guid.0).and_then(|e| names.creature_record(e)), + store + .and_then(|s| s.0.object_entry()) + .and_then(|e| names.creature_record(e)), store, ) == 3 || (rank <= 1 && level >= my_level + 10 && !unit_is_grey(my_level, level)) diff --git a/crates/benilla-app/tests/auction_frame.rs b/crates/benilla-app/tests/auction_frame.rs index d9280a5e..ed3a0dae 100644 --- a/crates/benilla-app/tests/auction_frame.rs +++ b/crates/benilla-app/tests/auction_frame.rs @@ -61,6 +61,7 @@ fn load_ui_with_classes(s: &mut UiScript) { s.set_auction_item_classes(vec![AuctionCategory { class_id: 4, name: "Armor".into(), + has_subclass_filter: true, subclasses: vec![AuctionSubCategory { sub_id: 1, name: "Cloth".into(), @@ -717,7 +718,12 @@ fn search_reads_the_filters_and_nothing_queries_before_it() { assert_eq!(query.name, "linen"); assert_eq!(query.min_level, 10); assert_eq!(query.max_level, 20); - assert_eq!(query.class, Some(1), "the selected class row, 1-based"); + // The stock Lua hands over the class row's POSITION (1); the wire carries its item class id. + assert_eq!( + query.class, + Some(4), + "Armor is item class 4, not menu row 1" + ); assert!(query.usable_only); assert_eq!(query.page, 0); assert!(s.errors().is_empty(), "clean search: {:?}", s.errors()); diff --git a/crates/benilla-formats/src/itemsubclass.rs b/crates/benilla-formats/src/itemsubclass.rs index fbb32b4b..559e5e27 100644 --- a/crates/benilla-formats/src/itemsubclass.rs +++ b/crates/benilla-formats/src/itemsubclass.rs @@ -36,6 +36,9 @@ pub struct ItemSubClassInfo { /// prerequisite is −1. pub prerequisite_proficiency: i32, pub postrequisite_proficiency: i32, + /// `Flags@4` (`[row+0x10]`), raw. Its consumers each own the bit they read — e.g. `0x200`, + /// the auction house's "offers the inventory-slot rows" gate (`0x4cfb63`). + pub flags: u32, /// Bit 0 = never print the type name on the slot|type line. pub display_flags: u32, /// `WeaponSwingSize@9` — the swinging weight, 0 light · 1 medium · 2 heavy, and the sole @@ -170,6 +173,11 @@ impl ItemSubClassCatalog { .is_some_and(|r| r.display_flags & 1 != 0) } + /// This subclass's raw `Flags@4` (`0` for an unknown key) — see [`ItemSubClassInfo::flags`]. + pub fn flags(&self, class: u32, subclass: u32) -> u32 { + self.rows.get(&(class, subclass)).map_or(0, |r| r.flags) + } + /// This subclass's raw `DisplayFlags` (`0` for an unknown key). Bit 0 is [`Self::hides_name`]; /// bit 1 marks a subclass the auction house's category filter does not offer; **bit 2 (`0x4`) /// is the "this bag counts what is inside it" gate** — `GetInventoryItemCount` (`0x4c881a`– @@ -183,18 +191,16 @@ impl ItemSubClassCatalog { .map_or(0, |r| r.display_flags) } - /// Every subclass id defined for `class`, ascending. The shipped table is small (72 rows - /// total), so the scan costs nothing and the alternative — assuming subclass ids are dense - /// from 0 — is false for several classes. + /// Every subclass id defined for `class`, in **file order** — the order the reference's own + /// linear scans walk (the auction house's `0x4cf9c0`/`0x4ce980` count the Nth matching row). + /// The shipped table is small (72 rows total), so the scan costs nothing and the alternative — + /// assuming subclass ids are dense from 0 — is false for several classes. pub fn subclasses_of(&self, class: u32) -> Vec { - let mut ids: Vec = self - .rows - .keys() + self.order + .iter() .filter(|(c, _)| *c == class) - .map(|(_, s)| *s) - .collect(); - ids.sort_unstable(); - ids + .map(|&(_, s)| s) + .collect() } /// Number of rows (for logging/diagnostics). @@ -293,6 +299,7 @@ pub fn load_item_sub_classes(chain: &mut Chain) -> Result { ItemSubClassInfo { prerequisite_proficiency: i32_at(r, 2).unwrap_or(-1), postrequisite_proficiency: i32_at(r, 3).unwrap_or(-1), + flags: u32_at(r, 4).unwrap_or(0), display_flags: u32_at(r, 5).unwrap_or(0), weapon_swing_size: u32_at(r, 9).unwrap_or(0), }, diff --git a/crates/benilla-protocol/src/messages/update_object/fields/unit.rs b/crates/benilla-protocol/src/messages/update_object/fields/unit.rs index b41f2084..e37ca292 100644 --- a/crates/benilla-protocol/src/messages/update_object/fields/unit.rs +++ b/crates/benilla-protocol/src/messages/update_object/fields/unit.rs @@ -158,7 +158,16 @@ impl ObjectFields { /// carry a non-zero number, while **guardians and totems carry 0** and therefore keep whatever /// rank their template gives them. pub fn unit_is_pet_or_charm(&self) -> bool { - self.get_u32(FIELD_UNIT_PETNUMBER).unwrap_or(0) != 0 + self.unit_pet_number() != 0 + } + /// `UNIT_FIELD_PETNUMBER` as a value — `0` when absent (a create omits zeros). Besides the + /// boolean read above, the reference **keys a unit's name on it**: `CGUnit_C::GetUnitName` + /// (`0x609210`) reads `[[unit+0x110]+0x214]` and, when non-zero, looks the name up in the + /// pet-name cache under *this value*; zero means the creature template's name. The number in a + /// `HIGHGUID_PET` guid is not a substitute — a companion pet's guid carries a pet number the + /// server never files in its charm info, so it answers no pet-name query for it. + pub fn unit_pet_number(&self) -> u32 { + self.get_u32(FIELD_UNIT_PETNUMBER).unwrap_or(0) } /// `UNIT_FIELD_PET_NAME_TIMESTAMP` — when this pet's name was last set. `None` when the field /// has never been sent (a create omits zeros), which reads the same as "never renamed". diff --git a/crates/benilla-ui/src/script/auction.rs b/crates/benilla-ui/src/script/auction.rs index db6cae00..89edb389 100644 --- a/crates/benilla-ui/src/script/auction.rs +++ b/crates/benilla-ui/src/script/auction.rs @@ -149,7 +149,13 @@ pub struct AuctionCategory { /// The wire's `mainCategory` — an item class id, **not** the menu position. pub class_id: u32, pub name: String, - /// `(subclass id, name, offers inventory-slot filters)`. + /// The `0x807060` table's second column: whether the class offers subclass rows at all. + /// `false` (Consumable, Trade Goods, Reagent, Miscellaneous) makes `GetAuctionItemSubClasses` + /// and `GetAuctionInvTypes` answer nothing — but `QueryAuctionItems` never reads it, so + /// [`Self::subclasses`] is still the class's full non-excluded list either way. + pub has_subclass_filter: bool, + /// The class's `ItemSubClass.dbc` rows whose `DisplayFlags & 2` is clear, in file order — + /// the list both the menu and the query's index scan walk. pub subclasses: Vec, } @@ -159,9 +165,8 @@ pub struct AuctionSubCategory { pub sub_id: u32, pub name: String, /// Whether selecting this subclass offers the 14 inventory-slot rows beneath it - /// (`GetAuctionInvTypes`). INTERIM (decision 1511): the exact predicate is pinned to the - /// in-flight §5's TU-4, and lives here as *data* precisely so correcting it is a one-line - /// change app-side rather than an engine change. + /// (`GetAuctionInvTypes`) — the row's `ItemSubClass.Flags & 0x200` (`0x4cfb63`), read + /// app-side off the player's own DBC. pub has_inv_types: bool, } @@ -176,15 +181,17 @@ pub struct AuctionState { pub deposit_percent: u32, } -/// A drained `QueryAuctionItems` — the Browse search, exactly as the reference builds it. The app -/// maps this to `CMSG_AUCTION_LIST_ITEMS` (decision 1511 P1). +/// A drained `QueryAuctionItems` — the Browse search, exactly as the reference builds it: every +/// filter is already the wire's item-template id (the binding maps the Lua's menu positions, as +/// `0x4ce980` does). The app copies this into `CMSG_AUCTION_LIST_ITEMS` (decision 1511 P1). #[derive(Clone, Debug, Default, PartialEq)] pub struct AuctionQuery { pub name: String, /// `0` = no filter, for both. pub min_level: u32, pub max_level: u32, - /// `None` = no filter (the wire's `0xFFFFFFFF` sentinel). + /// `None` = no filter (the wire's `0xFFFFFFFF` sentinel). Ids, never menu positions: an + /// `InventoryType`, an item class, a subclass of that class. pub inv_type: Option, pub class: Option, pub sub_class: Option, @@ -578,7 +585,8 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { )?; // GetAuctionItemSubClasses(classIndex) → the subclass names under a 1-based menu class. An - // out-of-range index returns nothing, which is the reference's own bound-with-no-error. + // out-of-range index, or a class whose `0x807060` flag is 0, returns nothing — the + // reference's own bound-with-no-error (`0x4cfa00`, `0x4cfa0e`). g.set( "GetAuctionItemSubClasses", lua.create_function(|lua, class_index: usize| { @@ -587,6 +595,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { class_index .checked_sub(1) .and_then(|i| model.auction_item_classes.get(i)) + .filter(|c| c.has_subclass_filter) .map_or_else(Vec::new, |c| { c.subclasses.iter().map(|s| s.name.clone()).collect() }) @@ -600,9 +609,11 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { )?; // GetAuctionInvTypes(classIndex, subclassIndex) → the 14 inventory-slot GlobalString KEYS, or - // nothing. The pair only decides WHETHER the list is offered — the list itself is fixed. Which - // pairs offer it is pushed as data (`has_inv_types`), so decision 1511's INTERIM on that - // predicate is corrected app-side, not here. + // nothing. The pair only decides WHETHER the list is offered — the list itself is fixed. + // `0x4cfab0`: no class (index out of range, or its `0x807060` flag 0) → nothing; a FOUND + // subclass offers the list only with `ItemSubClass.Flags & 0x200` (pushed as + // `has_inv_types`); and a subclass index the scan runs off the end of skips that gate and + // offers all fourteen (wow-re auction-house §9.3 — the exhaust edge lands past the test). g.set( "GetAuctionInvTypes", lua.create_function(|lua, (class_index, sub_index): (usize, usize)| { @@ -611,14 +622,19 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { class_index .checked_sub(1) .and_then(|i| model.auction_item_classes.get(i)) - .and_then(|c| sub_index.checked_sub(1).and_then(|i| c.subclasses.get(i))) - .is_some_and(|s| s.has_inv_types) + .filter(|c| c.has_subclass_filter) + .is_some_and(|c| { + sub_index + .checked_sub(1) + .and_then(|i| c.subclasses.get(i)) + .is_none_or(|s| s.has_inv_types) + }) }; if !offers { return Ok(MultiValue::new()); } let mut out = Vec::with_capacity(AUCTION_INV_TYPES.len()); - for key in AUCTION_INV_TYPES { + for (_, key) in AUCTION_INV_TYPES { out.push(Value::String(lua.create_string(key)?)); } Ok(MultiValue::from_vec(out)) @@ -627,8 +643,11 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { // QueryAuctionItems(name, minLevel, maxLevel, invTypeIndex, classIndex, subclassIndex, page, // isUsable, qualityIndex) — nine arguments (TBC's tenth, exactMatch, does not exist on 5875). - // The level boxes hand us strings, and every filter index is 1-based into the pushed tables; - // the app turns those into the wire's class/subclass ids and its 0xFFFFFFFF sentinels. + // The level boxes hand us strings. Every filter argument is a 1-based MENU POSITION, and — + // exactly as the reference's `0x4ce980` does — it is turned into the wire's item-template id + // here, before anything is queued: invType through the `0x8070b0` rows, class through the + // `0x807060` table, subclass as the Nth non-excluded subclass row of that class (a scan that + // never reads the class's filter flag). Anything that does not resolve is the wire's "any". g.set( "QueryAuctionItems", lua.create_function(|lua, args: MultiValue| { @@ -658,14 +677,26 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { Some(Value::Number(n)) if *n >= 0.0 => Some(*n as u32), _ => None, }; + // A 1-based position into a table, `None` for a missing, zero or negative argument. + let position = |i: usize| num(i).and_then(|n| (n as usize).checked_sub(1)); let mut model = lua.app_data_mut::().expect("model app_data"); + let inv_type = position(3) + .and_then(|i| AUCTION_INV_TYPES.get(i)) + .map(|&(id, _)| id); + let category = position(4).and_then(|i| model.auction_item_classes.get(i)); + let class = category.map(|c| c.class_id); + let sub_class = category.and_then(|c| { + position(5) + .and_then(|i| c.subclasses.get(i)) + .map(|s| s.sub_id) + }); model.auction_query = Some(AuctionQuery { name, min_level: num(1).unwrap_or(0), max_level: num(2).unwrap_or(0), - inv_type: num(3).filter(|n| *n > 0), - class: num(4).filter(|n| *n > 0), - sub_class: num(5).filter(|n| *n > 0), + inv_type, + class, + sub_class, quality, page: num(6).unwrap_or(0), usable_only: truthy(7), @@ -823,24 +854,25 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { Ok(()) } -/// The fourteen inventory-slot rows the reference offers, as GlobalString **keys** — the Lua -/// resolves each through the player's own strings, so no English ships here. Inventory ids 1..12 -/// plus CLOAK(16) and HOLDABLE(23); the order is the reference's own. -const AUCTION_INV_TYPES: [&str; 14] = [ - "INVTYPE_HEAD", - "INVTYPE_NECK", - "INVTYPE_SHOULDER", - "INVTYPE_BODY", - "INVTYPE_CHEST", - "INVTYPE_WAIST", - "INVTYPE_LEGS", - "INVTYPE_FEET", - "INVTYPE_WRIST", - "INVTYPE_HAND", - "INVTYPE_FINGER", - "INVTYPE_TRINKET", - "INVTYPE_CLOAK", - "INVTYPE_HOLDABLE", +/// The fourteen inventory-slot rows the reference offers — the `0x8070b0` table's +/// `{u32 invTypeId; char name[0x20]}` rows, in its own order. The name is a GlobalString **key** +/// the Lua resolves through the player's own strings, so no English ships here; the id is what +/// `QueryAuctionItems` puts on the wire for the row's 1-based position. +const AUCTION_INV_TYPES: [(u32, &str); 14] = [ + (1, "INVTYPE_HEAD"), + (2, "INVTYPE_NECK"), + (3, "INVTYPE_SHOULDER"), + (4, "INVTYPE_BODY"), + (5, "INVTYPE_CHEST"), + (6, "INVTYPE_WAIST"), + (7, "INVTYPE_LEGS"), + (8, "INVTYPE_FEET"), + (9, "INVTYPE_WRIST"), + (10, "INVTYPE_HAND"), + (11, "INVTYPE_FINGER"), + (12, "INVTYPE_TRINKET"), + (16, "INVTYPE_CLOAK"), + (23, "INVTYPE_HOLDABLE"), ]; /// Attach the cursor's held item to the sell slot, or — with an empty cursor — pick the attached @@ -1028,6 +1060,105 @@ mod tests { assert_eq!(deposit_for(5, 10_000, 60), 0); } + /// The browse tree as the reference's `0x807060` table shapes it: Weapon (filterable) first, + /// then Armor, then Consumable (flag 0 — no subclass rows offered, but its subclasses still + /// exist for the query's own scan). + fn tree() -> Vec { + let sub = |sub_id: u32, name: &str, has_inv_types: bool| AuctionSubCategory { + sub_id, + name: name.into(), + has_inv_types, + }; + vec![ + AuctionCategory { + class_id: 2, + name: "Weapon".into(), + has_subclass_filter: true, + subclasses: vec![sub(0, "Axe", false), sub(1, "Axe", false)], + }, + AuctionCategory { + class_id: 4, + name: "Armor".into(), + has_subclass_filter: true, + subclasses: vec![sub(1, "Cloth", true), sub(6, "Shield", false)], + }, + AuctionCategory { + class_id: 0, + name: "Consumable".into(), + has_subclass_filter: false, + subclasses: vec![sub(0, "Consumable", false)], + }, + ] + } + + /// `QueryAuctionItems` takes menu POSITIONS and puts item-template IDS on the wire — the + /// reference does the mapping itself (`0x4ce980`: invType through `0x8070b0`, class through + /// `0x807060`, subclass as the Nth non-excluded `ItemSubClass.dbc` row of that class). Sending + /// the position instead asked vmangos for class 1 (Container) when the player picked Weapon. + #[test] + fn query_sends_ids_not_menu_positions() { + let mut s = UiScript::new().unwrap(); + s.set_auction_item_classes(tree()); + + // Weapon / its first subclass / Back (the 13th inventory row). + s.run(r#"QueryAuctionItems("", "", "", 13, 1, 1, 0, nil, -1)"#) + .unwrap(); + let q = s.take_auction_query().expect("queued"); + assert_eq!(q.inv_type, Some(16), "INVTYPE_CLOAK is inventory type 16"); + assert_eq!(q.class, Some(2), "Weapon is item class 2, not position 1"); + assert_eq!(q.sub_class, Some(0), "the first weapon subclass is id 0"); + + // Armor / its SECOND subclass skips the id gap: Shield is 6, not 2. + s.run(r#"QueryAuctionItems("", "", "", 14, 2, 2, 0, nil, -1)"#) + .unwrap(); + let q = s.take_auction_query().expect("queued"); + assert_eq!( + (q.inv_type, q.class, q.sub_class), + (Some(23), Some(4), Some(6)) + ); + + // The query's scan ignores the class's filter flag: Consumable offers no subclass rows, + // yet an index still resolves against its (non-excluded) subclasses. + s.run(r#"QueryAuctionItems("", "", "", 0, 3, 1, 0, nil, -1)"#) + .unwrap(); + let q = s.take_auction_query().expect("queued"); + assert_eq!((q.inv_type, q.class, q.sub_class), (None, Some(0), Some(0))); + + // Out of range, or no class to scan under: the wire's "any". + s.run(r#"QueryAuctionItems("", "", "", 15, 11, 1, 0, nil, -1)"#) + .unwrap(); + let q = s.take_auction_query().expect("queued"); + assert_eq!((q.inv_type, q.class, q.sub_class), (None, None, None)); + s.run(r#"QueryAuctionItems("", "", "", nil, 1, 3, 0, nil, -1)"#) + .unwrap(); + let q = s.take_auction_query().expect("queued"); + assert_eq!((q.inv_type, q.class, q.sub_class), (None, Some(2), None)); + } + + /// `0x4cf9c0` / `0x4cfab0` return nothing for a class whose `0x807060` flag is 0, and + /// `0x4cfab0` gates a FOUND subclass on `ItemSubClass.Flags & 0x200` but pushes all fourteen + /// when the scan runs off the end (wow-re auction-house §9.1, §9.3). + #[test] + fn subclass_and_inv_type_menus_follow_the_reference_gates() { + let mut s = UiScript::new().unwrap(); + s.set_auction_item_classes(tree()); + assert_eq!(s.arity("GetAuctionItemSubClasses(1)").unwrap(), 2); + assert_eq!( + s.arity("GetAuctionItemSubClasses(3)").unwrap(), + 0, + "Consumable's flag is 0: no subclass rows" + ); + assert_eq!(s.arity("GetAuctionInvTypes(2, 1)").unwrap(), 14, "Cloth"); + assert_eq!(s.arity("GetAuctionInvTypes(2, 2)").unwrap(), 0, "Shield"); + assert_eq!(s.arity("GetAuctionInvTypes(3, 1)").unwrap(), 0, "flag 0"); + assert_eq!(s.arity("GetAuctionInvTypes(11, 1)").unwrap(), 0, "no class"); + assert_eq!( + s.arity("GetAuctionInvTypes(1, 9)").unwrap(), + 14, + "an exhausted scan skips the gate" + ); + } + /// The sort stack answers for any key it remembers, not just the primary — the reason a /// non-primary column's arrow can still point down. #[test] diff --git a/crates/benilla-ui/src/script/guild.rs b/crates/benilla-ui/src/script/guild.rs index f8d6e08d..75b25de5 100644 --- a/crates/benilla-ui/src/script/guild.rs +++ b/crates/benilla-ui/src/script/guild.rs @@ -403,6 +403,19 @@ fn live_rights(model: &Model, rank_one_based: u32) -> u32 { .unwrap_or(0) } +/// The rank-name gate `GuildControlSaveRank` (`0x4d20d0`) and `GuildControlAddRank` (`0x4d2210`) +/// both run before any packet, byte-for-byte the same: the C string (so up to the first NUL) must +/// be non-empty and `0x65b250(name) - 1` — its UTF-16 code-unit count — must lie in `1..=16`. +/// Nothing else: no trim, so a whitespace name passes. The name the gate measured is the name +/// sent. vmangos's own cap is 15 code points and it kicks over it; that server-side limit is the +/// app's to enforce at the send (`ui_guild::feed::capped_rank_name`), not the binding's. +fn gated_rank_name(name: &str) -> Option<&str> { + let c_name = name.split('\0').next().unwrap_or_default(); + (1..=16) + .contains(&c_name.encode_utf16().count()) + .then_some(c_name) +} + /// Register the guild globals against the snapshot store. pub(super) fn install(lua: &Lua) -> mlua::Result<()> { let g = lua.globals(); @@ -852,7 +865,9 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { // GuildControlSaveRank(name) (`0x4d20d0`) — flush the staged edits as one CMSG_GUILD_RANK. // The argument is the rank's (possibly renamed) NAME, taken from the popup's edit box // (`FriendsFrame.lua:839`); the rank it applies to is whichever GuildControlSetRank loaded. - // Converted to the wire's 0-based rank id here, once. + // Converted to the wire's 0-based rank id here, once. A name that fails + // [`gated_rank_name`] — empty above all, which vmangos would otherwise persist as the rank's + // name guild-wide — is a silent no-op, before any packet. // // **The fold is a masked set-or-clear over a freshly re-read LIVE baseline** — verified: the // binding does `or mask[k]` / `and ~mask[k]` per flag against the current rights, never a @@ -865,9 +880,14 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { g.set( "GuildControlSaveRank", lua.create_function(|lua, name: String| { + let Some(name) = gated_rank_name(&name).map(str::to_string) else { + return Ok(()); + }; let mut model = lua.app_data_mut::().expect("model app_data"); let edit = model.guild_control.clone(); - if edit.rank >= 1 { + // `[0x84966c] >= [0xb73120]`: no rank loaded (the reference's -1, our 0) or one at or + // past the rank count. + if edit.rank >= 1 && (edit.rank as usize) <= model.guild.ranks.len() { let mut rights = live_rights(&model, edit.rank); for (i, bit) in RANK_RIGHT_BITS.iter().enumerate() { if edit.staged[i] { @@ -886,14 +906,18 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; - // GuildControlAddRank(name) (`0x4d2210`) — append a rank. The client refuses at + // GuildControlAddRank(name) (`0x4d2210`) — append a rank, behind the same silent name gate + // as SaveRank ([`gated_rank_name`]). The client refuses at // `numRanks >= MAX_RANKS` **silently**, before any packet, which is why the reference's Add // button greys itself at ten (`FriendsFrame.lua:887`) rather than relying on a refusal. g.set( "GuildControlAddRank", lua.create_function(|lua, name: String| { + let Some(name) = gated_rank_name(&name).map(str::to_string) else { + return Ok(()); + }; let mut model = lua.app_data_mut::().expect("model app_data"); - if !name.trim().is_empty() && model.guild.ranks.len() < MAX_RANKS { + if model.guild.ranks.len() < MAX_RANKS { model.guild_requests.push(GuildRequest::AddRank(name)); } Ok(()) @@ -1020,6 +1044,78 @@ mod tests { ); } + /// A guild with `n` named ranks. + fn guild_with_ranks(n: usize) -> crate::script::UiScript { + let mut s = crate::script::UiScript::new().unwrap(); + s.set_guild(GuildState { + in_guild: true, + ranks: (0..n) + .map(|i| GuildRankInfo { + name: format!("Rank{i}"), + rights: 0, + }) + .collect(), + ..Default::default() + }); + s + } + + /// **`0x4d20d0` refuses a bad name silently, before any packet** — NULL, empty, or a length + /// outside 1..16 UTF-16 code units (`0x65b250` counts units plus the terminator), and a rank + /// index at or past the rank count. vmangos persists whatever name arrives — an empty one + /// blanks the rank guild-wide — so this gate is the only thing standing in the way. + #[test] + fn saving_a_rank_refuses_what_the_reference_refuses() { + let mut s = guild_with_ranks(5); + let saves = |s: &mut crate::script::UiScript| { + s.take_guild_requests() + .into_iter() + .filter(|r| matches!(r, GuildRequest::SaveRank { .. })) + .count() + }; + + s.run(r#"GuildControlSetRank(2); GuildControlSaveRank("")"#) + .unwrap(); + assert_eq!(saves(&mut s), 0, "an empty name is a silent no-op"); + + // Sixteen UTF-16 units is the ceiling; seventeen is refused. + s.run(r#"GuildControlSetRank(2); GuildControlSaveRank(string.rep("a", 16))"#) + .unwrap(); + assert_eq!(saves(&mut s), 1); + s.run(r#"GuildControlSetRank(2); GuildControlSaveRank(string.rep("a", 17))"#) + .unwrap(); + assert_eq!(saves(&mut s), 0, "seventeen units"); + // Units, not bytes: sixteen two-byte letters are sixteen units. + s.run(r#"GuildControlSetRank(2); GuildControlSaveRank(string.rep("é", 16))"#) + .unwrap(); + assert_eq!(saves(&mut s), 1, "32 bytes, 16 units"); + // Whitespace is not empty to the reference. + s.run(r#"GuildControlSetRank(2); GuildControlSaveRank(" ")"#) + .unwrap(); + assert_eq!(saves(&mut s), 1); + + // No rank loaded (a fresh popup — the selection persists across saves, as + // `[0x84966c]` does), or one past the ladder. + let mut fresh = guild_with_ranks(5); + fresh.run(r#"GuildControlSaveRank("Officer")"#).unwrap(); + assert_eq!(saves(&mut fresh), 0, "nothing selected"); + s.run(r#"GuildControlSetRank(6); GuildControlSaveRank("Officer")"#) + .unwrap(); + assert_eq!(saves(&mut s), 0, "rank index 5 >= 5 ranks"); + } + + /// `0x4d2210` runs the same name gate as `SaveRank` — and nothing more: a whitespace name is + /// not empty to it. + #[test] + fn adding_a_rank_runs_the_same_name_gate() { + let mut s = guild_with_ranks(5); + s.run(r#"GuildControlAddRank(""); GuildControlAddRank(string.rep("a", 17))"#) + .unwrap(); + assert!(s.take_guild_requests().is_empty()); + s.run(r#"GuildControlAddRank(" ")"#).unwrap(); + assert_eq!(s.take_guild_requests().len(), 1); + } + /// A 1-based index past the end, and the 0 the reference passes on every `GuildStatus_Update` /// before it checks the selection, both have to answer `None` rather than panic. #[test] diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index 4ad327d9..a498e00f 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -1448,16 +1448,16 @@ pub(crate) struct Model { pub(crate) death_actions: Vec, /// The quest-log seam ([`quest_log`]): the snapshot the app pushes, the engine-owned - /// synchronous selection + click-time abandon mark (1-based entry indices, `0` = none), and - /// the drained abandon intents. + /// synchronous selection (a 1-based entry index, `0` = none), the click-time abandon mark (a + /// QUEST ID, `0` = none — the reference's `0xbb7484`), and the drained abandon intents (quest + /// ids). pub(crate) quest_log: quest_log::QuestLogState, pub(crate) quest_log_selection: u32, pub(crate) quest_log_abandon_mark: u32, pub(crate) quest_log_abandons: Vec, /// Quest ids `QuestLogPushQuest()` queued for the app to push to the party (decision 1733). - /// **Ids, not entry indices** — unlike the abandon mark above, whose two-step confirm is the - /// whole reason it pins an index: a push is one click, so the id is resolved right then and a - /// log shuffle between click and drain cannot retarget it. + /// **Ids, not entry indices**, like the abandon mark above: the id is resolved at click time, + /// so a log shuffle between click and drain cannot retarget it. pub(crate) quest_log_pushes: Vec, /// How many times `ConfirmAcceptQuest()` was called — the escort confirm's Yes. A counter, not /// a quest id: the reference's verb takes no argument and answers whatever confirm the client diff --git a/crates/benilla-ui/src/script/quest.rs b/crates/benilla-ui/src/script/quest.rs index a151f789..561e544e 100644 --- a/crates/benilla-ui/src/script/quest.rs +++ b/crates/benilla-ui/src/script/quest.rs @@ -160,18 +160,25 @@ pub enum QuestAction { /// Reward panel Complete → `CMSG_QUESTGIVER_CHOOSE_REWARD` with the chosen choice index (0 when /// the quest has no choice rewards). Reward(u32), - /// **The quest session's end**, however it was reached: `DeclineQuest()` (the detail panel's - /// Decline, the progress/reward panels' Cancel, the greeting's Goodbye) and `CloseQuest()` - /// (the window's own OnHide — ESC, a UIPanel eviction) alike. + /// `DeclineQuest()` — the detail panel's Decline and the progress/reward panels' Cancel + /// (stock `QuestFrame.lua`: `QuestDetailDeclineButton_OnClick`, `QuestGoodbyeButton_OnClick`, + /// `QuestRewardCancelButton_OnClick`). Its binding `0x501d30` calls `0x5013f0`, the ONLY + /// routine that re-opens the giver: a four-way fork on the source's object type — a + /// gossip-flagged unit → `CMSG_GOSSIP_HELLO`, a plain unit → `CMSG_QUESTGIVER_HELLO`, an + /// item/player (or the `0xbe0824` latch set) → the teardown `0x501130(0,1)`, a GameObject → + /// its interact virtual. The app owns the fork, because only it knows the giver. + Decline, + /// `CloseQuest()` — the window's own `QuestFrame_OnHide`: ESC, the close button, the + /// greeting's Goodbye (`HideUIPanel(QuestFrame)`), a UIPanel eviction. Its binding `0x501a10` + /// calls the teardown `0x501130(0,1)` and **nothing else**; that routine's one send is + /// `MSG_QUEST_PUSH_RESULT{DECLINE}` for a PLAYER source, so closing an NPC's window is + /// network-silent (wow-re `system/ui/scratch/quest-share-flow.md` §4.1 caller table, §6). /// - /// **One action, because the reference has one routine.** 1733 briefly split this into - /// `Decline` and `Close` on the assumption that only the first was an answer on the wire; the - /// §5 that landed after it found both Lua verbs calling `0x501130`, along with the walk-away - /// watchdog and the leave-world teardown, and that routine both answers a share - /// (`MSG_QUEST_PUSH_RESULT{DECLINE_QUEST}`) and re-opens an NPC's list - /// (`CMSG_QUESTGIVER_HELLO`/`CMSG_GOSSIP_HELLO`). Which of the two it does is decided by the - /// GIVER, never by the button — so the app owns that fork and the engine reports one intent - /// (decision 1738). + /// **Two actions, because the reference has two routines.** 1738 merged this into + /// `DeclineQuest`'s action on the reading that both verbs are one routine; the note's caller + /// table shows they share only the teardown, and the HELLO re-open lives in `DeclineQuest`'s + /// fork alone. The merge made ESC on an NPC re-open its list — a multi-quest greeting could + /// not be closed. Close, } @@ -409,10 +416,11 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { })?, ) } - // AcceptQuest → Accept; DeclineQuest / CloseQuest → Close (the ONE session-end routine, see - // `QuestAction::Close`); CompleteQuest (the progress panel's Continue) → Continue. + // AcceptQuest → Accept; DeclineQuest → Decline (the re-opening fork `0x5013f0`); CloseQuest → + // Close (the teardown `0x501130` alone — see `QuestAction::Close`); CompleteQuest (the + // progress panel's Continue) → Continue. install_action(lua, "AcceptQuest", super::QuestAction::Accept)?; - install_action(lua, "DeclineQuest", super::QuestAction::Close)?; + install_action(lua, "DeclineQuest", super::QuestAction::Decline)?; install_action(lua, "CloseQuest", super::QuestAction::Close)?; install_action(lua, "CompleteQuest", super::QuestAction::Continue)?; @@ -731,7 +739,7 @@ mod tests { QuestAction::Continue, QuestAction::Reward(1), QuestAction::Reward(0), - QuestAction::Close, + QuestAction::Decline, ] ); assert!(s.take_quest_actions().is_empty(), "drained"); @@ -747,19 +755,18 @@ mod tests { assert!(s.eval::("return GetRewardSpell() == nil").unwrap()); } - /// `DeclineQuest` and `CloseQuest` are the SAME intent — one reference routine, `0x501130` - /// (decision 1738, correcting 1733's split). What differs between a decline and a close is - /// nothing; what differs between an NPC's quest and a shared one is everything, and that fork - /// belongs to the app, which knows the giver. + /// `DeclineQuest` and `CloseQuest` are TWO intents — two reference routines (`0x5013f0` and + /// `0x501a10`) that share only the teardown `0x501130`; the giver re-open is `DeclineQuest`'s + /// alone (wow-re `quest-share-flow.md` §4.1/§6). #[test] - fn decline_and_close_are_one_intent() { + fn decline_and_close_are_two_intents() { let mut s = UiScript::new().unwrap(); s.set_quest(Some(detail())); s.run("DeclineQuest()").unwrap(); s.run("CloseQuest()").unwrap(); assert_eq!( s.take_quest_actions(), - vec![QuestAction::Close, QuestAction::Close] + vec![QuestAction::Decline, QuestAction::Close] ); } diff --git a/crates/benilla-ui/src/script/quest_log.rs b/crates/benilla-ui/src/script/quest_log.rs index 4fccd981..1e965330 100644 --- a/crates/benilla-ui/src/script/quest_log.rs +++ b/crates/benilla-ui/src/script/quest_log.rs @@ -18,8 +18,10 @@ //! one detail per push instead made a whole log walk answer with a single row's text. //! - **The abandon mark** (`SetAbandonQuest`/`GetAbandonQuestName`/`AbandonQuest`) — the ref's //! two-step confirm (mark on button click, act on the popup's Yes — ref `QuestLogFrame.xml:463-472`, -//! `StaticPopup.lua:749-761`). The mark pins the *entry index at click time* so a log shuffle -//! between click and confirm can't retarget the abandon. +//! `StaticPopup.lua:749-761`). The mark is the selected **quest id** at click time, as the +//! reference's is (`0x4dfb50` copies the selection `0xbb7480`, which holds a quest id, into +//! `0xbb7484`), so a log that re-indexes between click and confirm — a fold, a quest arriving +//! or leaving — can't retarget the abandon. //! //! A third engine-owned bit joined later: the **watch set** (`IsQuestWatched`/`AddQuestWatch`/ //! `RemoveQuestWatch`/`GetNumQuestWatches`/`GetQuestIndexForWatch`) — keyed by the entries' stable @@ -208,9 +210,9 @@ impl super::UiScript { self.model_ref().quest_log_selection } - /// Drain the abandon intents: the 1-based entry index pinned by `SetAbandonQuest` at - /// click time, confirmed by the popup's `AbandonQuest()`. The app maps index → descriptor - /// slot → `CMSG_QUESTLOG_REMOVE_QUEST`. + /// Drain the abandon intents: the QUEST ID `SetAbandonQuest` marked at click time, confirmed + /// by the popup's `AbandonQuest()`. The app maps id → descriptor slot → + /// `CMSG_QUESTLOG_REMOVE_QUEST`. pub fn take_quest_log_abandons(&mut self) -> Vec { std::mem::take(&mut self.model_mut().quest_log_abandons) } @@ -593,25 +595,38 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; - // SetAbandonQuest() — pin the current selection as the abandon target (ref - // QuestLogFrame.xml:464). + // SetAbandonQuest() — mark the selected QUEST as the abandon target (ref + // QuestLogFrame.xml:464). The reference's `0x4dfb50` is `mov eax,[0xbb7480]; mov [0xbb7484],eax`: + // it copies the selection, and the selection is a quest id (`0x4def30` stores the row's +0x0 + // at `0x4def5d`). Ours holds the selection as a row index, so the id is resolved here, at the + // click — never at the confirm, when the rows may have moved. g.set( "SetAbandonQuest", lua.create_function(|lua, ()| { let mut model = lua.app_data_mut::().expect("model app_data"); - model.quest_log_abandon_mark = model.quest_log_selection; + let sel = model.quest_log_selection as usize; + model.quest_log_abandon_mark = sel + .checked_sub(1) + .and_then(|n| model.quest_log.entries.get(n)) + .filter(|e| !e.is_header) + .map_or(0, |e| e.quest_id); Ok(()) })?, )?; - // GetAbandonQuestName() → the pinned target's title ("" when none — the popup shows it). + // GetAbandonQuestName() → the marked quest's title ("" when none — the popup shows it). The + // reference (`0x4dfb60`) peeks its quest cache by the marked id; the rows carry the same + // cached title. g.set( "GetAbandonQuestName", lua.create_function(|lua, ()| { let title = { let model = lua.app_data_ref::().expect("model app_data"); - (model.quest_log_abandon_mark as usize) - .checked_sub(1) - .and_then(|n| model.quest_log.entries.get(n)) + let mark = model.quest_log_abandon_mark; + model + .quest_log + .entries + .iter() + .find(|e| mark != 0 && !e.is_header && e.quest_id == mark) .map(|e| e.title.clone()) .unwrap_or_default() }; @@ -624,13 +639,25 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { "GetAbandonQuestItems", lua.create_function(|_, ()| Ok(Value::Nil))?, )?; - // AbandonQuest() — queue the pinned target as an outbound intent (the popup's Yes). + // AbandonQuest() — the popup's Yes (`0x4dfe00` → `0x4df070`). The reference searches its + // whole row table `0xbb71c0` (folded quests included) for a quest row carrying the marked id; + // a hit sends that row's slot and clears the mark, a miss does nothing and KEEPS the mark. + // Ours checks the same membership — a visible quest row, or one folded under a collapsed + // header — and queues the id; the app resolves it to the slot. g.set( "AbandonQuest", lua.create_function(|lua, ()| { let mut model = lua.app_data_mut::().expect("model app_data"); - let mark = std::mem::take(&mut model.quest_log_abandon_mark); - if mark != 0 { + let mark = model.quest_log_abandon_mark; + let in_log = mark != 0 + && (model + .quest_log + .entries + .iter() + .any(|e| !e.is_header && e.quest_id == mark) + || model.quest_log.hidden_quest_ids.contains(&mark)); + if in_log { + model.quest_log_abandon_mark = 0; model.quest_log_abandons.push(mark); } Ok(()) @@ -1228,9 +1255,10 @@ mod tests { s.eval::("return GetAbandonQuestName()").unwrap(), "Kobold Camp Cleanup" ); - // Selection moves before the confirm — the mark must not follow it. + // Selection moves before the confirm — the mark must not follow it. What queues is the + // marked QUEST ID (entry 2 is quest 7), never a row index (0x4dfb50). s.run("SelectQuestLogEntry(1); AbandonQuest()").unwrap(); - assert_eq!(s.take_quest_log_abandons(), vec![2]); + assert_eq!(s.take_quest_log_abandons(), vec![7]); assert!(s.take_quest_log_abandons().is_empty(), "drained"); // A bare AbandonQuest with no mark queues nothing. s.run("AbandonQuest()").unwrap(); @@ -1463,10 +1491,56 @@ mod tests { ); } + /// The abandon mark follows its QUEST across a re-index, and a mark whose quest has left the + /// log is a no-op that is KEPT, as `0x4df070`'s miss path is (`ret` before the clear at + /// `0x4df0cf`). A quest folded under a collapsed header is still in the log. + #[test] + fn abandon_mark_is_a_quest_id_that_survives_a_reindex() { + let mut s = UiScript::new().unwrap(); + s.set_quest_log(two_quests()); + s.run("SelectQuestLogEntry(2); SetAbandonQuest()").unwrap(); // quest 7 + + // The log re-indexes under the popup: quest 7 is now row 1, and a stranger takes row 2. + let mut moved = two_quests(); + moved.entries.swap(0, 1); + moved.entries[1].quest_id = 999; + s.set_quest_log(moved.clone()); + assert_eq!( + s.eval::("return GetAbandonQuestName()").unwrap(), + "Kobold Camp Cleanup" + ); + s.run("AbandonQuest()").unwrap(); + assert_eq!(s.take_quest_log_abandons(), vec![7]); + + // Folded away: still abandonable. + s.run("SelectQuestLogEntry(1); SetAbandonQuest()").unwrap(); + let mut folded = moved.clone(); + folded.entries.remove(0); + folded.hidden_quest_ids = vec![7]; + s.set_quest_log(folded); + s.run("AbandonQuest()").unwrap(); + assert_eq!(s.take_quest_log_abandons(), vec![7]); + + // Gone from the log: nothing queues, and the mark stays for the quest's return. + s.set_quest_log(moved.clone()); + s.run("SelectQuestLogEntry(1); SetAbandonQuest()").unwrap(); + let mut gone = moved.clone(); + gone.entries.remove(0); + s.set_quest_log(gone); + s.run("AbandonQuest()").unwrap(); + assert!(s.take_quest_log_abandons().is_empty()); + s.set_quest_log(moved); + s.run("AbandonQuest()").unwrap(); + assert_eq!( + s.take_quest_log_abandons(), + vec![7], + "the kept mark still names 7" + ); + } + /// `QuestLogPushQuest` queues the selected entry's **quest id**, resolved at click time — not /// its index, so a log that reshuffles between the click and the app's drain cannot retarget - /// the push. (Contrast the abandon mark, which pins an index on purpose: its two-step confirm - /// is what makes the index the right thing to hold.) + /// the push — the same keying as the abandon mark. /// A party of one other, so the C verb's own party check (below) passes. fn in_a_party(s: &mut UiScript) { s.set_party(crate::script::PartyState {