diff --git a/crates/benilla-app/src/login/mod.rs b/crates/benilla-app/src/login/mod.rs index 4a04ee19..446dc758 100644 --- a/crates/benilla-app/src/login/mod.rs +++ b/crates/benilla-app/src/login/mod.rs @@ -32,7 +32,7 @@ use crate::textinput::{self, HostClipboard}; use bevy::input::ButtonState; use bevy::prelude::*; -use benilla_protocol::LoginStage; +use benilla_protocol::{DialFailure, LoginRefusal, LoginStage}; use crate::char_select::ClientState; use crate::glue_strings::GlueStrings; @@ -190,7 +190,72 @@ fn stage_text(strings: &GlueStrings, stage: LoginStage) -> &str { /// The authored failure string for an auth result byte (the vmangos-verified map, decision 0539 /// §6): each row is the client's own `GlueStrings` text; a transport failure (`None`) reads /// `LOGIN_FAILED` ("Unable to connect"). -fn fail_text(strings: &GlueStrings, code: Option) -> &str { +/// +/// **A dial that never opened a socket answers ahead of all of them**, because it is the only +/// failure the player can act on and — since 1667 made the address editable — the only one they can +/// misread. "Unable to connect" cannot distinguish *that name does not exist* from *the address is +/// fine and nothing is listening*, so a player whose server is simply down edits a correct address +/// until they give up. That is what the editor's first live use produced. Both replacement strings +/// are the reference's own (`AUTH_LOGIN_SERVER_NOT_FOUND` / `LOGIN_SERVER_DOWN`, so they localize); +/// the address line under them is ours, because the reference never had an address to get wrong. +fn fail_text( + strings: &GlueStrings, + refusal: Option, + dial: Option<&DialFailure>, +) -> String { + if let Some(dial) = dial { + let headline = if dial.unresolved { + strings.text("AUTH_LOGIN_SERVER_NOT_FOUND", "Invalid Login Server") + } else { + strings.text("LOGIN_SERVER_DOWN", "Login Server Down") + }; + return format!("{headline}\n{}", dial.address); + } + match refusal { + Some(LoginRefusal::World(code)) => world_refusal_text(strings, code).to_string(), + Some(LoginRefusal::Logon(code)) => logon_refusal_text(strings, Some(code)).to_string(), + None => logon_refusal_text(strings, None).to_string(), + } +} + +/// **The world server's** `SMSG_AUTH_RESPONSE` refusal, in the client's own words. +/// +/// A straight transcription of the client's own dispatch over this enum, decompiled in wow-re +/// `system/net/scratch/w2b-pack.c` — each case loads exactly the `GlueStrings` key named below, +/// and the numbering is `AuthResponseCodes` in cmangos `SharedDefines.h:1721+`. Nothing here is a +/// judgement call; where the client picks a string, so do we. +/// +/// Every one of these used to arrive as a generic "Unable to connect", because the world result +/// was formatted into an error string in `WorldSession::connect` and the byte thrown away. +fn world_refusal_text(strings: &GlueStrings, code: u8) -> &str { + use benilla_protocol::messages as m; + let (key, fallback): (&str, &str) = match code { + m::AUTH_FAILED => ("AUTH_FAILED", "Authentication failed"), + m::AUTH_REJECT => ("AUTH_REJECT", "Login unavailable"), + m::AUTH_BAD_SERVER_PROOF => ("AUTH_BAD_SERVER_PROOF", "Server is not valid"), + m::AUTH_UNAVAILABLE => ( + "AUTH_UNAVAILABLE", + "System unavailable - Please try again later", + ), + m::AUTH_SYSTEM_ERROR => ("AUTH_SYSTEM_ERROR", "System Error"), + m::AUTH_BILLING_ERROR => ("AUTH_BILLING_ERROR", "Billing system error"), + m::AUTH_BILLING_EXPIRED => ("AUTH_BILLING_EXPIRED", "Account billing has expired"), + m::AUTH_VERSION_MISMATCH => ("AUTH_VERSION_MISMATCH", "Wrong client version"), + m::AUTH_UNKNOWN_ACCOUNT => ("AUTH_UNKNOWN_ACCOUNT", "Unknown account"), + m::AUTH_INCORRECT_PASSWORD => ("AUTH_INCORRECT_PASSWORD", "Incorrect Password"), + m::AUTH_SESSION_EXPIRED => ("AUTH_SESSION_EXPIRED", "Session Expired"), + m::AUTH_SERVER_SHUTTING_DOWN => ("AUTH_SERVER_SHUTTING_DOWN", "Server Shutting Down"), + m::AUTH_ALREADY_LOGGING_IN => ("AUTH_ALREADY_LOGGING_IN", "Already Logging In"), + m::AUTH_LOGIN_SERVER_NOT_FOUND => ("AUTH_LOGIN_SERVER_NOT_FOUND", "Invalid Login Server"), + // `AUTH_OK` never reaches here (it is the success path) and `AUTH_WAIT_QUEUE` is not a + // refusal at all; anything else is a code this client does not know. + _ => ("AUTH_FAILED", "Authentication failed"), + }; + strings.text(key, fallback) +} + +/// The **realmd** logon-proof refusal, borrowed straight out of the table. +fn logon_refusal_text(strings: &GlueStrings, code: Option) -> &str { let (key, fallback): (&str, &str) = match code { None => ("LOGIN_FAILED", "Unable to connect"), Some(0x03) => ("AUTH_BANNED", "This account has been banned"), @@ -229,11 +294,21 @@ fn drive_policy( mut exit: MessageWriter, ) { let now = time.elapsed_secs(); - // A harness run (env creds, and not the smoke — the smoke owns its own verdict) has nobody at - // the keyboard: a login failure no resubmit can change leaves it parked on a dialog for its - // whole wall-clock, and every retry a runner grants it is spent the same way. Those failures - // exit non-zero instead, on one greppable marker — "login: FATAL" — that leg.sh keys on. - let harness = + // A harness run (env creds, and not the smoke — the smoke owns its own verdict) may have + // nobody at the keyboard: a login failure no resubmit can change would leave it parked on a + // dialog for its whole wall-clock, and every retry a runner grants it is spent the same way. + // Those failures exit non-zero instead, on one greppable marker — "login: FATAL" — that + // leg.sh keys on (decision 1371). + // + // **"May" is the whole correction.** This flag is derived from the environment + // ([`crate::run_mode::unattended_login`] — any of `$WOW_USER`/`$WOW_PASS`/`$WOW_CHAR`), and + // the environment cannot see who is in the room. The director keeps `$WOW_CHAR` set to skip + // character select, which made every one of their sessions "a harness" — so a **typed** + // password with a typo killed the process instead of showing the dialog the reference shows. + // The env fact is the right answer to "should the client log in without waiting for someone + // to type?" and the wrong answer to "is there anybody here?"; it was answering both. Whether + // an *attempt* was typed is not an inference at all — see `announced` at the use sites. + let harness_env = crate::run_mode::unattended_login() && std::env::var_os("WOW_LOGIN_SMOKE").is_none(); let empty = GlueStrings::default(); let strings = strings.as_deref().unwrap_or(&empty); @@ -286,31 +361,40 @@ fn drive_policy( attempt.intent.park = IoPark::AtLogin; // A terminal failure names something no resubmit can change (the server requires Warden, // say) — show the server's own words and drop the credentials so nothing retries. + // **Did a person submit the attempt that just failed?** `announced` is set by the login + // screen's own submit and by nothing else — the env fast path and every paced resubmit + // leave it false — so it is direct evidence rather than an inference about the room, and + // `LoginIntent::clear` below deliberately does not reset it. A typed attempt is attended + // by definition, and an attended failure gets the reference's answer: the dialog, and + // another go. Only an attempt nobody typed may kill the process. + let typed = attempt.intent.announced; + let fatal = harness_env && !typed; if msg.terminal { warn!("login: {}", msg.reason); attempt.intent.clear(); dialog.open_error(&msg.reason); - if harness { + if fatal { error!("login: FATAL — terminal login failure with nobody at the keyboard ({}); exiting", msg.reason); exit.write(AppExit::error()); } continue; } - match msg.code { - Some(code) => { - // A refusal: surface it (even on the silent path — the credentials went stale) - // and never auto-retry against it. - warn!("login: refused (code {code:#04x}) — {}", msg.reason); + match msg.refusal { + Some(refusal) => { + // A refusal — from EITHER server: surface it (even on the silent path, since the + // credentials went stale) and never auto-retry against it. + let byte = refusal.byte(); + warn!("login: refused ({refusal:?}, {byte:#04x}) — {}", msg.reason); attempt.intent.clear(); - dialog.open_error(fail_text(strings, Some(code))); - if harness { - error!("login: FATAL — refused (code {code:#04x}) and no resubmit can change it; exiting"); + dialog.open_error(&fail_text(strings, Some(refusal), None)); + if fatal { + error!("login: FATAL — refused ({refusal:?}, {byte:#04x}) and no resubmit can change it; exiting"); exit.write(AppExit::error()); } } None if attempt.intent.announced => { warn!("login: {}", msg.reason); - dialog.open_error(fail_text(strings, None)); + dialog.open_error(&fail_text(strings, None, msg.dial.as_ref())); } None => { // Silent transport failure with pending intent: schedule the paced resubmit. @@ -1198,15 +1282,132 @@ mod tests { assert!(!dialog.edit.password, "an address is not a secret"); } + /// Did this update ask the app to quit? + fn exited(app: &mut App, cursor: &mut bevy::ecs::message::MessageCursor) -> bool { + let msgs = app.world().resource::>(); + cursor.read(msgs).next().is_some() + } + + /// Drive one refusal through the policy and report whether it killed the process. + fn refusal_exits(typed: bool) -> bool { + let (mut app, _requests) = policy_app(); + let mut cursor = bevy::ecs::message::MessageCursor::::default(); + // Drain whatever the first update writes before the message under test. + app.update(); + let _ = exited(&mut app, &mut cursor); + app.world_mut().resource_mut::().announced = typed; + app.world_mut().write_message(LoginFailedMessage { + refusal: Some(LoginRefusal::Logon(0x05)), + reason: "server rejected logon: result 0x05".into(), + terminal: false, + dial: None, + }); + app.update(); + assert_eq!( + app.world().resource::().kind, + Some(DialogKind::Error), + "every refusal shows the dialog, exit or not", + ); + exited(&mut app, &mut cursor) + } + + /// **A typo must not kill the client.** + /// + /// The reported crash: `login: FATAL — refused (code 0x04) … exiting`, on a password typed at + /// the screen. `$WOW_CHAR` was set — the director keeps it to skip character select — which + /// makes [`crate::run_mode::unattended_login`] true and therefore made *every* session "a + /// harness", so a refusal took `AppExit::error()` instead of the reference's dialog. The + /// environment cannot see who is in the room; `announced` can, because only the screen's own + /// submit sets it. + #[test] + fn a_typed_password_refusal_never_exits() { + let _lock = crate::local_state::test_env::ENV_LOCK.lock(); + // The harness environment the director actually runs with. + let _char = crate::local_state::test_env::EnvGuard::set("WOW_CHAR", "Somebody"); + let _smoke = crate::local_state::test_env::EnvGuard::unset("WOW_LOGIN_SMOKE"); + assert!( + crate::run_mode::unattended_login(), + "the fixture must reproduce the env that made this fatal", + ); + assert!( + !refusal_exits(true), + "a password typed at the screen is attended by definition — dialog, then another go", + ); + assert!( + refusal_exits(false), + "an attempt nobody typed still exits non-zero: decision 1371's leg guarantee", + ); + } + /// The code→string map quotes the client's own strings for the vmangos-verified rows. #[test] fn fail_text_maps_the_verified_codes() { let strings = GlueStrings::default(); // empty table → the fallback literals - assert_eq!(fail_text(&strings, Some(0x04)), "Unknown account"); - assert_eq!(fail_text(&strings, Some(0x05)), "Incorrect Password"); - assert_eq!(fail_text(&strings, None), "Unable to connect"); - assert_eq!(fail_text(&strings, Some(0x09)), "Wrong client version"); - assert_eq!(fail_text(&strings, Some(0xEE)), "Authentication failed"); + let logon = |b| fail_text(&strings, Some(LoginRefusal::Logon(b)), None); + assert_eq!(logon(0x04), "Unknown account"); + assert_eq!(logon(0x05), "Incorrect Password"); + assert_eq!(fail_text(&strings, None, None), "Unable to connect"); + assert_eq!(logon(0x09), "Wrong client version"); + assert_eq!(logon(0xEE), "Authentication failed"); + } + + /// **The two result enums are different enums**, and the same byte must not mean the same + /// thing in both. 0x0C is `AUTH_LOGON_FAILED_SUSPENDED` to realmd and `AUTH_OK` to the world + /// server; 0x05 is a bad password to realmd and nothing at all to the world server. A single + /// `Option` could not tell them apart, which is why `LoginRefusal` exists. + #[test] + fn the_two_auth_enums_do_not_share_a_byte() { + use benilla_protocol::messages as m; + let strings = GlueStrings::default(); + let logon = |b| fail_text(&strings, Some(LoginRefusal::Logon(b)), None); + let world = |b| fail_text(&strings, Some(LoginRefusal::World(b)), None); + + assert_eq!(logon(0x0C), "This account has been temporarily suspended"); + assert_eq!(world(m::AUTH_BILLING_ERROR), "Billing system error"); + assert_ne!(logon(0x0C), world(0x0C)); + + // Every world row is the client's own dispatch (`w2b-pack.c`), transcribed. + assert_eq!(world(m::AUTH_INCORRECT_PASSWORD), "Incorrect Password"); + assert_eq!(world(m::AUTH_SESSION_EXPIRED), "Session Expired"); + assert_eq!(world(m::AUTH_SERVER_SHUTTING_DOWN), "Server Shutting Down"); + assert_eq!(world(m::AUTH_ALREADY_LOGGING_IN), "Already Logging In"); + assert_eq!( + world(m::AUTH_UNAVAILABLE), + "System unavailable - Please try again later" + ); + // A code this client does not know still lands on the authored catch-all. + assert_eq!(world(0xEE), "Authentication failed"); + } + + /// **The report that prompted this** — a dead local server read as a bad address, twice, because + /// "Unable to connect" cannot tell the two apart. Each dial verdict now gets the reference's own + /// string for that exact condition, with the address under it so there is nothing left to guess. + #[test] + fn a_dial_failure_says_which_failure_it_was() { + let strings = GlueStrings::default(); + let down = DialFailure { + address: "127.0.0.1:3724".into(), + unresolved: false, + }; + assert_eq!( + fail_text(&strings, None, Some(&down)), + "Login Server Down\n127.0.0.1:3724", + "the address resolved and nothing answered — editing the address will not help", + ); + let missing = DialFailure { + address: "logon.nonesuch.example:3724".into(), + unresolved: true, + }; + assert_eq!( + fail_text(&strings, None, Some(&missing)), + "Invalid Login Server\nlogon.nonesuch.example:3724", + "the name resolved to nothing — this one IS the address", + ); + // A refusal still wins its own authored string; the dial verdict only speaks for the dial. + assert_eq!( + fail_text(&strings, Some(LoginRefusal::Logon(0x05)), None), + "Incorrect Password", + ); } /// Save → load → clear round-trips through the dot-file (the ref's Get/SetSavedAccountName). diff --git a/crates/benilla-app/src/login/smoke.rs b/crates/benilla-app/src/login/smoke.rs index 131af907..f582c03c 100644 --- a/crates/benilla-app/src/login/smoke.rs +++ b/crates/benilla-app/src/login/smoke.rs @@ -59,7 +59,10 @@ pub(super) fn debug_login_smoke( } 1 => { if let Some(f) = failures.read().last() { - error!("login-smoke: FAILED code={:?} reason={}", f.code, f.reason); + error!( + "login-smoke: FAILED refusal={:?} reason={}", + f.refusal, f.reason + ); // `WOW_LOGIN_SMOKE_HOLD=1`: keep running on a refusal instead of exiting — the // error dialog stays up, so a shot instrument can photograph it (the dialog is // otherwise unreachable headlessly; pair with `WOW_PROBE_EXIT_AT`). diff --git a/crates/benilla-app/src/net.rs b/crates/benilla-app/src/net.rs index 67c9b35e..b3ca57f5 100644 --- a/crates/benilla-app/src/net.rs +++ b/crates/benilla-app/src/net.rs @@ -1736,9 +1736,16 @@ impl DisconnectedMessage { /// client — e.g. it requires Warden): show `reason` and stop, never resubmit. #[derive(Message, Clone)] pub(crate) struct LoginFailedMessage { - pub(crate) code: Option, + /// Which server refused us and with which byte — see [`benilla_protocol::LoginRefusal`] for + /// why this is not a bare `Option`. + pub(crate) refusal: Option, pub(crate) reason: String, pub(crate) terminal: bool, + /// The dial that never opened a socket, when that is what failed — see + /// [`benilla_protocol::DialFailure`]. What lets the screen say *which* of "that name does not + /// exist" and "nothing is answering there" happened, instead of one "Unable to connect" that + /// sends a player off editing a correct address. + pub(crate) dial: Option, } /// A same-map teleport for our player: snap to the pose, then echo the ack. Written by diff --git a/crates/benilla-app/src/net/apply.rs b/crates/benilla-app/src/net/apply.rs index b99ec849..bded3f82 100644 --- a/crates/benilla-app/src/net/apply.rs +++ b/crates/benilla-app/src/net/apply.rs @@ -456,10 +456,11 @@ pub(super) fn apply_net_updates( match ev { SessionEvent::LoginStage { stage } => session::login_stage(stage, &mut login_stages), SessionEvent::LoginFailed { - code, + refusal, reason, terminal, - } => session::login_failed(code, reason, terminal, &mut login_failures), + dial, + } => session::login_failed(refusal, reason, terminal, dial, &mut login_failures), SessionEvent::CharacterList { characters, realm } => { session::character_list(characters, realm, &mut status, &mut char_lists) } diff --git a/crates/benilla-app/src/net/apply/session.rs b/crates/benilla-app/src/net/apply/session.rs index 236b83e0..0e2616f5 100644 --- a/crates/benilla-app/src/net/apply/session.rs +++ b/crates/benilla-app/src/net/apply/session.rs @@ -37,15 +37,17 @@ pub(super) fn login_stage( /// A login attempt failed before the roster (decision 0539): the IO thread is back at its pre-logon /// park, and [`crate::login`]'s policy decides what happens next. pub(super) fn login_failed( - code: Option, + refusal: Option, reason: String, terminal: bool, + dial: Option, out: &mut MessageWriter, ) { out.write(LoginFailedMessage { - code, + refusal, reason, terminal, + dial, }); } diff --git a/crates/benilla-app/src/net/io.rs b/crates/benilla-app/src/net/io.rs index 4b9919ed..4b1f7dd6 100644 --- a/crates/benilla-app/src/net/io.rs +++ b/crates/benilla-app/src/net/io.rs @@ -264,20 +264,34 @@ fn run( let stage = |s: LoginStage| { let _ = events_tx.send(SessionEvent::LoginStage { stage: s }); }; - let fail = |code: Option, reason: String| { + let fail = |refusal: Option, reason: String| { let _ = events_tx.send(SessionEvent::LoginFailed { - code, + refusal, reason, terminal: false, + dial: None, + }); + Ok(Cycle::Repark) + }; + // The dial that never opened a socket — the only failure whose cause the *screen* can act on, + // now that the address is something a player types (1667). Kept separate from `fail` so the + // classification happens once, at the one place holding the error object. + let fail_dial = |dial: benilla_protocol::DialFailure, reason: String| { + let _ = events_tx.send(SessionEvent::LoginFailed { + refusal: None, + reason, + terminal: false, + dial: Some(dial), }); Ok(Cycle::Repark) }; // A failure resubmitting cannot fix — the app shows it and stops (no paced retry). let fail_terminal = |reason: String| { let _ = events_tx.send(SessionEvent::LoginFailed { - code: None, + refusal: None, reason, terminal: true, + dial: None, }); Ok(Cycle::Repark) }; @@ -291,9 +305,15 @@ fn run( return Ok(Cycle::Repark); } // A server refusal carries its auth result byte (the app maps it to the client's - // own AUTH_* string); a transport failure carries None. - let code = e.downcast_ref::().map(|r| r.code); - return fail(code, format!("{e:#}")); + // own AUTH_* string); a transport failure carries None. A failure to get a socket at + // all carries the dial verdict, which is the one the screen can turn into advice. + if let Some(dial) = e.downcast_ref::() { + return fail_dial(dial.clone(), format!("{e:#}")); + } + let refusal = e + .downcast_ref::() + .map(|r| benilla_protocol::LoginRefusal::Logon(r.code)); + return fail(refusal, format!("{e:#}")); } }; if canceled() { @@ -320,6 +340,14 @@ fn run( if let Some(w) = e.downcast_ref::() { return fail_terminal(w.to_string()); } + // The world server's own refusal, in its own enum — the screen owes the player the + // authored `AUTH_*` string for it, which it cannot recover from a formatted message. + if let Some(r) = e.downcast_ref::() { + return fail( + Some(benilla_protocol::LoginRefusal::World(r.code)), + format!("{e:#}"), + ); + } return fail(None, format!("world handshake with {world_addr}: {e:#}")); } }; diff --git a/crates/benilla-app/src/target/cursor_mode.rs b/crates/benilla-app/src/target/cursor_mode.rs index dfcfea1a..2eca1bb3 100644 --- a/crates/benilla-app/src/target/cursor_mode.rs +++ b/crates/benilla-app/src/target/cursor_mode.rs @@ -21,21 +21,23 @@ //! Loot *rights* never gray — they gate whether the loot cursor shows at all; the mid-loot state //! block and the open-loot-window able-override are not modeled. //! -//! *Interim*: the reference's interactability predicate (`CGUnit::CanInteract 0x606880`) isn't -//! fully derived; we approximate it as "has service flags and isn't attack-worthy (reaction ≥ -//! neutral)". Attackability approximates the PvP/attack matrix as "reaction rank ≤ neutral" — -//! the same approximation the ring's player branch documents. (The questgiver bit's own -//! quest-status gate, `0x5df490`, and the auto-loot Pickup/LootAll split, `0x41f8f0`, both used -//! to be listed here as unmodelled; both are modelled now — [`questgiver_has_quest`], +//! **Both branch predicates are the reference's own, not thresholds** (decision 1674): the +//! service/loot split is `CanInteract 0x6067f0` (`0x482310`, via the `CanInteractNow 0x606880` +//! wrapper) and the sword is `CanAttack 0x606980` (`0x48269a`) — [`super::can_interact`] / +//! [`super::can_attack`], both byte-verified complete. That matters because each reads +//! `UnitReaction` in the **player → unit** direction, which answers a reputation-slot faction with +//! the **at-war bit** and never the standing. The reaction-rank approximation these legs used to +//! carry read the *other* direction, and drew the sword over every not-at-war neutral faction — +//! a Cenarion Circle druid, a Booty Bay goblin, an Argent Dawn quartermaster. (The questgiver +//! bit's own quest-status gate, `0x5df490`, and the auto-loot Pickup/LootAll split, `0x41f8f0`, +//! both used to be listed here as unmodelled; both are modelled now — [`questgiver_has_quest`], //! [`loot_cursor`].) use bevy::prelude::*; -use benilla_protocol::EntityKind; +use crate::net::{ObjectStore, Reputations, SelfPlayer}; -use crate::net::{NetEntity, ObjectStore, Reputations, SelfPlayer}; - -use super::ring::{ring_reaction, Factions}; +use super::ring::Factions; use super::{go_is_nearest, Hovered, HoveredObject}; /// The blp-name set of world cursor modes benilla can currently trigger (named off the client's own @@ -706,7 +708,6 @@ pub(super) fn classify_cursor( units: Query<( &Transform, Option<&ObjectStore>, - Option<&NetEntity>, Option<&crate::go_anim::GoAnim>, )>, self_q: Query<(&Transform, &ObjectStore), With>, @@ -735,7 +736,7 @@ pub(super) fn classify_cursor( // grayed twin is still the interim distance gate (decision 0243); the client's fuller `usable` // (lock satisfaction, player-state) is later. let resolve_go = || { - let (go_tf, store, _, anim) = units.get(hovered_object.target?).ok()?; + let (go_tf, store, anim) = units.get(hovered_object.target?).ok()?; let store = store?; let (self_tf, self_store) = self_q.single().ok()?; let reaction = go_reaction( @@ -794,7 +795,7 @@ pub(super) fn classify_cursor( // The reference makes one pick over all CGObjects and switches on type; benilla picks unit and // GameObject separately, then classifies whichever is nearer under the cursor. let resolve_unit = || { - let (unit_tf, store, net, _) = units.get(hovered.target?).ok()?; + let (unit_tf, store, _) = units.get(hovered.target?).ok()?; let store = store?; let (self_tf, self_store) = self_q.single().ok()?; let dist_sq = unit_tf.translation.distance_squared(self_tf.translation); @@ -825,28 +826,35 @@ pub(super) fn classify_cursor( return None; // a plain corpse: Point } - // Reaction rank (0..=7) toward us — the ring's own resolver (reputation first, then the - // faction-template comparator). - let rank = ring_reaction( + // `CanInteract 0x6067f0` — the classifier's own gate (`0x482310`, through the + // `CanInteractNow 0x606880` wrapper), and it is not a reaction threshold: service bits + // plus BOTH reaction directions >= neutral, the player->unit one answered by the AT-WAR + // bit. TRUE goes to the ladder, FALSE to the loot/skin/attack block. + if super::can_interact( + Some(store), factions.as_deref(), &reputations, - Some(store), Some(self_store), - ); - let is_player = net.is_some_and(|n| n.kind == EntityKind::Player); - // Interactable NPC (interim CanInteract): a consulted service bit + not attack-worthy. - // A repair-only unit yields None here and falls through, exactly like the binary. - if !is_player && rank >= 3 { + ) { let status = hovered.guid.and_then(|g| quest.status(g)); - if let Some(kind) = service_cursor(store.0.unit_npc_flags(), status) { - return Some((kind, dist_sq > SERVICE_RANGE_SQ)); - } + // A unit that reaches the ladder and matches no consulted bit — repair-only, or a + // QUESTGIVER with nothing on offer — falls `je 0x4826cb` straight to the cursor + // CLEAR. It never reaches the attack leg, so it reads Point, not the sword. + return service_cursor(store.0.unit_npc_flags(), status) + .map(|kind| (kind, dist_sq > SERVICE_RANGE_SQ)); } - // Attackable (interim matrix): hostile/unfriendly/neutral NPC, or a hostile player. - if (!is_player && rank <= 3) || (is_player && rank <= 1) { + // `CanAttack 0x606980` (`0x48269a`) + the fixed 10.45 yd gray. The predicate is shared + // with TAB, the combat flash and `UnitCanAttack`, so the sword can never disagree with + // what a click or a spell will actually be allowed to do. + if super::can_attack( + Some(store), + factions.as_deref(), + &reputations, + Some(self_store), + ) { return Some((CursorKind::Attack, dist_sq > ATTACK_RANGE_SQ)); } - None // friendly non-service unit / friendly player: Point + None // nothing matched → the reference's `0x4826cb` cursor-clear: Point }; let resolved = if go_is_nearest(&hovered, &hovered_object) { resolve_go() @@ -1398,6 +1406,37 @@ mod tests { assert_eq!(service_cursor(0, None), None); } + /// **A unit that reaches the ladder and matches nothing falls to the cursor CLEAR, never to + /// the sword** (`je 0x4826cb`, past the attack leg's `0x48269a`). This is the second half of + /// 1674: the ladder's fall-out used to drop into benilla's attack leg, so a repair-only NPC — + /// or a QUESTGIVER with nothing on offer — read as attackable purely because its reaction was + /// neutral. `service_cursor` returning `None` inside the interactable branch must mean Point. + #[test] + fn the_ladder_falls_out_to_point_not_to_the_sword() { + use npc_flags::*; + // Both shapes that reach the ladder and match no consulted bit. + assert_eq!(service_cursor(REPAIR_ONLY, None), None); + assert_eq!(service_cursor(QUESTGIVER, Some(dialog_status::NONE)), None); + // The classifier's own expression for that branch: `Option::map` over the ladder, so a + // `None` returns `None` from `resolve_unit` and the caller's `unwrap_or` lands on Point. + // (Written as the identity it is, so a future edit that adds an `else` fall-through to the + // attack leg has to delete this test to compile a different shape.) + let unable = 20.0 > SERVICE_RANGE_SQ; + assert_eq!( + service_cursor(REPAIR_ONLY, None).map(|kind| (kind, unable)), + None, + "repair-only reads Point, not UnableAttack" + ); + assert_eq!( + service_cursor(GOSSIP, None).map(|kind| (kind, unable)), + Some((CursorKind::Speak, false)), + "…while a bit that IS consulted still reaches its cursor" + ); + } + + /// `UNIT_NPC_FLAGS` REPAIR — the one service bit the ladder never tests. + const REPAIR_ONLY: u32 = 0x4000; + /// The QUESTGIVER leg's `0x5df490` gate: the bit alone never makes a unit talkable. This is the /// "client invents 'Greetings NAME'" bug at its root — a questgiver-flagged NPC with nothing to /// offer must fall out of the ladder entirely, so we never send `CMSG_GOSSIP_HELLO` and never diff --git a/crates/benilla-app/src/target/mod.rs b/crates/benilla-app/src/target/mod.rs index f67d6a9d..8cfe3051 100644 --- a/crates/benilla-app/src/target/mod.rs +++ b/crates/benilla-app/src/target/mod.rs @@ -76,7 +76,7 @@ pub(crate) use flash::CombatFlash; // (`scan`) answers it with the same core TAB uses. `attack_order_target` + `EnemyScan` are that // same core called *synchronously*, by the pet bar's ATTACK arm: the pet's order has to leave in // the frame it was pressed carrying the acquired guid, so it cannot go round through a request. -pub(crate) use relations::{can_assist, can_attack}; +pub(crate) use relations::{can_assist, can_attack, can_interact}; pub(crate) use scan::{attack_order_target, AttackNearestRequest, EnemyScan}; // The chat layer's by-name selection asks (`/target`, `/assist` — decision 0886), answered by the // shared resolver the reference parameterises per caller. diff --git a/crates/benilla-app/src/target/relations.rs b/crates/benilla-app/src/target/relations.rs index 98c3e3b9..7e82a1b0 100644 --- a/crates/benilla-app/src/target/relations.rs +++ b/crates/benilla-app/src/target/relations.rs @@ -1,50 +1,89 @@ -//! **Who may I attack, who may I help** — the two unit-relationship predicates the reference -//! shares across systems, kept together because they are one concern and neither belongs to the -//! system that happens to call them first. +//! **Who may I attack, who may I interact with, who may I help** — the three unit-relationship +//! predicates the reference shares across systems, kept together because they are one concern and +//! none belongs to the system that happens to call it first. //! -//! Both are byte-derived and both key off [`ring_reaction`], on opposite sides of the same bar: -//! `CanAttack 0x606980` wants reaction ≤ 3 (not friendly), `CanAssist 0x6066f0` wants ≥ 4 -//! (friendly) plus two flag clauses. They lived in `scan.rs` while `can_attack` had exactly one -//! caller; `can_assist` (the `UnitBuff` gate, decision 1035) made that a second concern in a file -//! about TAB targeting. +//! The first two are byte-verified **complete** functions and live next door in [`super::ring`], +//! beside the two reaction directions they turn on — because *which direction* is the whole +//! content of both (`0x6061e0(this = player)` answers a reputation-slot faction with the **at-war +//! bit**; `0x6061e0(this = unit)` reads the standing, and the two routinely disagree). This file +//! is the shared store-only entry point every consumer calls: +//! +//! - `CanAttack 0x606980` → [`super::ring::can_attack_from_player`], +//! - `CanInteract 0x6067f0` → [`super::ring::can_interact_from_player`], +//! - `CanAssist 0x6066f0` → still the reaction-rank derivation below, on [`ring_reaction`]. +//! +//! They lived in `scan.rs` while `can_attack` had exactly one caller; `can_assist` (the `UnitBuff` +//! gate, decision 1035) made that a second concern in a file about TAB targeting. -use benilla_protocol::messages::OwnerFallback; +use benilla_protocol::messages::{ObjectType, OwnerFallback}; use crate::net::{ObjectStore, Reputations}; use super::{ring_reaction, Factions}; -/// The five `UNIT_FIELD_FLAGS` disqualifiers `CanAttack 0x606980` tests (bit positions -/// byte-verified; names vmangos-corroborated): NON_ATTACKABLE(1), NOT_ATTACKABLE_1(7), -/// NON_ATTACKABLE_2(16), TAXI_FLIGHT(20), NOT_SELECTABLE(25). -pub(super) const FLAG_DISQUALIFIERS: u32 = (1 << 1) | (1 << 7) | (1 << 16) | (1 << 20) | (1 << 25); - -/// `CanAttack 0x606980` — the shared attackability predicate (the combat flash's gate and the -/// scan's filter 3): the flag disqualifiers clear AND an attackable reaction. A unit without a -/// store passes the flag leg (nothing known to disqualify). The duel/PVP legs are deferred with -/// duels/PvP. +/// `OBJECT_FIELD_TYPE` bit 4 — the reference's own "is this a Player" test, read as +/// `[obj->descriptorBlock[2] + 8] >> 4 & 1` (`0x606984` in `CanAttack`, `0x6067fc` in +/// `CanInteract`). Taken off the object's own field rather than the spawning `NetEntity`, because +/// that is the bit the binary reads and it cannot drift from the store the rest of the predicate +/// walks. /// -/// The reaction leg is **≤ neutral (rank ≤ 3)** — attackable means *not friendly*. First -/// director-observed (0170), then **byte-confirmed** by the `0x606980` re-pin: the function has -/// three reaction legs selected by `UNIT_FLAG_PVP_ATTACKABLE` (bit 3) on both parties, and the -/// player-vs-creature case is Leg B — `UnitReaction(player→target) < 4` (`cmp eax,4; setl`), -/// single direction, friendly-only blocked. The old "≤ 1 hostile" gloss was Leg A (both parties -/// UN-flagged: NPC-vs-NPC, bidirectional), never the player's check. Leg C (both flagged: the -/// PvP/duel/group machinery) is deferred with PvP. +/// **The one thing not verified against a live stream** is whether the server always puts field 2 +/// in the create block (vmangos sends every non-zero field, and a player's is `0x19`, so it should +/// always be there — but that is a reading of the emulator, not an observation). The failure mode +/// is bounded and inert: this feeds only `CanAttack`'s ghost leg, so a false negative on a real +/// player skips a refusal that the both-player-controlled arm then makes anyway — a ghost carries +/// no duel, PvP flag or FFA pair. `/reaction` prints this beside the `NetEntity` kind so the first +/// hover over a player settles it by observation. +fn is_player_object(store: Option<&ObjectStore>) -> bool { + store.and_then(|s| s.0.object_type()) == Some(ObjectType::Player) +} + +/// `CanAttack 0x606980` — the shared attackability predicate: the world cursor's sword leg +/// (`0x48269a`), the combat flash's gate, the TAB scan's filter 3, `UnitCanAttack`, and hostile +/// spell targeting all ask this one question. +/// +/// **This forwards to the byte-verified complete function** ([`super::ring::can_attack_from_player`], +/// wow-re `object-layer/scratch/nameplate-category-gate.md` §3, §5 cross-checked 2026-08-22) — a +/// ghost gate, five `UNIT_FIELD_FLAGS` refusal bits on the target, four cross-flag immunity legs, +/// then three terminal arms selected by `UNIT_FLAG_PVP_ATTACKABLE` on both parties. +/// +/// It used to be `flag disqualifiers && ring_reaction ≤ 3`, and **the threshold was reading the +/// wrong reaction direction** — the very substitution decision 1530 named as the shipped defect +/// and corrected for nameplates alone. `0x606980`'s mixed arm is `UnitReaction(**player** → target) +/// < 4`, and that direction answers a reputation-slot faction with the **at-war bit**, never the +/// standing. Keeping the standing here made every not-at-war neutral faction attackable: hovering +/// a Cenarion Circle NPC drew the sword, TAB targeted it, and `UnitCanAttack` agreed (decision 1674). pub(crate) fn can_attack( store: Option<&ObjectStore>, factions: Option<&Factions>, reputations: &Reputations, self_store: Option<&ObjectStore>, ) -> bool { - if store.is_some_and(|s| s.0.unit_flags() & FLAG_DISQUALIFIERS != 0) { - return false; - } - ring_reaction(factions, reputations, store, self_store) <= 3 + super::ring::can_attack_from_player( + factions, + reputations, + store, + self_store, + is_player_object(store), + ) +} + +/// `CanInteract 0x6067f0` — "may I take a service from this unit?", the gate the world-cursor +/// classifier runs at `0x482310` (through the `CanInteractNow 0x606880` wrapper) to choose between +/// the NPC service ladder and the loot/skin/attack block. +/// +/// Forwards to [`super::ring::can_interact_from_player`], which is the complete `0x6067f0`. +pub(crate) fn can_interact( + store: Option<&ObjectStore>, + factions: Option<&Factions>, + reputations: &Reputations, + self_store: Option<&ObjectStore>, +) -> bool { + super::ring::can_interact_from_player(factions, reputations, store, self_store) } /// `UNIT_FLAG_NOT_SELECTABLE` (bit 25) — `CanAssist`'s own first disqualifier, and one of -/// [`FLAG_DISQUALIFIERS`]' five (vmangos `UnitDefines.h`). +/// `CanAttack`'s five refusal bits (vmangos `UnitDefines.h`). const UNIT_FLAG_NOT_SELECTABLE: u32 = 1 << 25; /// `UNIT_FLAG_PVP` (bit 12) — what `IsPvP 0x605ff0` tests, after resolving the unit's owner /// (vmangos `UnitDefines.h:UNIT_FLAG_PVP = 0x1000`). diff --git a/crates/benilla-app/src/target/ring.rs b/crates/benilla-app/src/target/ring.rs index a58e3872..3edc5bee 100644 --- a/crates/benilla-app/src/target/ring.rs +++ b/crates/benilla-app/src/target/ring.rs @@ -691,14 +691,7 @@ pub(crate) fn reaction_from_player( let catalog = &factions?.0; let target_tpl = catalog.template(target_store?.0.unit_faction_template()?)?; // Leg 3: a faction that owns a reputation slot is answered by the AT-WAR bit alone. - // `faction_flags::AT_WAR` is the same wire byte the reputation pane's war checkbox reads, - // which is what makes declaring war on Booty Bay turn its goblins' plates red *and* move - // them into the enemy category — one bit, both consequences, as the reference has it. - if let Some(info) = catalog.reputation_faction(target_tpl.faction) { - let at_war = usize::try_from(info.rep_index) - .ok() - .and_then(|i| reputations.0.get(i)) - .is_some_and(|&(flags, _)| flags & benilla_formats::faction_flags::AT_WAR != 0); + if let Some(at_war) = at_war_with(catalog, reputations, target_tpl.faction) { return Some(if at_war { Reaction::Hostile as u8 } else { @@ -712,6 +705,76 @@ pub(crate) fn reaction_from_player( resolved.unwrap_or(Reaction::Neutral as u8) } +/// **Leg 3's whole content**: is the local player at war with the `Faction.dbc` id a unit's +/// faction template names? `None` when that faction owns no reputation slot — the one case leg 3 +/// does not answer, and the reaction falls through to the template comparator. +/// +/// `faction_flags::AT_WAR` is the same wire byte the reputation pane's war checkbox writes, which +/// is what makes declaring war on Booty Bay turn its goblins' plates red, move them into the enemy +/// category, and put the sword on the cursor — one bit, three consequences, as the reference has +/// it. Shared with `/reaction`, which prints it: the bit is the cause of every "why is this +/// friendly NPC attackable" question and it is visible nowhere else in the client. +pub(crate) fn at_war_with( + catalog: &benilla_formats::FactionCatalog, + reputations: &Reputations, + faction_id: u32, +) -> Option { + let info = catalog.reputation_faction(faction_id)?; + Some( + usize::try_from(info.rep_index) + .ok() + .and_then(|i| reputations.0.get(i)) + .is_some_and(|&(flags, _)| flags & benilla_formats::faction_flags::AT_WAR != 0), + ) +} + +/// `CGUnit::CanInteract(this = the local player → arg = unit)` — `0x6067f0`, byte-verified +/// complete (disassembly read at `0x6067f0`–`0x606871`; wow-re `ui/scratch/cursor-system.md` §3 + +/// `cursor-decomp`, and the register set-up confirmed at every call: `0x4822f2 push esi(unit); mov +/// ecx,edi(player); call 0x606880`, then `0x60691d push esi(unit); mov ecx,edi(player); call +/// 0x6067f0`). +/// +/// **This is the world cursor's service gate — not a reaction threshold** (`0x482310`, `test al,al; +/// je 0x4824e6`): TRUE sends the hover into the `UNIT_NPC_FLAGS` service ladder, FALSE into the +/// loot/skin/attack block. Three terms, in the binary's order: +/// +/// 1. the unit's `UNIT_FIELD_FLAGS` **bit 25** clear (`0x606835 shr ecx,0x19`), +/// 2. its `UNIT_NPC_FLAGS` **non-zero** (`0x60683d`) — a unit carrying no service bit at all is +/// never interactable, whatever its reaction, +/// 3. **both** reaction directions `>= 3` (neutral), and they are genuinely different functions: +/// `0x606847 push esi; mov ecx,edi; call 0x6061e0` is [`ring_reaction`] (unit → player, the +/// standing), `0x606854 push edi; mov ecx,esi; call 0x6061e0` is [`reaction_from_player`] +/// (player → unit, the **at-war bit**). Each is `cmp eax,3; jl → 0`. +/// +/// Term 3's second half is what makes declaring war on a faction drop its vendors out of the +/// service ladder and into the sword — one bit, both consequences, exactly as it moves the plate +/// category (1530). +/// +/// **Deferred, and each can only ever make a unit *less* interactable than we say** — so none of +/// them can manufacture a service cursor we would not already show: the `CanInteractNow 0x606880` +/// wrapper's own gates (player `UNIT_FIELD_FLAGS` bit 20 clear @ `0x606893`, both sides' +/// `UNIT_FIELD_CHARMEDBY` zero, player alive, the shapeshift-form clause `0x60e9f0` + +/// `SpellShapeshiftForm` flag `0x8`, and the two unnamed unit predicates `0x613230` / `0x60ecd0`), +/// and `0x6067f0`'s own ghost leg `0x605f70(unit)` — which term 2 already subsumes, since a ghost +/// is a player and a player has no `UNIT_NPC_FLAGS`. +pub(crate) fn can_interact_from_player( + factions: Option<&Factions>, + reputations: &Reputations, + target_store: Option<&ObjectStore>, + self_store: Option<&ObjectStore>, +) -> bool { + let Some(target) = target_store else { + return false; // fields not streamed yet — nothing to take a service from + }; + const UNIT_FLAG_NOT_SELECTABLE: u32 = 1 << 25; + if target.0.unit_flags() & UNIT_FLAG_NOT_SELECTABLE != 0 || target.0.unit_npc_flags() == 0 { + return false; + } + const NEUTRAL: u8 = Reaction::Neutral as u8; + ring_reaction(factions, reputations, target_store, self_store) >= NEUTRAL + && reaction_from_player(factions, reputations, target_store, self_store) >= NEUTRAL +} + /// `CGUnit::CanAttack(this = the local player → arg = unit)` — `0x606980`, byte-verified complete /// (wow-re `nameplate-category-gate.md` §3, §5 cross-checked 2026-08-22). Every leg, in the /// binary's order. @@ -1053,6 +1116,102 @@ mod tests { assert_eq!(rank(&unit(12), &quiet), 4); } + /// **The reported bug, on the real build-5875 DBC, naming the faction** (1674): hovering a + /// **Cenarion Circle** NPC with no gossip drew the ATTACK sword at neutral standing, with the + /// faction not set to war. + /// + /// Cenarion Circle is faction 609, reputation slot 36 — one of the 36 templates wow-re's §7 + /// measured as decided entirely by the at-war bit, and one 1530 named while correcting the + /// plate category alone. The cursor and `relations::can_attack` kept the reaction *threshold*, + /// which reads the OTHER direction (the standing, 0 → neutral → "≤ 3 is attackable"), so the + /// sword came back for every one of those factions. + /// + /// The pin asserts the divergence the threshold cannot express: **the rank stays 3 — the ring + /// and the bar are untouched — while both branch predicates say Point.** A "simplification" + /// back to a threshold has to break this, on real data, naming the creature. + #[test] + fn a_not_at_war_cenarion_circle_npc_takes_no_cursor_at_neutral() { + use crate::net::{ObjectStore, Reputations}; + use benilla_protocol::ObjectFields; + + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let factions = Factions(benilla_formats::load_faction_catalog(&mut chain).expect("dbc")); + // 35 = UNIT_FIELD_FACTIONTEMPLATE, 46 = UNIT_FIELD_FLAGS, 147 = UNIT_NPC_FLAGS. + let npc = |tpl: u32, npc_flags: u32| { + ObjectStore(ObjectFields::from_pairs(&[(35, tpl), (147, npc_flags)])) + }; + let me = ObjectStore(ObjectFields::from_pairs(&[ + (35, 1), // Human + (46, UNIT_FLAG_PVP_ATTACKABLE), + ])); + let quiet = Reputations::default(); // nothing at war — the out-of-box state + let mut slots = vec![(0u8, 0i32); 64]; + slots[36] = (benilla_formats::faction_flags::AT_WAR, 0); // Cenarion Circle = slot 36 + let at_war = Reputations(slots); + + let attackable = |u: &ObjectStore, r: &Reputations| { + can_attack_from_player(Some(&factions), r, Some(u), Some(&me), false) + }; + let interactable = |u: &ObjectStore, r: &Reputations| { + crate::target::ring::can_interact_from_player(Some(&factions), r, Some(u), Some(&me)) + }; + + // FactionTemplate 996 → faction 609 "Cenarion Circle" (also 635/994/1254/1608). The + // reported subject: no service bit at all, so the ladder was never in play — the unit fell + // straight to the attack leg. + let silent = npc(996, 0); + assert_eq!( + ring_reaction(Some(&factions), &quiet, Some(&silent), Some(&me)), + 3, + "neutral standing — the ring and the bar are RIGHT and must not move" + ); + assert!( + !attackable(&silent, &quiet), + "not at war → CanAttack says no: the sword was the bug" + ); + assert!( + !interactable(&silent, &quiet), + "and with no service bit it is not interactable either → the cursor CLEARS to Point" + ); + + // A Cenarion Circle NPC that DOES gossip keeps its speech bubble — the control that must + // not move, and the half of the report that was already right. + let gossip = npc(996, crate::target::cursor_mode::npc_flags::GOSSIP); + assert!(interactable(&gossip, &quiet), "gossip NPC still talks"); + assert!(!attackable(&gossip, &quiet)); + + // Declaring war moves BOTH, off one bit: leg 3 answers hostile, so the gossiper drops out + // of the service ladder and the silent one takes the sword. This is the behaviour the + // director named as the discriminator — "even if you don't have the faction set to at war". + assert!( + attackable(&silent, &at_war), + "at war → the sword is correct" + ); + assert!(attackable(&gossip, &at_war)); + assert!( + !interactable(&gossip, &at_war), + "at war → no more gossip cursor; the reference leaves the ladder entirely" + ); + // …and only that faction's own slot moves. + assert_eq!( + ring_reaction(Some(&factions), &at_war, Some(&silent), Some(&me)), + 3, + "the STANDING is still neutral — at-war does not touch the bar" + ); + + // **The control that must not change**: an ordinary neutral creature whose faction owns no + // reputation slot (Chicken, FT 31 → faction 28 "Prey") is answered by the template + // comparator, reads neutral, and is still attackable. Neutral mobs keep the sword — the fix + // is about the at-war bit, not about neutrality. + let chicken = npc(31, 0); + assert!( + attackable(&chicken, &quiet), + "a neutral critter still takes the sword — the comparator, not at-war" + ); + assert!(!interactable(&chicken, &quiet)); + } + /// **The PLAYER subject** — the arm 1530 landed and left unpinned, on the real DBC. /// /// A player is friendly-category only if `CanCooperate` ALSO says yes, and that predicate is diff --git a/crates/benilla-app/src/target/scan.rs b/crates/benilla-app/src/target/scan.rs index 2a8f0f28..0dac3d1e 100644 --- a/crates/benilla-app/src/target/scan.rs +++ b/crates/benilla-app/src/target/scan.rs @@ -56,7 +56,8 @@ use crate::net::{ClientCommand, Guid, NetEntity, ObjectStore, Reputations, SelfP use benilla_assets::{LockRecover, WorldAssets}; use super::relations::can_attack; -use super::{ring_reaction, Factions, Selection}; +use super::ring::reaction_from_player; +use super::{Factions, Selection}; // == The Classic-priority dials == // Each names the Classic Era cvar it stands in for. The VALUES are tuned, not pinned — the @@ -263,9 +264,18 @@ impl EnemyScan<'_, '_> { /// `0x6130a3`'s keep-or-drop test on a guid we are already holding: is the actor hostile to it? /// /// The reference reads `0x6061e0(actor, target) >= 4` — the **reaction alone**, not the full - /// `0x606980` CanAttack, which it saves for the final gate at `0x613167`. So this is - /// [`ring_reaction`] and nothing else. A guid whose unit is not streamed answers `false`, - /// which is the same exit `0x613099` takes when its object lookup misses. + /// `0x606980` CanAttack, which it saves for the final gate at `0x613167`. A guid whose unit is + /// not streamed answers `false`, which is the same exit `0x613099` takes when its object lookup + /// misses. + /// + /// **The direction is ACTOR → target**, byte-read at the call: `0x61309b push eax` (the + /// candidate) `; mov ecx,edi` (the actor) `; call 0x6061e0`, then `cmp eax,4; jl keep`. So it is + /// [`reaction_from_player`], the leg-3 direction that answers a reputation-slot faction with the + /// **at-war bit** — *not* [`ring_reaction`], which this used to call and which reads the + /// standing from the other side (1674; the two are deliberately not each other's mirror, and + /// substituting one for the other is the defect 1530 named). With the standing here, pressing + /// Attack while holding a not-at-war Cenarion Circle NPC kept it as the target and then failed + /// the final gate; the reference drops it and goes and finds a real enemy. /// /// **The actor is the player here, and in the reference it is the caller's** — the pet on the /// pet arm (`0x4bd40d` passes the pet object). The two agree: vmangos gives a pet its owner's @@ -278,12 +288,12 @@ impl EnemyScan<'_, '_> { .iter() .find(|(_, _, g, _, _, _)| g.0 == guid) .is_some_and(|(_, _, _, _, store, _)| { - ring_reaction( + reaction_from_player( self.factions.as_deref(), &self.reputations, store, self_store, - ) <= 3 + ) < 4 }) } @@ -724,8 +734,12 @@ fn keeps_held_target(selection: Option, hostile: impl Fn(u64) -> bool) -> O /// `UNIT_DYNAMIC_FLAGS` bit 5 is set (`0x613159 shr edx,5; test dl,1`); /// - `CanAttack 0x606980` — the full predicate this time, not the bare reaction the fork above uses. /// -/// A target with no streamed descriptor passes both, matching [`can_attack`]'s own missing-data -/// posture: nothing known to disqualify is not a disqualification. +/// A target with no streamed descriptor is alive by the first leg (nothing known to disqualify) and +/// then **refused by the second**, which is [`can_attack`]'s own missing-data posture since 1674: +/// the reference reaches this gate holding two live CGUnits and has no null path at all, so the +/// honest stand-in is to refuse rather than to swing at an object we know nothing about. The result +/// is `ERR_INVALID_ATTACK_TARGET`, which is the same answer the gate gives for every other way a +/// target can be wrong. fn attack_target_valid( store: Option<&ObjectStore>, factions: Option<&Factions>, @@ -1070,9 +1084,14 @@ mod tests { const HEALTH: u16 = 22; const DYNFLAGS: u16 = 143; const FLAGS: u16 = 46; + const TPL: u16 = 35; let reps = Reputations::default(); let unit = |pairs: &[(u16, u32)]| ObjectStore(ObjectFields::from_pairs(pairs)); - let valid = |s: &ObjectStore| attack_target_valid(Some(s), None, &reps, None); + // `CanAttack` needs both sides now (1674), and with no catalog both reactions resolve to + // neutral — the mixed arm's `< 4`, so a plain live mob is attackable and the legs below + // are the ones actually under test. + let me = unit(&[(TPL, 1), (FLAGS, 1 << 3)]); + let valid = |s: &ObjectStore| attack_target_valid(Some(s), None, &reps, Some(&me)); assert!(valid(&unit(&[(HEALTH, 120)])), "a live mob"); assert!(!valid(&unit(&[(HEALTH, 0)])), "a corpse"); @@ -1084,8 +1103,16 @@ mod tests { !valid(&unit(&[(HEALTH, 120), (FLAGS, 1 << 25)])), "NOT_SELECTABLE is one of CanAttack's disqualifiers" ); - // No descriptor is no disqualification — `can_attack`'s own posture, unchanged. - assert!(attack_target_valid(None, None, &reps, None)); + // No descriptor on either side is a REFUSAL now — see the fn doc. The reference holds two + // live CGUnits here and has no null path; swinging at an object we know nothing about was + // the approximation's posture, not the binary's. + assert!(!attack_target_valid(None, None, &reps, Some(&me))); + assert!(!attack_target_valid( + Some(&unit(&[(HEALTH, 120)])), + None, + &reps, + None + )); } /// The history: pruning honors the window, a re-visit moves to most-recent. diff --git a/crates/benilla-app/src/ui_action/cast_target.rs b/crates/benilla-app/src/ui_action/cast_target.rs index f4c42f6c..6e22dbd4 100644 --- a/crates/benilla-app/src/ui_action/cast_target.rs +++ b/crates/benilla-app/src/ui_action/cast_target.rs @@ -547,11 +547,19 @@ mod tests { /// The three wire shapes without any world state: mask 0 self-commits (never the selection — /// the Battle Shout/Ice Armor bug), unit masks refuse without a candidate, and the no-target /// vs wrong-target refusals use the client's two error strings. + /// + /// Both stores are present but empty, because `CanAttack` refuses outright when either side is + /// missing (1674) and this test is about the wire shapes, not about that posture. #[test] fn resolution_wire_shapes() { + use benilla_protocol::ObjectFields; + // `UNIT_FIELD_FLAGS` bit 3 (player-controlled) on us alone selects `CanAttack`'s mixed + // arm; with no catalog the reaction resolves neutral, so the enemy bit clears. + let me = crate::net::ObjectStore(ObjectFields::from_pairs(&[(46, 1 << 3)])); + let it = crate::net::ObjectStore(ObjectFields::from_pairs(&[(35, 0)])); let rel = TargetRelations { - target_store: None, - self_store: None, + target_store: Some(&it), + self_store: Some(&me), factions: None, reputations: &Reputations(Vec::new()), }; @@ -566,7 +574,8 @@ mod tests { resolve_cast_target(Some(&fireball), &cands(None, Some(1)), true, &rel), CastWireTarget::Refused(ERR_NO_TARGET) ); - // Reaction defaults to neutral (3) with no stores: attackable (≤3), not assistable. + // With no catalog the reaction resolves neutral (3): the mixed arm's `< 4` is attackable, + // and `CanAssist`'s `>= 4` is not. assert_eq!( resolve_cast_target(Some(&fireball), &cands(Some(42), Some(1)), true, &rel), CastWireTarget::Unit(42) diff --git a/crates/benilla-app/src/ui_action/usable.rs b/crates/benilla-app/src/ui_action/usable.rs index 616cf428..c634aab1 100644 --- a/crates/benilla-app/src/ui_action/usable.rs +++ b/crates/benilla-app/src/ui_action/usable.rs @@ -253,7 +253,11 @@ mod tests { // Field indices mirrored from the protocol crate's own consts (private there): health 22, // power1 23, flags 46, aurastate 125, bytes_1 138. fn player(pairs: &[(u16, u32)]) -> ObjectStore { - let mut base = vec![(22u16, 100u32), (23, 500)]; + // `UNIT_FIELD_FLAGS` bit 3 (`UNIT_FLAG_PVP_ATTACKABLE`, behaviourally "player-controlled") + // — every real player carries it, and `CanAttack 0x606980` selects which of its three + // terminal arms to run on that bit for BOTH parties (1674). Without it the fixture takes + // the NPC-vs-NPC arm, which wants a hostile reaction rather than a non-friendly one. + let mut base = vec![(22u16, 100u32), (23, 500), (46, 1 << 3)]; base.extend_from_slice(pairs); ObjectStore(ObjectFields::from_pairs(&base)) } diff --git a/crates/benilla-app/src/ui_chat/input/mod.rs b/crates/benilla-app/src/ui_chat/input/mod.rs index 505004ee..bacc5830 100644 --- a/crates/benilla-app/src/ui_chat/input/mod.rs +++ b/crates/benilla-app/src/ui_chat/input/mod.rs @@ -460,6 +460,65 @@ pub(super) fn drain_chat_input( own_store ) )); + // **The at-war bit, named** (decision 1674). It is the entire content of the + // player→unit reaction's leg 3, it is what the world cursor, the plate category and + // `UnitCanAttack` all turn on for a reputation-slot faction, and it is printed + // nowhere else — so "why does this friendly NPC take the sword?" is unanswerable + // from any other line here. + let war = (|| { + let catalog = factions.as_deref()?.catalog(); + let tpl = catalog.template(target_store?.0.unit_faction_template()?)?; + let at_war = + crate::target::ring::at_war_with(catalog, reputations, tpl.faction); + Some(match at_war { + Some(at_war) => format!( + "faction {} {:?} owns reputation slot {} → leg 3 answers with AT WAR \ + = {at_war} (reaction {}); the standing is never read", + tpl.faction, + catalog.faction_name(tpl.faction).unwrap_or(""), + catalog + .reputation_faction(tpl.faction) + .map_or(-1, |i| i.rep_index), + if at_war { "hostile" } else { "friendly" }, + ), + None => format!( + "faction {} {:?} has NO reputation slot → the template comparator \ + answers; at-war does not apply", + tpl.faction, + catalog.faction_name(tpl.faction).unwrap_or(""), + ), + }) + })() + .unwrap_or_else(|| "at-war not evaluated (no catalog, store or template)".into()); + lines.push(format!("reaction: {war}")); + // The world cursor's two branch predicates, in the order `0x482200` runs them — + // `CanInteract` picks the service ladder or the loot/skin/attack block, then + // `CanAttack` decides the sword. Neither is a reaction threshold, so neither can + // be read off RANK above. + let interactable = crate::target::can_interact( + target_store, + factions.as_deref(), + reputations, + own_store, + ); + let attackable = crate::target::can_attack( + target_store, + factions.as_deref(), + reputations, + own_store, + ); + lines.push(format!( + "reaction: CURSOR npc_flags 0x{:04x} · can_interact {interactable} · \ + can_attack {attackable} → {}", + target_store.map_or(0, |s| s.0.unit_npc_flags()), + if interactable { + "the NPC service ladder (a bit it consults → its cursor; none → Point)" + } else if attackable { + "the ATTACK sword (grayed past 10.45 yd)" + } else { + "nothing matched → Point" + }, + )); // The V-plate CATEGORY, which is a **different predicate** from the rank above // (`CanAttack` player→unit, plus `CanCooperate` for a player subject — decision // 1530) and so cannot be read off it. Both legs are printed, with the faction-group @@ -479,8 +538,10 @@ pub(super) fn drain_chat_input( is_player, ); lines.push(format!( - "reaction: plate is_player {is_player} · faction-group mask self {} \ + "reaction: plate is_player {is_player} (OBJECT_FIELD_TYPE says {:?}; the two \ + must agree — the predicates read the field, 1674) · faction-group mask self {} \ target {} · can_cooperate {} · can_attack(player→unit) {}", + target_store.and_then(|s| s.0.object_type()), mask(own_store), mask(target_store), crate::target::ring::can_cooperate_with_player( diff --git a/crates/benilla-protocol/src/events.rs b/crates/benilla-protocol/src/events.rs index b3bf7129..eb18452b 100644 --- a/crates/benilla-protocol/src/events.rs +++ b/crates/benilla-protocol/src/events.rs @@ -109,6 +109,33 @@ pub enum SessionEnd { Lost, } +/// **Who refused the login, and with which byte.** +/// +/// A login crosses two servers and each has its own result enum. They overlap numerically and mean +/// unrelated things — 0x0C is `AUTH_LOGON_FAILED_SUSPENDED` to realmd and `AUTH_OK` to the world +/// server — so a bare `Option` could not say what it held, and the screen could not look up the +/// right authored string from it. It used to be exactly that bare byte: the realmd code rode it and +/// the world code was formatted into the reason string and lost, which is why every world-side +/// refusal reached the player as a generic "Unable to connect". +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum LoginRefusal { + /// realmd's logon-proof `AuthLogonResult` (`crate::AuthReject`) — bad password, banned, … + Logon(u8), + /// The world server's `SMSG_AUTH_RESPONSE` code (`crate::WorldAuthReject`, the + /// `messages::AUTH_*` block) — session expired, server shutting down, … + World(u8), +} + +impl LoginRefusal { + /// The raw byte, for logging. Deliberately not a `From`/`Into`: the whole point of the type is + /// that the byte alone is ambiguous, so getting it back should look like a narrowing. + pub fn byte(self) -> u8 { + match self { + LoginRefusal::Logon(b) | LoginRefusal::World(b) => b, + } + } +} + /// One decoded event from the world stream. Carries only primitives + the coarse [`EntityKind`] /// classification — no wire types leak to the app, and no running state lives here. #[derive(Debug, Clone)] @@ -124,9 +151,14 @@ pub enum SessionEvent { /// the server is simply unusable by this client (e.g. [`crate::WardenRequired`]). The app must /// surface `reason` and stop, never fold it into the paced-resubmit path. LoginFailed { - code: Option, + refusal: Option, reason: String, terminal: bool, + /// Set when the failure was the **dial itself** — the socket never opened — carrying which + /// of the two reasons it was and the address it was aimed at (decision 1667's follow-up). + /// `None` for every other transport failure (a handshake that got a socket and then broke) + /// and for every server refusal, which have a `code` instead. + dial: Option, }, /// The account's character roster (`SMSG_CHAR_ENUM`): the world socket is authenticated and /// parked at character select. The IO thread emits this each connection cycle, then waits for diff --git a/crates/benilla-protocol/src/lib.rs b/crates/benilla-protocol/src/lib.rs index c53231dc..5d500f40 100644 --- a/crates/benilla-protocol/src/lib.rs +++ b/crates/benilla-protocol/src/lib.rs @@ -14,7 +14,8 @@ pub mod wire; pub mod world; pub use auth::AuthReject; pub use events::{ - decode, CharAction, EntityKind, LoginStage, MoveSpeeds, Poll, SessionEnd, SessionEvent, + decode, CharAction, EntityKind, LoginRefusal, LoginStage, MoveSpeeds, Poll, SessionEnd, + SessionEvent, }; pub use messages::{ CharCreateReq, CharEnumItem, Character, CreateSpline, ItemInfo, JumpInfo, MonsterMoveFacing, @@ -22,7 +23,9 @@ pub use messages::{ CHARACTER_FLAG_GHOST, CHARACTER_FLAG_HIDE_CLOAK, CHARACTER_FLAG_HIDE_HELM, CHARACTER_FLAG_RENAME, }; -pub use world::{WardenRequired, WorldReader, WorldSession, WorldWriter, WORLD_PORT}; +pub use world::{ + WardenRequired, WorldAuthReject, WorldReader, WorldSession, WorldWriter, WORLD_PORT, +}; use std::net::TcpStream; @@ -52,6 +55,72 @@ pub fn host_port(host: &str, default: u16) -> (&str, u16) { } } +/// The dial never got a socket — **and which of the two reasons it was**. +/// +/// This distinction only started mattering when the realmlist became something a player types +/// (decision 1667). Before that the address was a constant and "Unable to connect" could only mean +/// one thing; afterwards it means either *you typed a name that does not exist* or *the address is +/// fine and nothing is answering on it*, and a player with no way to tell them apart edits a +/// correct address over and over. That is exactly what the first live use of the editor produced. +/// +/// The split is drawn where it is honest — resolution and connection are two separate operations, +/// so we do them separately rather than reading tea leaves out of one `io::Error` (a DNS failure +/// surfaces as `ErrorKind::Uncategorized` on macOS, so the kind cannot carry this). +#[derive(Debug, Clone)] +pub struct DialFailure { + /// What we tried to reach, as the player would recognise it (`host:port`). + pub address: String, + /// `true` = the host name resolved to nothing; `false` = it resolved and the connection failed. + pub unresolved: bool, +} + +impl std::fmt::Display for DialFailure { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if self.unresolved { + write!(f, "cannot find a server called {}", self.address) + } else { + write!(f, "nothing answered at {}", self.address) + } + } +} + +impl std::error::Error for DialFailure {} + +/// Resolve `host:port` and open a socket to it, reporting the two failures apart ([`DialFailure`]). +/// +/// `to_socket_addrs` is the resolution step on its own; an empty result is a name that resolved to +/// no addresses, which is as much a "cannot find it" as an outright lookup error. Connecting to the +/// resolved list (rather than to `(host, port)` again) also means the OS does not repeat the +/// lookup, and that every address the name carries is tried — the `localhost` → `::1` then +/// `127.0.0.1` case on a machine whose server binds only IPv4. +fn dial(host: &str, port: u16) -> Result { + use std::net::ToSocketAddrs; + let address = format!("{host}:{port}"); + let addrs: Vec = match (host, port).to_socket_addrs() { + Ok(addrs) => addrs.collect(), + Err(e) => { + return Err(anyhow::Error::new(DialFailure { + address, + unresolved: true, + }) + .context(format!("resolving {host}: {e}"))) + } + }; + if addrs.is_empty() { + return Err(anyhow::Error::new(DialFailure { + address, + unresolved: true, + })); + } + TcpStream::connect(&addrs[..]).map_err(|e| { + anyhow::Error::new(DialFailure { + address, + unresolved: false, + }) + .context(format!("connecting: {e}")) + }) +} + /// A realm as advertised by the auth server's realm list. #[derive(Debug, Clone)] pub struct RealmInfo { @@ -96,8 +165,7 @@ pub fn logon(host: &str, username: &str, password: &str) -> Result { let (mut stream, reply, server_public_key) = { let mut dialed = None; for _ in 0..MAX_CHALLENGE_DIALS { - let mut stream = TcpStream::connect((host, port)) - .with_context(|| format!("connecting to {host}:{port}"))?; + let mut stream = dial(host, port)?; auth::write_logon_challenge(&mut stream, &username.to_uppercase(), CLIENT_BUILD) .context("sending logon challenge")?; let reply = diff --git a/crates/benilla-protocol/src/messages/mod.rs b/crates/benilla-protocol/src/messages/mod.rs index 69efb897..64827cfc 100644 --- a/crates/benilla-protocol/src/messages/mod.rs +++ b/crates/benilla-protocol/src/messages/mod.rs @@ -222,8 +222,35 @@ pub use vendor::{ }; pub use world_state::InitWorldStates; -/// `SMSG_AUTH_RESPONSE` AuthOk result. +/// The **world server's** `SMSG_AUTH_RESPONSE` result codes — a **different enum** from the +/// realmd logon-proof's `AuthLogonResult` (`crate::AuthReject`, 0x00..=0x12), which is the trap +/// this block exists to close: both are "the auth result byte", they overlap numerically, and they +/// mean unrelated things. 0x0C is `AUTH_OK` here and `AUTH_LOGON_FAILED_SUSPENDED` there. +/// +/// VERIFIED two ways. The client's own dispatch over exactly this enum is decompiled in wow-re +/// `system/net/scratch/w2b-pack.c` — case 0x0c → `AUTH_OK`, 0x0d → `AUTH_FAILED`, 0x0e → +/// `AUTH_REJECT`, … 0x15 → `AUTH_UNKNOWN_ACCOUNT`, 0x16 → `AUTH_INCORRECT_PASSWORD`, 0x1b → +/// `AUTH_WAIT_QUEUE`, 0x22..=0x26 → the `REALM_LIST_*` strings — and the same numbering is +/// `AuthResponseCodes` in cmangos `src/game/Globals/SharedDefines.h:1721+`. Each constant's name +/// is the `GlueStrings` key the client shows for it, which is what makes the mapping in +/// `crate::login`'s `world_refusal_text` a transcription rather than a judgement call. pub const AUTH_OK: u8 = 0x0C; +pub const AUTH_FAILED: u8 = 0x0D; +pub const AUTH_REJECT: u8 = 0x0E; +pub const AUTH_BAD_SERVER_PROOF: u8 = 0x0F; +pub const AUTH_UNAVAILABLE: u8 = 0x10; +pub const AUTH_SYSTEM_ERROR: u8 = 0x11; +pub const AUTH_BILLING_ERROR: u8 = 0x12; +pub const AUTH_BILLING_EXPIRED: u8 = 0x13; +pub const AUTH_VERSION_MISMATCH: u8 = 0x14; +pub const AUTH_UNKNOWN_ACCOUNT: u8 = 0x15; +pub const AUTH_INCORRECT_PASSWORD: u8 = 0x16; +pub const AUTH_SESSION_EXPIRED: u8 = 0x17; +pub const AUTH_SERVER_SHUTTING_DOWN: u8 = 0x18; +pub const AUTH_ALREADY_LOGGING_IN: u8 = 0x19; +pub const AUTH_LOGIN_SERVER_NOT_FOUND: u8 = 0x1A; +/// The realm is full and we are **queued**, not refused — the one code here that is not an ending. +pub const AUTH_WAIT_QUEUE: u8 = 0x1B; /// `LogoutResult::Success` (`SMSG_LOGOUT_RESPONSE`). pub const LOGOUT_SUCCESS: u32 = 0x0; /// Chat `Language` wire ids (VERIFIED vmangos `SharedDefines.h:256-261`): the faction tongues. diff --git a/crates/benilla-protocol/src/messages/update_object/fields/player.rs b/crates/benilla-protocol/src/messages/update_object/fields/player.rs index 9d287efe..100a7a9c 100644 --- a/crates/benilla-protocol/src/messages/update_object/fields/player.rs +++ b/crates/benilla-protocol/src/messages/update_object/fields/player.rs @@ -872,8 +872,8 @@ impl ObjectFields { } /// The object's class from the type field (`OBJECT_FIELD_TYPE`), used for `Values`/`Movement` - /// updates that carry no separate `TypeId`. Unused by the create path (which has a `TypeId`). - #[allow(dead_code)] + /// updates that carry no separate `TypeId`. Also the reference's own "is this a Player" test + /// (`[obj->descriptorBlock[2] + 8] >> 4 & 1`), which is how `CanAttack`/`CanInteract` ask. pub fn object_type(&self) -> Option { self.get_u32(FIELD_OBJECT_TYPE).map(|bits| { if bits & 0x10 != 0 { diff --git a/crates/benilla-protocol/src/world/mod.rs b/crates/benilla-protocol/src/world/mod.rs index 6d2ea02e..56db9e81 100644 --- a/crates/benilla-protocol/src/world/mod.rs +++ b/crates/benilla-protocol/src/world/mod.rs @@ -28,7 +28,7 @@ mod session; mod writer; pub use reader::WorldReader; -pub use session::{WardenRequired, WorldSession}; +pub use session::{WardenRequired, WorldAuthReject, WorldSession}; pub use writer::WorldWriter; /// Default `mangosd` world-server port — the stock one, which our vmangos deploy maps straight diff --git a/crates/benilla-protocol/src/world/session.rs b/crates/benilla-protocol/src/world/session.rs index a1023826..17deaa7f 100644 --- a/crates/benilla-protocol/src/world/session.rs +++ b/crates/benilla-protocol/src/world/session.rs @@ -28,6 +28,35 @@ const HANDSHAKE_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_se #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct WardenRequired; +/// The **world server** refused the session (`SMSG_AUTH_RESPONSE` with anything but +/// [`messages::AUTH_OK`]). +/// +/// Typed, and carrying its raw code, for the same reason [`crate::AuthReject`] is: the screen owes +/// the player the client's own authored words for what happened, and it cannot look them up from a +/// formatted string. Before this the code was interpolated into a `bail!` and thrown away, so every +/// world-side refusal — expired session, server shutting down, already logging in — arrived at the +/// login screen as the same generic "Unable to connect". +/// +/// **Its codes are `messages`' `AUTH_*` block, NOT [`crate::AuthReject`]'s.** See that block for +/// why the distinction is load-bearing: the two enums overlap numerically and mean unrelated +/// things. +#[derive(Debug, Clone, Copy)] +pub struct WorldAuthReject { + pub code: u8, +} + +impl std::fmt::Display for WorldAuthReject { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "world server refused the session: result {:#04x}", + self.code + ) + } +} + +impl std::error::Error for WorldAuthReject {} + impl std::fmt::Display for WardenRequired { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( @@ -131,7 +160,7 @@ impl WorldSession { match session.recv()? { ServerPacket::AuthResponse { result } if result == messages::AUTH_OK => break, ServerPacket::AuthResponse { result } => { - bail!("world auth rejected: result {result:#x}") + return Err(WorldAuthReject { code: result }.into()) } ServerPacket::Other { opcode: opcode::SMSG_WARDEN_DATA,