Commit Graph
6 Commits
Author SHA1 Message Date
Sam SungandClaude Opus 5.5 1d1f0a2c03 Comments in benilla-app, the glue screens, harness, settings and the rest: one line of fact each, no history, no record numbers (#365)
Part of #41: the last of five `benilla-app` pieces. This one is everything the others did not cover: the addon harness, the settings registry, the glue screens (login, realm list, character select and create), key bindings, the minimap, nameplates, the loading screen, transports, the performance instruments, the smaller modules, the integration tests, and the UI asset files (`OptionsFrame.xml`, `benilla.toc` and the rest). Their comments are rewritten to the rule in `docs/METHOD.md`, which is what the code does and the reference fact behind it, in a line. 27,351 comment lines become 11,010.

Fixes #33. Fixes #103.

- **The mechanical strip.** The citations that were only a private record number go first, and each edit is shown to be a pure removal.
- **Wrong facts corrected.** The rewrite corrects the comments that stated wrong facts. Among them:
  - The weapon trail's emissive is 0 (`0x70ca30`): the trail is a type-5 render record, so its ambient-only light term is exact.
  - The camera-style dropdown writes "1", "2", "0" (`UIOptionsFrame.lua:525,536,547`); several register-site addresses in the settings table are corrected.
  - The graphics settings are held until `RestartGx`, as in 1.12; `Fonts.xml`'s early load no longer carries a false reason.
  - vmangos sends a player-name invalidation after a rename (`CharacterHandler.cpp:839`), and the cinematic camera's announce packet has an empty handler there (`MiscHandler.cpp:919`).
  - Stock FrameXML and GlueXML line citations are checked against the stock 1.12 files, and vmangos line citations against the pinned revision.
- **Deviation labels.** "Deviation:" now marks only a choice with its recorded reason. Declining to copy a reference defect is stated plainly, and so is an unbuilt behaviour or an untraced one.
- **Verification.** Every file strips to identical code, apart from test literals in `login/`, `death/` and `ui_script/death_tests.rs` that now use placeholder account and character names, and `cargo fmt --check` is clean. Six second readers compared every file old against new, restored the facts the rewrite had dropped, and checked each new citation against the code, the stock files, vmangos and the reference.

A second commit corrects one comment in `benilla-ui` (`unit_is_grey`: the reference's exclusion is players only). A third lowers `ui_script_wall`'s ceiling from 45 to 40: five gameplay files named `UiScript` only in comments the rewrite removed, and the ratchet asks for the number to follow.

Behaviour differences the rewrite surfaced are filed as their own issues (#339 to #353 among them). String literals are untouched; they are #92.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2026-09-26 00:27:56 +07:00
SamandClaude Fable 5.1 baf4e37070 the create screen marks your pick, and a new character inherits addons
- the character-create screen marks the chosen race, gender and class again — a lit square and its name on the icon, as the reference's LockHighlight draws it; a query term had filtered all eighteen icons out since 2072
- a new character inherits the addon enables the realm's other characters agree on, as the reference computes them: a Disable All saved just before no longer comes back all-checked (2311)
- an auction list-update event names one list: a first search on Browse no longer throws the stock Auctions tab's page error, and a sale or outbid notification never fetches a list the window never asked for (2308)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-19 17:09:59 +07:00
SamandClaude Fable 5.1 8562fb0ef1 a crash leaves a report, and the wire's counts are capped
- a panic writes benilla-config/Diagnostics/crash-<time>.txt with the build id, the location, a backtrace and the last 200 log lines, and names the file as its last line, so a bug report can attach it (2266)
- every wire-count allocation hint is capped and the compressed update object reads through the cursor, so a malformed packet can no longer ask for gigabytes (2266)
- the 23 in-world run conditions are one set, the 45 probe variables one registry, the install-gated tests say when they skip, and every gate is timed (2266)
- one workspace-level lint allowance replaces 413 per-site attributes, and the decision corpus gets a lint in the gate (2266)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-17 16:38:59 +07:00
SamandClaude Fable 5.1 1d7252aec2 unselectable units take no target, quieter wounds, a realm-list dialog
- a NOT_SELECTABLE unit takes no target: Stratholme's Baron Rivendare, pre-event, is spawned by vmangos with `UNIT_FLAG_NOT_SELECTABLE | UNIT_FLAG_SPAWNING`, and where the reference can neither damage nor target him benilla refused the damage (`CanAttack` already read bit 25) and took him as a target anyway — most visibly through the ATTACKERSTATEUPDATE self-defence acquire, which hands you your attacker the moment he opens on you with no click and no hover involved, and every other writer (left-click, right-click, TAB, `TargetUnit`/`/target`/`/assist`, the attack auto-acquire) had the same hole; the reference's gate is the third early-out of `SetSelection` `0x493540` (`0x4935ee`/`0x4935f3`): the resolved object's `+0x58` vcall to `0x60be60` `IsSelectable` = `!(UNIT_FIELD_FLAGS & 0x02000000) || UNIT_FIELD_CREATEDBY == your guid`, sitting BEFORE the dedup so a refusal is a complete no-op — no `CMSG_ATTACKSTOP`, no `CMSG_SET_SELECTION`, the target already in hand survives — and skipped entirely for an unresolved object, which is what keeps an out-of-range party member selectable off the roster; `target::relations::is_selectable` transcribes it whole, the CREATEDBY clause (your own totem stays clickable, somebody else's does not) and the xor-eax base stub every non-unit vtable carries at `+0x58` included, and `scan::commit` runs it first behind a mandatory `target_store` parameter so none of its eight call sites can forget a gate it does not compute; the hover is the SAME predicate at a second site, byte-verified by the wow-re round this dispatched: the hover grader `0x4828d0` calls slot 21 directly on the pick's winner at `0x482982`, and a false answer jumps to `0x482090(0,0)` + `ResetCursor` `0x523d30`, skipping the publisher `0x492890` that holds the mouseover globals' only two writers image-wide — so no tooltip, no cursor, no emissive brighten and no `mouseover` token, all from one rule; it is a GRADER, not a filter, and the difference is observable — a census over the pick itself (`0x480a50`, `0x480d90`, `0x713cb0`, `0x481190`, `0x480df0`) finds zero reads of the unit flags against four inside `CanAttack`, so the flagged unit wins the trace and is then thrown away, which `Hovered::refused` carries with the pick's distance kept so a trigger stalker in front of a chest takes the pick and the chest behind it does not inherit the mouseover; graded at `update_hover`'s publish rather than in a system of its own, because `drive_mouseover_tooltip` is ordered only after the net stage and a second system would have bought a one-frame name plate for a unit that must never have one; the V-plate suppression turns out to be load-bearing, since a plate hover publishes the mouseover directly (`0x7cb869 call 0x492890`) with no grading at all; and a left-click on a refused unit no longer clears what you had — the grader had dropped it into `select_on_click`'s arm beside "clicked the sky", where the reference's deselect lives in the terrain and NOTHING legs only: the down-edge pick `0x481f00` still found the object, so the release takes the object leg `0x4925d0` whose tail hits `SetSelection`'s early-out and changes nothing, the reasoning the corpse guard beside it already carried and which gets its first test here (2060)
- three things a landed hit showed that the reference does not: a lootable corpse took blood, flinch, impact sounds and a floating number — the victim dispatcher `0x624530`, which owns every victim-side consequence of a swing, opens with `test byte [[victim+0x110]+0x224],1` (`UNIT_DYNAMIC_FLAGS` bit `0x1`) and jumps past all four — but the obvious fix, gating the victim, is wrong: benilla fans that dispatcher out into one message emitted on three occasions and the reference gates the DISPATCHER, and an image-wide census of the number emitter `0x6243e0` (exactly three call sites, zero tail-jumps, zero address-takes, earned against a positive control) puts `0x624566` inside the gate but the `SMSG_ATTACKSTOP` flush `0x624ea7` and the supersede flush `0x625893` outside it, so a lootable victim still floats a number for a superseded record; gated: the impact-tag dispatch (record still consumed first, as `0x6247d0` clears the GUIDs before calling) and the unresolved-attacker fallback `0x625a6d`; ungated: both flushes and the whiff slow-down, `0x624ca0` being its own function past the dispatcher's end; the flush's own predicate `0x625e40` reads an unnamed `[rec+0xac]` and stays unbuilt rather than guessed (2066); a crit played a full CombatCritical clip on a fighting unit — vmangos sends `SMSG_EMOTE(EMOTE_ONESHOT_WOUNDCRITICAL)` to the victim of every damaging crit (`Unit.cpp:1601`), `Emotes.dbc` resolves it to AnimID 10, and on the director's 90 s brawl trace that was 7 one-shots plus 9 fast-path entries on the warrior cutting its own swing (`fastpath cur=87 req=10`), 19 across all units; the reference's shared emote player `0x5fcd20` refuses at `0x5fcd96` when `0x60ecd0` is true — `UNIT_FIELD_FLAGS` bit `0x800` OR an auto-attack target `[+0xc48]` — and 1469 had built the flag half alone, dormant on vmangos (its `0x800` is `PET_IN_COMBAT`; a fighting player or creature carries `IN_COMBAT 0x80000`, a bit the client never tests), so `emote_anim::play_eligible` takes `engaged` and both producers, `SMSG_EMOTE` and the chat gesture, pass `Has<Engaged>` through one `PerformerQuery`: a crit on a fighting unit shows only as the wound overlay, and the wound trace lines carry `unit=` (2067); and an ending effect's particles popped — the teardown `0x6203e0` that both the completion callback and the same-slot replace reach hides the node and queues it rather than freeing it, and whether that is visible turned on a half no wow-re note covered, dispatched rather than assumed: the draw path's own admission test is the live-particle count (`0x7b4b46 mov eax,[esi+0x64]`) and the model enqueues onto two scene lists on two fields — MESH on the visible flag (`0x719207`), PARTICLE on the aggregate `CM2Model+0x3d8` (`0x71922c`), set while any particle lives and the same word `0x61f680` frees on — so for `Strike_Impact_Chest` a node destroyed at 0.10 s still shows ~30 particles at 0.20 s, gone by 0.334 s, and benilla cut up to 0.30 s of a 0.334 s effect; the three points where an effect ENDS — expiry, the same-slot replace, the reap's immediate destroy — now hand their emitters to the drain by the instance root they already carry as anchor (`drain_instance_emitters`, `ParticleEmitter::drain_on_owner_loss`) and the root still despawns, the reference dropping the mesh the same frame; flipped at the ending rather than set at spawn because the other way an owner vanishes is the model dtor (gear change, display swap, stream-out), where the reference frees synchronously and draining that case is what stranded ghost clouds in the air (0826/0833), and the collateral path still frees (2070)
- a creature whose template carries `DO_NOT_PLAY_WOUND_ANIM` takes no flinch and no parry, and the wound grunt is rolled: the creature template's `type_flags` carries the bit at `0x8`, benilla consumed the two bits either side — `0x10 NO_FACTION_TOOLTIP` in the unit tooltip, `0x20 MORE_AUDIBLE` in the parked-event gate — with neither reader naming its bit, and `wound.rs` carried the gap in a doc comment claiming "no template cache" that had been false since the cache landed; the reference refuses the flinch inside `0x60ea70` itself (`0x60ea9f`, so every trigger is covered) through the getter `0x6125f0`, whose null-record leg fails OPEN, so `names::type_flags` names all eight bits (a call site could say `& 0x10` and look right — that is what let it be wrong) and the wound trigger refuses a flagged victim as its first gate, keyed off the descriptor's `OBJECT_FIELD_ENTRY` the way `0x60b160` keys the cache; animation only, since no creature-type mechanism suppresses the blood spurt — a flagged creature bleeds (2068); the vocal half was dispatched rather than built, because wow-re's published "makes no pain sound" called the flag's second consumer `0x60ec00` a pain-sound dispatcher while three of its own notes said otherwise, and the cross-check came back with all seven questions VERIFIED: `0x60ec00` is an ANIMATION dispatcher — `GetWeapon(0,0)`, the mainhand subclass through the 18-entry remap `0x60ec98` and jump table `0x60ec84` to Parry1H/2H/2HL/Unarmed, handed to `0x5fe2f0` = `PlayAnimation`, whose 40 call sites push literals in `[16, 202]` and which reaches no sound band — so the flag gates two animations and no sound, and the parry now rides it (victimState 3 only; Dodge 30 and Block 24 reach `PlayAnimation` directly from `0x624a90`/`0x624a74`, and our subclass map agrees with the raw remap on all 18 rows); nothing suppresses the injury vocal — of the fourteen functions in the wound-vocal chain the only one dereferencing `[unit+0xb30]` is `0x623b70`, testing bit 5 for a bus — but the ordinary grunt IS rolled: `0x623490` runs the class chance roll at `0x8626d4[2] = 60` for a creature and `0x86424c[2] = 30` for a player, by the VICTIM's type, while classes 3 (critical) and 9 (crushing) carry 100 and always sound, so `INJURY_CHANCE_CREATURE`/`INJURY_CHANCE_PLAYER` thin the grunt under sustained melee where benilla had played all three every time — 40 grunts per 100 too many on a creature, 70 on a player — and a crushing blow's class 9 punches through a server-pushed object sound's suppression, which stops at class 8 (`0x6234bb`/`0x6234bf`/`0x6234c4`); a pet gets a template record at last, since `NameCache::resolve` routes a `HIGHGUID_PET` guid to the pet-NAME query and a tamed unit had none — 18 of the 471 flagged templates on the world DB carry a `pet_family` — so the create arm also asks by the descriptor's entry, the reference's own key (`[[unit+8]+0xc]`); 2068's claim that templates are queried lazily is corrected, the create arm having warmed the cache for every streamed unit all along, at the reference's own timing (`0x60afb0` → `0x60b157`, on the create path) rather than as the convenience its comment called it; and the skeletons of the report still flinch, and should — neither vmangos nor cmangos flags them (2 of 62 templates in each), `Skeleton.m2` authors CombatWound and CombatCritical, so what changes for them is the grunt's rate, not its existence (2073)
- the realm list is a dialog, not a screen: the first live run of 2056's realm selection reported no hover highlight, Cancel and the X throwing you out of the session instead of back to character select, the character screen gone from behind it, and then a login that hung on "Connecting" forever with nothing in the log — three of the four are one mistake, since `GlueScreenInfo` names all seven glue screens and `RealmList` is not among them: `RealmList.xml` is a `frameStrata="DIALOG"` frame with a full-screen 75%-black quad that `Show()`s over whatever screen is up and `RealmList_OnCancel` only `Hide()`s it, while 2056 had built it as a `ClientState`, so Cancel had to pick somewhere to go, the screen behind was torn down, and — the one that mattered — a roster arriving while that state was up moved nothing because `to_select_on_roster` only ever leaves `Login`; Okay looked inert while the IO thread walked on to the character park, the next Cancel answered a park two ahead of the thread, and every later login queued behind a `pick_rx.recv()` that was never coming, silently, reproduced with a scripted walk before anything changed; the realm list is a `shown` flag now, served in place by the character park over `select!` — refreshing on the realmd socket the cycle still holds, since `RequestRealmList` (`0x46b8d0`) begins `COP_GET_REALMS` on a socket closed only at world entry (`0x46b70c`) and only `ConnectToRealm` (`0x46b210`) drops the world connection — and leaving only when a realm is picked, `HeldLogon` and the resumed cycle gone with it; Cancel sends one `Abandon` and the park that hears it decides, which is the reference's own shape: `RealmListDialogCancelled` (`0x46ed20` → `0x46b810`) compares the current screen name against "login" and returns immediately when it differs, tail-jumping `CLoginMgr::Cancel` (`0x5b3320`) only from the login screen; the hover was never a realm-list oversight — `Hilight` was "the owning screen's to drive", four screens drove it, two folding selection into the same expression and one scoped so the `GlueCloseButton` X on every panel could never light, and the realm list made five by having none — so one owner now, `glue_hilights`, lit by hover or `LockHighlight` and walking UP to the nearest enclosing button because a modal dialog's root is itself a button, with `art_swaps`, `glue_button_visuals`, `glue_hilights` and `sync_outlines` registered once in `GluePlugin` instead of hand-copied per screen; the IO thread names each park it stops at, a logout comes back through the same park a first login does, so the policy ladder puts the realm the player is currently on ahead of the persisted `realmName`, which benilla writes only at world entry, the reproduction is now a smoke leg (Change Realm → Okay → Change Realm → Cancel → Okay against the live server) that fails on the old code by construction, and two divergences are stated: the X abandons like Cancel because our thread parks on the question rather than polling for it, and Change Realm shows the list already in hand at once rather than waiting a round trip behind a status dialog (2072)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-08 16:08:46 +07:00
SamandClaude Fable 5 cf581aa2ac the auction house opens, the Honor tab lands, and a day of fixes
- the auction house end to end: right-clicking an auctioneer opens Browse/Bids/Auctions on the server's reply — search, the category tree from your own DBCs, the reference's 50-a-page pager, multi-key sorting (directions corrected at the bytes), bid/buyout, the sell slot with deposit and duration, cancel with its fee popup; prices sit on the reference's own money-frame kit in lined-up columns, the twenty auction messages split refusals-red / outcomes-to-chat, and a won or sold auction reads as a proper mail receipt — the window's Lua re-worded in our own terms after the overlap ratchet caught the first cut leaning on the reference's text
- the PvP honor system whole: the Honor tab and the inspect page over a real wire model, with current rank and highest-ever rank correctly two different fields
- a day of reported fixes: WASD survives a /logout re-entry, addon settings save on a window close and Cmd+Q (exactly once — a mac close could fire the tail twice), clams open again, random-enchant items name themselves on every surface, the loot window's rows hold their seats with legible counts, idle players stop sinking through building floors, the shooter re-pulls between Auto Shots, portraits loosen ~19% to the client's own aspect-1.0 matrix, and big creatures thump the camera off the CameraShakes table
- character select clicks like 1.12 — captions whiten on hover, buttons fire on release, Humans face straight ahead off the stage bone's real yaw, the facing is per-character, and hunter/warlock pets stand beside you at their family-ramp size with their own emitters (the imp's flames), the Voidwalker's sequence-hidden shackles culled
- the smaller player-facing set: nameplate on/off rows in Options with the CanAttack category gate (emissaries stop plating), the bonus bar slides up with igBonusBarOpen, the camera's framing height glides on a shapeshift, the open letter evicts the centre seat, the far-band fog hull stops printing a pale horizon stripe at low view distance, the guide arrow is gold with per-source rim art, error text fades through its shadow too, and /convertraid converts the party
- README: the auction house and PvP honor move out of "not built yet"

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-22 19:04:27 +07:00
SamandClaude Fable 5 3b6975fdef the pet gets its frame, flooded rooms fill, and macros really fire
- the pet gets its own unit frame beside yours, and an Attack order no longer latches as the standing command
- five rooms that always read bone dry hold their water now, the Deeprun Tram's flooded stretches among them
- macros fire through the client's own chat door, and the macro window fits its tabs and scrolls its icon chooser
- the options window grows a Nameplates page and its first dropdown, and the name-display switches apply live in the world

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-05 19:03:43 +07:00