c315fb4f35f315ce1c198101e8e14011904e37e2
1656
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  gromandClaude Opus 5
|
c315fb4f35 |
Anisotropic filtering and trilinear apply in the session
The Options row shipped last commit said the setting needed a relaunch.
It does not have to. The reference's "enabled upon restart" is about its
TEXTURES, not its CVars — both register with `flags = 1`, live — because
its sampler state was baked inside each `TextureCreate` (`0x449ae0`) and
it kept no list of live textures. Ours can be re-stamped in place.
Two things make it possible, and neither is new work:
- A governed sampler now wears `POLICY_SAMPLER_LABEL`, written by the one
constructor `TexFilterSetting::descriptor`. That is the `applyGlobal`
split the reference draws, made legible at runtime: terrain, WMO/M2 and
the liquid sheets take the policy; weather sprites, the minimap, glue,
Lua widgets, the sky and decals keep their own mode. All four governed
lanes went through that constructor, so a lane cannot forget the label
and silently opt itself out of every later change.
- Extracting a `RENDER_WORLD`-only `Image` takes its *pixels* and leaves
the asset behind (`take_gpu_data` is an `Option::take` on `image.data`).
So every resident texture's own descriptor is still readable in the main
world, for nothing — there is no ledger to keep and none to go stale.
The policy itself stops being a `OnceLock` and becomes one atomic word
plus a revision, so it can be published again on every write and both
halves of the refresh can key off the same number in the same frame:
`refresh_policy_samplers` (render world, `ExtractSchedule`, before every
Prepare) swaps `GpuImage::sampler`, and `invalidate_materials::<M>` raises
`AssetEvent::Modified` so bind groups that captured the OLD sampler are
built again. `static_gx` carries two samplers of its own and rebuilds its
cached cell bind groups in place rather than dropping them — clearing the
cache would be one line, but a region with no GPU state does not draw, so
every building in view would blink out and trickle back under the bake
budget.
Measured, not asserted. `WOW_CVAR_LATE=<name>=<value>` is new here: it
writes CVars *after* the scene has settled, then re-settles the image
around them. A latched lever and a live one give the same boot picture, so
no boot-time A/B can tell them apart. On the Elwynn vista:
anisotropic 16 at boot vs 16 written late:
0 of 1,440,000 pixels differ
and the control that proves the harness is not doing the work (boot 16,
written 16 again late, so no refresh runs but the re-settle is still paid)
is byte-identical to a plain boot. The cost of the change is 873 samplers,
75 material bind groups and 264 static_gx cell bind groups, once; nothing
is re-uploaded, since the pixels never move.
The first run of that A/B was NOT identical — 6.7% of the frame, a dapple
of stale tree shadow on a road that was otherwise correct. That is
`CutoutShadowCasterMaterial`, which deliberately reuses the forward pass's
own leaf handle and so holds a governed sampler, and which lives in
`benilla-app` rather than the engine. Hence the shape the invalidation
took: a public `invalidate_materials::<M>` each crate registers beside its
own `MaterialPlugin`, published through the world-API wall with the record
that says why (the engine naming every material type in the program is the
inversion that wall exists to stop). Forgetting one is silent in exactly
that way — a bind group built after the change is right and one already
cached is wrong — so the capture A/B is written into `docs/GRAPHICS.md` as
the check for this feature, not a unit test pretending to be one.
The Options row keeps its deferral to Apply, now for the real reason: the
commit is cheap once and silly sixteen times, which is what a drag across
the groove would ask for.
Verified: ALL GATES GREEN (520 s); `benilla-app --lib` against the install
at the exact 269-failure baseline, set-identical by `comm`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
1c786b7ecb |
Anisotropic Filtering takes its seat on the Graphics page
The CVar has been registered and read since the filter policy landed, and last commit put it on the Graphics Preset ladder — but a player had no way to reach it. The reference's own `OptionsFrameSliders` declares the row at index 7; ours left that index a hole, so the only route to anisotropy was `/console anisotropic 16`, and a player whose preset reads Custom (the seeder never overwrites one) never met the setting at all. Seated where the reference's own slider order puts it: after Brightness, before Weather Intensity. Three things it does differently, each stated beside it: - The groove runs 1 to 16, the CVar's own `ANISO_RANGE`, not the stock table's 1 to 4. That ceiling was the reference *slider's*, not the CVar's — its callback parses 1-16 (`0x689110`) — and both the High and the Ultra rung ask for 16. So the row carries literal bounds and stays out of `BENILLA_GRAPHICS_SLIDER_ROWS`, exactly as `RowRenderScale` does for a CVar the stock table never had, which leaves `OptionsFrameSliders` verbatim for any addon that reads it. - `1` reads out as "Off", not "1x": the reference writes filter mode 5 only at 2 and above, so 1 is the feature off and not its weakest stop. - The description is ours. 1.12's own OPTION_TOOLTIP_ANISOTROPIC describes a four-stop slider, and this one is sixteen. Deferred to Apply, as Vertical Sync and Render Scale are: a sampler is baked into each `Image` at load, so the write cannot reach a texture that is already resident. The tooltip says so in the row rather than leaving a player to wonder why the ground did not change — the same thing the reference's UI means by "enabled upon restart". Verified: `benilla-app --lib` at the exact 269-failure data-gated baseline, set-identical by `comm`, with the new row's own test and the two census counts (tooltip keys, raised plates) moved 116 -> 117 as the row asks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
b6f3bb001c |
Anisotropic filtering joins the Graphics Preset ladder from High
The reference ships `anisotropic` at 1, which is *off*, so ground texture at a
grazing angle smears to mush a few yards out — which is most of the screen in
an outdoor scene. The row was registered and wired (`benilla_assets::
TexFilterSetting`, the reference's own `[1, 16]` clamp at `0x689110`) but sat
outside the preset ladder, so no rung ever spent it and even Ultra left it off.
Measured on an Elwynn vista (`WOW_CAPTURE=vista`, the offline harness
`docs/GRAPHICS.md` now documents), one knob changed, 1600x900:
* **34.1% of the frame moves** (490,600 of 1,440,000 pixels by more than 2),
mean |d| 4.21, max 212. By band it peaks at **66.5%** across y 420-479 and
runs 48-66% from y 300 to y 780 — exactly where the ground recedes to the
horizon, which is what anisotropy is for. For scale, the interior
prop-probe table's whole stake is 7.6% of pixels by at most 14.
* **+1.0 ms of GPU and nothing on the main thread** (gpu 0.8 -> 1.8 ms, main
unchanged at 4.2-8.4 ms). The filter policy confirms it lands: "mode 4
(trilinear, aniso off)" -> "mode 5 (trilinear + aniso) anisotropic=16".
That ms is one this client is not spending. It is main-thread bound with the
GPU about two thirds idle — flat ~3 ms in play at every framerate, 0.8 ms in
this capture — so a sampler state is the cheapest picture on the table.
Ladder: Classic and Low keep the reference's 1, Medium takes 4, High and Ultra
16. Classic is the reference client, so it is judged against the registered
default the way `farclip` already is, not against zero — `anisotropic` is not a
0/1 lane, its "off" *is* 1. The High column differs from the reference, so it
is declared in `SEEDED_DEVIATIONS` with its reason, and
`graphics_rows_pass_their_observers_unclamped` now drives all five rungs
through the real setter and reads the level back off the sampler policy.
The sampler is baked into each texture at load, so a rung change lands on the
next launch, as the reference's own options window says ("enabled upon
restart"). A player already on a Custom preset is not seeded and keeps their
saved value; `/console anisotropic 16` and a restart is the whole fix there.
`docs/GRAPHICS.md` is new: where the frame's budget actually is, the offline
capture-and-diff harness every claim here uses, what is already shipped and
unspent, and the ordered list of what the picture is missing — costed on this
engine, with upscaling placed where the measurement puts it rather than where
it is usually asked for.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
ba909276a0 |
docs: record the bourbon fork, the engine wall, and what each tree holds
`docs/BOURBON_SYNC.md`, beside the existing `UPSTREAM_SYNC.md`: what
tripleslash/benilla-bourbon is, measured rather than read off its README, and
the three things that decide what can be taken from it.
* It is published as **one squash commit** — one branch, no tags, 1,662 files
and 1,139,563 insertions in `b91c12e5` on samwhosung's `359eff416`. There is
no per-change history, so a pick is the whole fork or nothing.
* Its renderer is a **Bevy 0.18 -> 0.19 engine move**, not a file copy. Bevy
0.19 made the render graph systems: bourbon has 0 files implementing a
`Node` and this tree has 14, ten of them our own graphics. samwhosung is on
0.18.1/wgpu 27 at both that base and his head, so the move is bourbon's own
and lives inside the squash.
* Its Turtle WoW 1.18.1 support is **web-only** in its own tree, and this
chain is Turtle-derived rather than Turtle: no `patch-T`, instead
`patch-1`..`patch-5` and `patch-A`..`patch-Q`. The content and protocol
fixes inside that programme are the portable part; the layer switch cannot
fire here.
Also the file-set split (339 files only ours, 1,374 only theirs), what each side
holds that the other has no answer for — bourbon has no SuperWoW, nampower,
ClassicAPI, UnitXP_SP3, VanillaUtils or VanillaHelpers surface at all — the four
fixes this sync landed with the numbers measured over this install's chain, the
thirteen things refused with a reason each, and the three shapes a renderer take
could have, with their costs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
b8e8fd4382 |
A creature-skin record fills its slot only where it holds a path
`Monster1/2/3` exist so `CreatureDisplayInfo`'s skin variation can be dropped in
at spawn, and the record's own filename is the fallback where the display has no
variation for that index. The fallback was unconditional, and the slot does not
always hold a name: the stock models leave whatever the exporter left there.
That matters because a filled `texture` wins over the slot downstream
(`benilla_app::entities::display::skin_for`), so junk in it would mask the
display's real variation for good.
Measured over this chain, separating the two populations by whether the name
reads as a path at all:
* **26 records in 17 models bake a real `.blp` path** and are the winning copy
— the Crab, Scorpion, Crocodile, Thunder Lizard and RavenLord mounts, the
WC2 Death Knight (body, mount and a cape out of `ITEM\ObjectComponents`),
the Wizard Frog's hat, the Duck wearing a turkey skin, the Goblin rocket
car, the Worgen Demon Mage reaching for the Felhound's skin, Incubus,
Crystal Satyr, the alternate and Northrend bear mounts. These have nothing
else to draw with.
* **55 more carry unprintable leftovers** — Banshee, every DragonSpawn,
DruidCat, FrostSabre, Lobstrok, ObsidianDestroyer and the rest — all of them
in `model.MPQ` and `patch.MPQ` copies that this chain's letter patches
shadow, so none of them is a copy a read reaches today.
So `monster_skin_name` keeps only what reads as a texture path: printable ASCII,
no control bytes, longer than the extension, ending `.blp`. Every one of the 26
passes it and not one of the leftovers does.
benilla-bourbon (tripleslash/benilla-bourbon, "junk creature-skin records")
drops the fallback outright, on Turtle WoW's `Stag.m2` carrying one junk byte
there. Applied here that removes the skin from all 17 models above — the new
test fails exactly that way, "17 of 17 models with a baked skin path did not get
it" — so this takes the diagnosis and not the patch. The leftovers it names are
real; dropping the whole fallback is what this chain cannot afford.
Tests: a synthetic pair beside the guard for both kinds of name, and a corpus
test gated on `$WOW_DATA` that measures both populations over the winning copies
and asserts a path reaches `texture` while a leftover never does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
d95f26e57f |
A group WMO's MOGP runs to EOF, whatever size it declares
MOGP is a group file's last top-level chunk, so EOF is its real end. The walk
trusted the declared size instead, and `parse_group` answers an empty group for
a payload shorter than the 0x44 header — so a group that declares less than it
holds left MOVT/MOVI/MOPY sitting as top-level siblings *after* the payload and
parsed to no vertices, no indices and no batches. A building that draws nothing,
with no error anywhere to say why.
Seven files in this realm's chain do it, and all seven are the winning copy
(`patch-3.mpq`):
* the six `World\wmo\Outland\Shadowmoon\Slagpit\Shadowmoon_Slagpit01_00{0..5}
.wmo` declare 0x40, four bytes short of the header, over 33 KiB to 546 KiB
of geometry;
* `HumanHouse\WestfallHouse_MediumStraw_000.wmo` declares 0, over 41 KiB.
Measured before and after over the whole corpus: 7 of 7,795 group files parsed
to an empty group, now 0.
`wmo_chunk_overrun.rs` already covered the other direction — Undercity_144
declares one byte *past* EOF — but it only asks for the MOGP header, which
`find_wmo_chunk` clamps and which survives a short declaration too (0x40 still
carries flags, the portal refs and groupLiquid). The geometry is what was lost,
so the new corpus test asks for that: no group file of 1 KiB or more may parse
to zero vertices.
Ported from benilla-bourbon (tripleslash/benilla-bourbon, "a WMO group's MOGP
runs to EOF", credited there to kalistoking), whose `mogp_payload` and unit test
are taken unchanged; the Westfall house and the corpus test are this chain's.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
54fa948cfd |
A BLP level whose span runs past the file ends the chain
`level_data` asked `bytes.get(off..off + sz)` and turned a miss into `Error::OutOfBounds`, which fails the whole decode. Two faults in one line: a level table may name a span past the file, and `off + sz` may overflow — the sizes that do this are wrapped values like `0xFFFFD0C4`, which on a 32-bit `usize` wraps the sum back inside the buffer and hands the caller another level's bytes. Now a deeper level that cannot be sliced ends the chain, as a zero size or offset already does; level 0 still errors, because there is no image without it. The sum is `checked_add`. Measured over this install's chain (138,540 BLP headers, read independently of this decoder): 316 copies declare a level whose span runs past the file. 282 of those levels sit past `max(log2 w, log2 h)`, so the decoder never reads them. 34 do not — Troll facial hair and `item\TextureComponents\` armour art at 128x32 with a wrapped level 6 — and every one of those 34 is shadowed by an intact copy in a higher patch, so nothing on this install decodes differently today. What the fix buys is that the layering is no longer load-bearing: the shadowing is a property of which patches this realm happens to ship, and a level that runs past the file now costs the levels below it rather than the image. Ported from benilla-bourbon (tripleslash/benilla-bourbon, "a BLP mip past the file's end ends the chain"), which measured it on patch-A's `Character\Troll\SCALPUPPERHAIR00_01.blp` — level 6 at the file's end over six intact levels. That file is in this chain too, where `Patch-P.mpq` shadows it with an intact 99,476-byte copy. The test is theirs, unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
8866e151bc |
An encrypted or single-unit MPQ entry reads instead of being refused
The reader refused both as "outside the 1.12.1 envelope". The 1.12.1 part was
right and the envelope was wrong: this realm's chain carries 98 files of both
kinds, and every one of them failed.
Measured over `Data/` (the block tables, read independently of this reader):
* `(listfile)` is `COMPRESS|ENCRYPTED|FIX_KEY` in patch-1, -2, -4, -5 and -A,
and `COMPRESS|ENCRYPTED` in patch-N, which also holds two encrypted
`textures\UnitSelectTextur*.blp`. The listfile union is what
`Chain::list_under` walks, so the macro-icon catalog
(`benilla_formats::load_macro_icon_catalog`, and the ClassicAPI
`GetMacroIcons` built on it) and the archive pass of the addon loader each
lost five archives' worth of names to one `Unsupported`.
* `Patch-Q.mpq` stores all 93 of its files as single units, 92 of them
`ITEM\TextureComponents\` armour art — Tauren battleground sets, the Raid
Shaman robe, the tm_* cloth/leather/plate sets, the alchemy coat, PvP Horde
plate, the Outland belts. Q mounts above every other patch, and
`Chain::read` answers from the winning archive and does not fall through on
an error, so those are the copies the armour composite asks for and the
read failed outright.
Encryption: the key is the name's last component under the FILE_KEY hash,
offset by the block position and XOR'd with the size for a `FIX_KEY` entry. The
offset table decrypts under `key - 1` and sector i under `key + i`, over the
stored (compressed) bytes. A single unit is sector 0, so its key does not step.
`decrypt_bytes` leaves a tail shorter than four bytes as stored, as Storm's
`DecryptMpqBlock` leaves it. The plain path still reads straight out of the one
bulk sector buffer; only an encrypted file pays a per-sector copy.
Single unit: `compressed_size` bytes at `file_pos`, no offset table, a
codec-mask byte first only where the unit actually shrank. `BlockEntry` now
keeps `compressed_size`, which a sectored file still never reads.
Tests: a byte-wise crypto round trip that pins the short tail; an encrypted
stored entry and an encrypted `FIX_KEY` compressed entry, both read by name; a
stored and a deflated single unit, the deflated one longer than a test sector
so nothing can sector it. Three more are corpus-backed and gated on `$WOW_DATA`:
every `(listfile)` in the install reads (asserting at least one of them was
encrypted, so an install without any cannot pass while proving nothing), and
every single-unit file an archive names reads to the size its block entry
promises (92 of them here).
Verified against the live chain: `ITEM\TEXTURECOMPONENTS\HandTexture\
tm_cloth_01_Glove_HA_U.blp` now reads 175,938 bytes from `Patch-Q.mpq` and
decodes as a 512x128 BLP2.
The encryption half is a port of benilla-bourbon's fix (tripleslash/
benilla-bourbon, "MPQ files encrypted under their name read by name"), adapted
to this fork's single-function bulk read — upstream's streaming `open_file`/
`ArchiveFile` is not in this tree. Its nameless-read API (`entries`,
`read_block`, `name_key`) is left out: it exists for that fork's BENPACK
container builder, which this tree has no counterpart to. The single-unit half
is this tree's own; bourbon still refuses those.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
44c0b1269c |
docs: record the upstream sync, the divergence, and what was refused
A record so this is not re-derived. The measurement (349 upstream commits since the 2026-10-12 base, 589 files a full merge conflicts in), why most of it cannot be cherry-picked (upstream replaced mlua with its own Lua 5.0 VM, and ~200 commits are the deferred-to-inline conversion of the app/VM contract), the two resolution traps this sync hit, and all 27 refusals with the reason each. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
89ae4699bb |
Re-seat two picks the #1730 revert and the realm's interface displaced
Two follow-ups to the series, both consequences of the same root cause. **#2150's OptionsFrame.xml hunk, re-ported.** It cherry-picked with no conflict only because the botched #1730 resolution had already replaced this fork's `OptionsFrame.xml` with upstream's, so it applied against upstream's file. The revert then restored ours and took it with it. Ported by hand here: our rows write `self.applyFunc` where upstream's write `this.applyFunc`, and inherit `OptionsCheckboxRowTemplate`/`OptionsDropdownRowTemplate` where upstream's inherit the `Benilla`-prefixed names. Both `TargetofTarget_Update` applyFuncs are gone and the reference note comes along. **#1972's blanket dialog test, scoped to our two dialogs.** The fix — this page's two confirms declaring `timeout`, `whileDead`, `showAlert` — is right and stands. Its test asserted a numeric `timeout` on *every* entry in `StaticPopupDialogs`, which holds for stock 1.12.1 FrameXML but not here: this fork loads the realm's own interface, whose `StaticPopup.lua` leaves `timeout` off 73 of its dialogs. Those are the realm's to declare, and failing on them says nothing about this fix. benilla-app --lib is back to the 269-failure baseline exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
7a8765aa52 |
Revert "The Benilla scroll bar template scrolls its parent itself (#1730)"
This reverts
|
||
|
gromandClaude Opus 5
|
b57f49faa8 |
fmt: blank lines left by the dropped trainer tests
Three tests the #1947 conflict hunk carried in from other upstream commits were dropped; their removal left a triple blank line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
Sam SungandClaude Opus 5
|
a97e063f9c |
A dismount during a pending mount load no longer crashes (#2390)
Dismounting while the mount model was still pending despawned that child, but the same attach pass could still visit it afterwards. Its plain inserts then landed on a despawned entity, and Bevy's default command handler panicked. The attach pass now records the children it drops and skips them for the rest of the pass, so the outcome no longer depends on visit order. The despawns there became `try_despawn`. The player build also installs a command error handler. A failed command now logs a warning naming the command and the entity, instead of ending the session. Every other error still panics, and the tests keep the panicking default, so our own bugs still fail there. Tests: a dropped pending child is skipped whatever the visit order, for both the dismount and the stale display; the handler logs a failed command and still panics on other errors. Fixes #2377 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit ce5503ca538dcd98424efe136d12cdfe7259643a) # Conflicts: # crates/benilla-app/src/entities/attach/mod.rs # crates/benilla-app/src/lib.rs Absorbed into the octo fork. Merged surgically rather than taken: upstream's hunk also restores the 'a player attaches once its equipment settles' gate, which this fork deliberately removed — gating on Equipment::settled withheld a body for good on any chain that never answers a visible-item query, which in a battleground crowd is every other player. Our gate and comment stand; what is absorbed is the crash fix itself (the dropped-children list, the skip at the top of the loop, try_despawn) plus the command error handler in lib.rs. The arrival/shown_rebuild query legs belong to other upstream commits and are left out. attach 28 green, crash 2 green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
Sam SungandClaude Opus 5
|
91bcc19ac0 |
Test builds open no audio device (#2388)
Every headless test app built `GamePlugins`, whose `SoundPlugin` opened the default output device unless `$WOW_NOSOUND` or `$WOW_CAPTURE` was set, and no test run sets either. Under nextest that is one CoreAudio client and speaker stream per test process. With a few runs going at once, `coreaudiod` sat near 300% CPU, every sound on the machine crackled, and each test blocked on the audio server's connect. A test build now takes the existing silent path, so its mixer is `None`, as with no device. Test: `a_test_client_opens_no_audio_device`. With the change undone it fails on a machine with a speaker, taking 96 s; with it, the test passes in 0.06 s. Fixes #2380 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit fbc90b9bbf272a784f3703eb0c68ea466fe90951) # Conflicts: # crates/benilla-app/src/game_plugins.rs # docs/DEVIATIONS.md Absorbed into the octo fork. The sound/mod.rs fix applies unchanged. Of the tests the conflict hunk carried, only this commit's own a_test_client_opens_no_audio_device is kept, retargeted from upstream's headless_client_unrun to our headless_client (same contract: every plugin's build runs, no schedule does); resource_writers and the_glue_preview_has_one_feeder come from other upstream commits and need glue_scene and GluePreview, which this fork has not got. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
andi79h-claude
|
73f4bf808d |
The target-of-target rows only write their globals, as 1.12's options do (#2150)
1.12's options declare Show Target of Target as a bare uvar (UIOptionsFrame.lua:65) and the mode is read on save (UIOptionsFrame.lua:365); neither calls TargetofTarget_Update. The frame follows from TargetFrame_OnUpdate and TargetofTarget_OnUpdate on the next frame (TargetFrame.lua:492-501). Our rows called it as their applyFunc, with the row as `this`; the function refreshes `this`'s debuffs (RefreshBuffs(this, 0, "targettarget"), TargetFrame.lua:538), so with a target's target up the switch raised "BuffFrame.lua:281: attempt to index local 'debuffBorder' (a nil value)" (seen on a Linux client). Verified: the_target_of_target_switch_leaves_the_frame_to_its_own_updates (with a target's target up: no error, the frame shows after one tick on and hides after one tick off) fails with the applyFunc put back, with exactly that error, and passes without it; the options and tot_frame tests pass on a deDE install; cargo test -p benilla-app --lib 5145, clippy -D warnings, fmt. Built on macOS only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 4d6f9ecc8a0d99e6d3ecf7fa4a8da229ebad92c2) |
||
|
andi79h-claude
|
2b1e285101 |
The combat log fills deDE's numbered holes, so a German client prints its fights (#1994)
- The combat log fills deDE's numbered holes, so a German client prints its fights - The fill's comment cites the client's ordered printf Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Sam <samuv.eth@gmail.com> (cherry picked from commit b14793c294f6f33ad1c3491c99bf973011d21d28) |
||
|
Sam Sung
|
8f658cb516 |
Key-binding confirm dialogs no longer raise a Lua error each frame (#1972)
Unticking Character Specific Key Bindings, or pressing Defaults, raised `StaticPopup.lua:1713: attempt to compare number with nil` every frame the confirm stayed up. 1.12.1: `StaticPopup_Show` copies an entry's `timeout` into `dialog.timeleft`, and `StaticPopup_OnUpdate` compares it with 0 each frame; every stock entry declares one, the stock key-binding confirm included (`Blizzard_BindingUI.lua:10-24`, `timeout = 0`, `whileDead = 1`, `showAlert = 1`). Our key-bindings page's two confirms declared none. Both now do, the character-set one with the stock entry's three fields. The checkbox test now runs a frame with the confirm up, and a new test fails on any dialog the loaded interface defines without a numeric timeout; both fail without the fix. Fixes #1971 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 881152bda6d48cc3fd1a47679a7465add212ae05) |
||
|
Sam SungandClaude Opus 5
|
f0adc9a3ba |
GetTrainerServiceInfo answers "" for a rankless service's subText (#1947)
`GetTrainerServiceInfo` answered nil for a rankless service's subText. In 1.12.1 the subText is the Spell record's NameSubtext (`0x4d8b50`, read at `0x4d8b84`), the empty string for a spell with no rank; only a service with no Spell record answers nil (`0x4d8b58`). The resolver now keeps an empty rank as `""` and the binding pushes None as nil. Tests cover both rows and the install-backed Apprentice Blacksmith trainer. Supersedes #1935. Co-authored-by: kalistoking <148718830+kalistoking@users.noreply.github.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit fdbb4bb070382a6cf6b30bdaf33214c972cd02f4) # Conflicts: # crates/benilla-ui/src/script/trainer/tests.rs # docs/DEVIATIONS.md Absorbed into the octo fork. Mostly already true here: this fork's GetTrainerServiceInfo already answered "" for a rankless service, with its own note on the stock window's unguarded format. What is kept is upstream's test for it and the field doc. Two tests the conflict hunk dragged in from other upstream commits are dropped: a_service_row_answers_one_for_its_fourth_value and service_description_is_nil_without_text need struct shapes this fork lacks (an Option description). A third, a_service_without_a_spell_record_answers_nil_for_its_subtext, is dropped because it contradicts our deliberate choice — we answer "" for a missing Spell record too, and that is now recorded as a Deviation on the field rather than left as an undocumented difference. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
andi79h-claudeandClaude Opus 5
|
5fa31a073b |
IsRaidOfficer answers 1 for an assistant or the leader, as 1.12.1 does (#1894)
It was a nil stub, so an assistant got no raid target icons in the unit popup (UnitPopup.lua offers RAID_TARGET_ICON to the leader or to IsRaidOfficer) and the raid frame's add-member button stayed off for them. 0x4bb910 walks the raid roster (0xb712a8, count [0xb713e0]) for the row whose guid is the local player's and pushes 1 when that row's rank ([row+0xc], GetRaidRosterInfo's return 2) is above 0, so for an assistant and for the leader; outside a raid, without a player or without our row it answers nil. Verified: cargo test -p benilla-ui --lib (is_raid_officer_reads_our_own_roster_rank), clippy -D warnings. Built on macOS only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit ed630ffb784a4038a0aee343fb909d35147aa388) # Conflicts: # crates/benilla-ui/src/script/party.rs Absorbed into the octo fork. Adjacency only: our SuperWoW SetRaidTarget flag test sits immediately before the test upstream retitled, so both are kept. The binding change applies unchanged; party:: tests green (34). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
0dcaca03df |
fixup: #1809's ClientState path is char_select in this fork
`Login notices show after the loading screen (#1809)` cherry-picked with no conflict and still did not build: it refers to `crate::client_state::ClientState`, the module upstream renamed `char_select` to in a commit this fork has not taken. A clean apply is not a compiling one — the 3-way merge has no way to know a path moved on the other side. One line, no behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
Sam SungandClaude Opus 5
|
2c4458ee1d |
A jump on a moving lift measures its fall in the lift's frame (#1818)
A jump on a descending lift (Undercity, Gnomeregan) played the hard-landing grunt and dust. While airborne, benilla carried the rider with the deck but kept the jump's start height in world coordinates. So the lift's drop during the arc, about 24 yd on Undercity's 29 yd/s cars, counted as a fall, and FALLINGFAR latched early. 1.12.1 measures the fall height (`0x7c60c0`) in the mover's own frame: transport-local while aboard, re-mapped when the mover boards or leaves (`0x7c6620`). `Player::carry_to` now moves the start height with the deck. A remote mover's arc is measured in the deck's frame when it takes off and lands on the same transport. Tests: a 60 fps jump on a deck dropping 28.95 yd/s lands with no fall height and no FALLINGFAR, for us (23.64 yd before) and through packets for a remote (23.88 yd before). Fixes #1250 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 560d7ff63e8394ca08c97a0867c852155f3e6b52) # Conflicts: # crates/benilla-app/src/spell/range_units.rs # docs/DEVIATIONS.md Absorbed into the octo fork. spell/range_units.rs is upstream-only (added after our base) and its change here was a test fixture's fall_start_z -> fall_start rename, so the file is dropped rather than orphaned; the rename's real call sites applied unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
Sam SungandClaude Opus 5
|
b69ea5cd04 |
Sound on Linux skips ALSA's null output when the default fails (#1810)
#1808's stand-in search ranked ALSA devices only when the host's name read "ALSA", but cpal names it "Alsa". So the ranking never ran, and the first device ALSA lists won. On the reported machine that was `null`, which discards every sample, so sound stayed silent behind a warning that it was playing. The check now compares the host's `HostId::Alsa` variant, so the ranking applies: `pipewire`, then `pulse`, then each card's own outputs, and never `null`. The Linux crosscheck compiles it. The ranking's tests are #1808's. Fixes #1806 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 8a6c74036cfe3d00275fd75ad1aeae1f4ea7d0df) # Conflicts: # docs/DEVIATIONS.md Absorbed into the octo fork. Generated docs reconciled: MAP.md stays ours, DEVIATIONS.md stays absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
Sam Sung
|
0fde3c281f |
Login notices show after the loading screen, as in 1.12.1 (#1809)
The red "GM mode is ON" line did not show on a session's first login, only on a relog. In 1.12.1 the interface and the world load before `CMSG_PLAYER_LOGIN` goes out (`0x46c272`), and the loading screen drops when the player is created (`0x5debbd`). vmangos sends that create ahead of the login notices (`Handlers/CharacterHandler.cpp:675`), so the line is added after the reveal and gets its full time on screen. benilla's cover stays up through its own warm-up, about 6 s on a cold login, and the line faded underneath it. A relog is warm, so the line survived. Server notice lines (`SMSG_NOTIFICATION`, area-trigger messages) now wait while the cover is up and fire on the frame it drops. Test: a notice under the cover fires once, at the reveal. A live cold login shows the line the frame after the cover clears. Fixes #1252 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 86c71919284d51473602a5ee668b5f818d71d980) |
||
|
Sam SungandClaude Opus 5
|
440d20fd60 |
Sound plays on another output when ALSA's default will not open (#1808)
On Linux, sound stayed silent for the whole session when ALSA's `default` would not open. On the reported machine, PipeWire holds the card and `default` is left as the stock `dmix`, which cannot open it. benilla opened only the host default and gave up when that failed. Now, when the default will not open, the first other output that does stands in: `pipewire`, then `pulse`, then each card's `sysdefault` and `plughw`, with HDMI cards last. It never picks `null` or the rate and mix plugins. The device watch now compares against the default the device was chosen against, so a stand-in is not torn down every poll. A `Deviation:` line records that 1.12.1 runs silent when FMOD's driver 0 fails (`0x7a445f`). Tests: the ranking over three `aplay -L` lists, including the reported one. The Linux crosscheck is green. Fixes #1806 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 1e28153ae23dd35d579c5ef45e5aeac1622c99ca) # Conflicts: # docs/DEVIATIONS.md Absorbed into the octo fork. Generated docs reconciled: MAP.md stays ours, DEVIATIONS.md stays absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
Sam SungandClaude Opus 5
|
0c247396b7 |
The ESC menu's Support rung opens the help window (#1807)
The ESC menu's Support rung was greyed with nothing behind it. The 1.15 era menu that the layer's ladder follows opens the help window from Support. In 1.12 that window is the stock `HelpFrame`, opened by the "?" micro button's `ToggleHelpFrame()` (`MainMenuBarMicroButtons.xml:217`). Support is now live: it plays the menu click, closes the menu as the Macros rung does, and toggles the help window. Edit Mode stays greyed. Test: `support_closes_the_menu_and_toggles_the_help_window` clicks the rung and checks the toggle and the closed menu; the live-entries test now lists Support. Fixes #1800 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit bb434a181f6a3815ec6fe3e5ace89c864fa1808c) Absorbed into the octo fork. Ported by hand: upstream wires Support in GameMenuAdapters.xml, a Lua adapter layered over stock FrameXML that this fork does not carry — our menu declares its own rungs in GameMenuFrame.xml, where GameMenuButtonSupport was GameMenuButton_Pending (SetText + Disable). It now sets its text on load and, on click, plays igMainMenuOption, hides the menu as the Macros rung does and calls ToggleHelpFrame(). Our rungs carry no Benilla prefix, so upstream's test names are retargeted; our own narrowing_the_frame_keeps_the_button_column_centred test is kept. The 6 FloatingChatFrame failures in this module are pre-existing baseline (10 passed/6 failed before, 11/6 after). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
Sam SungandClaude Opus 5
|
f83533b2a6 |
Linux audio plays: cpal 0.17.3 fixes ALSA's stalled callback (#1785)
On some Linux setups the game was silent. The device opened, but its data callback ran once and stopped, with no error. The music position never moved. This is cpal 0.17.1's ALSA bug (RustAudio/cpal#1099, "Data callback called only once on ALSA"), fixed in 0.17.3 by setting playback's `avail_min` to one period. benilla's stream starts only at the start threshold, which is exactly that case. cpal goes to 0.17.3. alsa 0.11 and bevy_audio's unbuilt cpal 0.15 cannot share a lock (`links = "alsa"`). A lock-only stand-in for alsa 0.9 settles that, documented in `third_party/alsa-0.9`. The realtime-fallback log line now says what happens on each platform. Following the default device gets its `Deviation:` line: 1.12.1 initialises sound once (`0x7a4330`). Built on macOS, with Linux and Windows linted. Fixes #820 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit d93c231d8ebb27934791b726a0575f4c9b709446) # Conflicts: # Cargo.toml # crates/benilla-app/examples/extended_launcher.rs # docs/DEVIATIONS.md Absorbed into the octo fork. Cargo.toml unions both patch entries (our mlua-sys traceback patch and upstream's alsa 0.9 lock-only stand-in). Cargo.lock is regenerated locally rather than taken from upstream, with cargo update -p cpal@0.17.1 --precise 0.17.3: cargo reaches the same resolution (alsa 0.9.1 from the stand-in, alsa 0.11 + alsa-sys 0.4 for the built cpal, windows-* dropped), so it is our own lock, resolved here. examples/extended_launcher.rs is upstream-only and stays absent. cargo check -p benilla-app clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
Sam SungandClaude Opus 5
|
e0895013ed |
The Benilla scroll bar template scrolls its parent itself (#1730)
`BenillaScrollBarTemplate`'s own OnValueChanged was a faux-list handler that both instances replaced with the real scroll, so a new bar that forgot the override wrote `offset`/`updateFunc` on its parent instead of scrolling. The reference's `UIPanelScrollBarTemplate` carries `this:GetParent():SetVerticalScroll(arg1)` itself (`UIPanelTemplates.xml:156-158`). The template now calls `BenillaScrollBar_OnValueChanged(this, arg1)`; the two instance overrides, the dead faux handler and its header text are gone. Test: a bare bar from the template scrolls its ScrollFrame parent with no `offset` or `updateFunc` write; it fails with the old template. Fixes #359 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 142ed718d3a360a5992c78b53540a356cacf6d1e) # Conflicts: # crates/benilla-app/assets/ui/OptionsFrame.xml # crates/benilla-app/assets/ui/ScriptLogFrame.xml # crates/benilla-app/assets/ui/ScrollTemplates.xml # docs/DEVIATIONS.md Absorbed into the octo fork. Upstream's side taken on all three XML files, and it fixes a live fault in this fork as well: our BenillaScrollBarTemplate called BenillaFauxScrollBar_OnValueChanged, a global defined nowhere in the repo, so every bar built from the template raised on value change — survived only because OptionsFrame and ScriptLogFrame each overrode the handler per instance. The template now carries the real handler and the two overrides are gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
Sam SungandClaude Opus 5
|
132776c197 |
cameraYawSmoothSpeed follows at the stored rate, not the slider's 90-270 (#1712)
`cameraYawSmoothSpeed` was clamped to 90-270 when applied, the Auto-Follow Speed slider's range, so `/console cameraYawSmoothSpeed 360` followed at 270. In 1.12.1 its validator `0x50c000` refuses only outside [0.1, 360] (`0x50b330`, `Value out of range` at `0x50b36c`) and the follow reader takes the stored rate as is (`[0xbe1070]` at `0x512d75`). The camera now uses the stored value; the registry's existing check, which already matched `0x50b330` and also runs on the config load, does the refusing. `FOLLOW_SPEED_RANGE` is gone. Test: 270, 360, 0.2 and 30 are stored and applied as given; 360.5, 400, 9000, 0, 0.05 and -30 are vetoed and keep 30. It fails with the clamp put back. Fixes #323 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 9e1b3e9ae9c7fd488ea0addd3f11460678cd61fc) # Conflicts: # crates/benilla-app/src/cvars/mod.rs # crates/benilla-app/src/cvars/table.rs Absorbed into the octo fork. Ported by hand: our cvars registry is one file, not upstream's cvars/ split, and it has no .checked() validators or SetOutcome::Vetoed, so the clamp cannot simply be removed — without a veto channel that would turn a clamp into an unvalidated accept. FOLLOW_SPEED_RANGE is widened from the slider's 90..=270 to the reference validator's 0.1..=360 instead, so /console cameraYawSmoothSpeed 360 now follows at 360; refusing (rather than clamping) outside that window is recorded as a deviation on the constant. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
Sam SungandClaude Opus 5
|
694ef5832d |
A boot with no readable WoW install is fatal, with an error dialog (#1630)
With no WoW install found, or a `Data` folder whose archives will not open, benilla booted into a window of textless buttons, since fonts, art and the stock interface all come from the install. It now ends before any window exists. A native error dialog (`rfd`: AppKit, Win32, `zenity` on Linux) names every folder searched, or the folder and the archive error, and says where to put the install; then it exits 1. An unattended run prints the same text to stderr and exits without a dialog. 1.12.1 ends at a fatal dialog for a present but corrupt base archive (`0x403b6f`) and only logs a missing one (`0x403b00`); ending at no install at all is recorded as a `Deviation:`. Tests cover both messages and the failing exit; both cases were run live unattended. Built on macOS, Linux and Windows. Fixes #836 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 6f6531078333fcd0c0c1bcbbd8e61ce531cdf607) # Conflicts: # docs/DEVIATIONS.md # docs/MAP.md Absorbed into the octo fork. Generated docs reconciled: MAP.md stays ours, DEVIATIONS.md stays absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
badmagick329
|
4c98a8d981 |
GetTradeSkillNumMade answers 1, 1 on a header or past the rows (#1459)
`GetTradeSkillNumMade` answered 0, 0 for a header row or an index past the rows. 1.12.1's `GetTradeSkillNumMade` (`0x4fdc50`) sends a bad index, a header (spell -1, `0x4fdcaa`) and a missing spell record to the fallback at `0x4fdd13`, which pushes 1 twice. A recipe with no spell record never becomes a row in benilla, so the past-the-rows case covers it. The miss now answers 1, 1. Tests: the header, no-window and past-the-rows cases pin 1, 1. Fixes #1262 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 129b3bb852b2f328de529f3c698595c14baf5fa5) |
||
|
Sam SungandClaude Opus 5
|
8e0a0155d6 |
A fifth loot roll waits for a frame and opens when one closes (#1458)
A fifth open loot roll was announced at once. The stock frames hold four, so its frame never opened and it could not be voted. 1.12.1 starts a roll through `0x61b430`: below four open it counts up and fires `START_LOOT_ROLL` with the wire countdown. Otherwise it defers the roll. Every close (own vote, WON, ALL_PASSED) fires `CANCEL_LOOT_ROLL` and then promotes the oldest deferred roll (`0x61b760`). An uncached item takes its turn when its record arrives (`0x61b460`). benilla now does the same. Each roll waits, defers, starts or is open; at most four are open, and every close promotes oldest first. The client's server-close paths decrement the count even for a deferred roll that never counted (`0x61b6f8`, `0x61bc17`), so a fifth frame can open there. benilla keeps the count exact, as a `Deviation:`. Tests: five announce four; each close path promotes; the wire countdown; a deferred close. Fixes #1061 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit a9c2672b3fb7442975988a6fb527b6771196e84d) # Conflicts: # crates/benilla-app/src/ui_loot_roll.rs # docs/DEVIATIONS.md # docs/MAP.md Absorbed into the octo fork. vote() takes both sides: upstream's promote() alongside the fork's cancelled-roll log. MAP.md stays ours (generated, fork superset); DEVIATIONS.md stays absent (we do not carry its generator). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
Sam Sung
|
454555dfb0 |
Quest givers streamed in before our own tag are still asked their status (#1455)
After login, quest givers showed no mark and could not be talked to until a quest update or a GM toggle. 1.12.1 asks a giver's status at every create (`0x5fba72`, `0x607380`) and when a released object comes back (`0x5fbe2f`). The right-click takes the quest service only with a cached status (`0x5df490`), so the missing mark and the failed interaction are one defect. `query_statuses` ran before our own entity was tagged `SelfPlayer`, returned early and used up every change, so no giver was ever asked. A relog brought givers back under stale guid keys. Now: - the ask key lives on each creature (`AskKey`); - a new self resets it; - the markers run after `WorldStage::Net`. Test: givers spawned a frame before the tag are asked on entry and on relog. Live: 0 queries before; 11 asked and answered on each login after, and on a teleport and back. Fixes #918 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit d1fa9bdedfa83a8c415c2c382cf2ec5736b68fcd) |
||
|
Chris
|
9706184635 |
The client dials the first IPv4 address alone, as 1.12.1 does (#1385)
**Problem.** `localhost` resolves to `::1` first. benilla dialled every resolved address in order, so a realmd bound only to IPv4 left a Windows login waiting about 2 s on `::1`. **Reference.** 1.12.1 resolves the realmlist host to one IPv4 address and dials it alone (`WowConnection::ConnectToHost` `0x5b6e40`): `inet_addr` (WSOCK32 ordinal 10), else the first entry of `gethostbyname` (ordinal 52). It never dials IPv6. **Change.** The dial takes the resolver's first IPv4 address and nothing else. A host with no IPv4 address fails as unresolved. **Tests.** `localhost` dials `127.0.0.1`. The first IPv4 address of several is dialled. A host that resolves only to IPv6 gives no target. Smoke: green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Sam <samuv.eth@gmail.com> (cherry picked from commit 8c3587479059423dc69e67a345fc933a9ba40b17) |
||
|
Sam SungandClaude Opus 5
|
b42cd7ef6b |
Update packets: self release, mover blocks, spline facing, placement (#1324)
Five fixes to how `SMSG_UPDATE_OBJECT` is applied, each against the client's code: - An out-of-range block never releases the active player (0x465f39–0x465f45). - A movement block for the unit we move, the player or a possessed unit, is dropped (0x4670a5–0x4670bb). - A create spline reads its three final-facing fields independently, in the order point, GUID, angle (0x47f0bf, 0x47f0e8, 0x47f0f9), so a server that sets two no longer breaks the packet. - A game object is placed by its create block's position over its position fields (0x5f7046), and a re-create re-poses units only (0x46638c). - The leave-world sweep's comment names the destroy path alone. Byte-exact fixtures for the spline; each change has a test that fails without it. Fixes #1307 Fixes #1308 Fixes #1309 Fixes #1310 Fixes #1311 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> (cherry picked from commit b816ff2f6054f496b7fb169a8acbd430323a98d3) # Conflicts: # crates/benilla-app/src/ui_script/sweep.rs Absorbed into the octo fork. Our tree has no ui_script/sweep.rs (upstream added it after our last sync), and the only change to it here was a comment; the file is dropped rather than orphaned. The five packet fixes apply unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
44aac335fd |
classicapi: the last two with data behind them — item sets and area triggers
Both over DBCs 1.12 reads C-side and publishes to Lua nowhere, so both
are facts no addon on this realm could reach.
`C_Item.GetItemSetInfo`: the tooltip prints a set block and `GetItemInfo`
reports no set at all, so nothing can tell which pieces complete one. The
table's shape is its caller's own documentation (`SuperCleveRoidMacros/
ClassicAPI.lua:171-174`) — including the bonus pair being
`{ spellID, threshold }`, which is the REVERSE of the DBC's column order,
so the swap happens once in the host and not in every reader.
`C_Map.GetAreaTriggerInfo`: pfQuest's own comment says it reads trigger
geometry from this "instead of a shipped coordinate table", and the three
fields it takes — areaID, mapX, mapY — go straight into `pfMap:AddNode`,
whose coordinates are 0-100. So the percentages are the consumer's
measured shape rather than a guess. The zone resolution is the host's,
through the same `zone_uv` law the map window's own hover uses
(`WorldMapUiData::zone_at`), so a trigger lands in the zone the map would
name for that point — quirk included: a point exactly on a rect's east or
south edge reads as outside, which is that law's own `(0, 0)`. The three
map fields are absent TOGETHER when the centre resolves to no zone (an
instance interior), which is exactly the case pfQuest guards for.
Both ride host probes rather than pushed tables: the resolution needs the
`WorldMapArea` rects, which do not cross the seam, and neither catalog is
something to copy per VM for a verb nothing calls at boot. Two catalogs
gained an `iter()` for it, each said to be for this.
That leaves four of the original 56. Two have no caller in the corpus;
`C_FriendList.SetFriendNotes` has no note field on the 1.12 wire at all,
so a note typed and lost at logout would be worse than a button that does
nothing; and `C_BattleNet.SendGameData` stays deliberately absent, because
every caller guards it and registering it would claim a BattleNet this
client does not have.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
546e60765d |
classicapi: fourteen members over four namespaces — the measured tail down to six
Of the 153 `C_<Namespace>.<Member>` names grom's addons reference, the fresh VM was missing 56; this takes it to 6. Fourteen members here, each over data or a stock verb that already existed: `C_Spell` gains the four melee-timing predicates and `IsSpellInRange`. 1.12 answers all of those only about an ACTION BAR SLOT — `IsAttackAction` and `IsAutoRepeatAction` — so a spell the player knows but has not placed on a bar cannot be asked at all, which is the gap a condition engine keeps running into. The three attribute answers ride named booleans on the spell lookup rather than a raw attribute word, so the bits and their reference citations stay in `benilla-formats` where they can be checked; `do_not_reset_combat_timers` is named there for the first time, beside the `ranged_speed_cooldown` that already tested the same bit. The melee auto-attack is an ID (6603) rather than an attribute, because no bit marks it — the ranged half has the auto-repeat flag and the melee half is that spell. `C_SpellBook.GetSkillLineName` is the most-read missing member of the whole family (nine call sites) and is one delegation to `GetSpellTabInfo`. `IsAutoAttackSpellBookItem` resolves by id against the same 6603, so the two cannot disagree about what auto-attack is. `C_Item.GetCurrentItemLevel` answers a number 1.12 prints in a tooltip and returns from nothing, so no addon can compare two pieces of gear. `RequestLoadItemDataByID` is the modern async load over the ask-once queue `ItemMixin:ContinueOnItemLoad` already drives. `GetWeaponEnchantInfo` is a pass-through. `C_QuestLog`'s abandon pair delegates to the stock two-step, so the mark, the still-in-the-log search and the send keep one owner — a namespaced abandon cannot drop a quest the stock path would have refused. The six left are each written down with what they need. Two are a host probe over a DBC `benilla-formats` already parses (`AreaTrigger` for pfQuest's trigger geometry, `ItemSet`), two have no caller in the corpus, `SetFriendNotes` has no note field on the 1.12 wire at all — a note typed and lost at logout is worse than a button that does nothing — and `C_BattleNet.SendGameData` stays deliberately absent, because every caller guards it and registering it would claim a BattleNet we do not have. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
7bfdefb206 |
classicapi: C_EncodingUtil for real, and two namespaces 1.12 has no subsystem for
`C_EncodingUtil` is the one namespace in this family that is pure algorithm, which is why it is built properly rather than stubbed: all eight members, in the REAL formats. pfUI's profile sharing is the reader and its own header names the pipeline — SerializeCBOR → CompressString (zlib) → EncodeBase64, prefixed `!pf1!` — with the export path NOT inside a pcall, so clicking Export raised, and the import path inside one, so it failed silently. A profile string is pasted between players, so a blob this client writes has to be readable by the real one and the other way round. So: zlib through flate2 (already the workspace's inflate for MPQ sectors and its compressor for CMSG_AUTH_SESSION's addon block, so this adds a crate to one more manifest and not to the build), RFC 4648 base64, RFC 8949 CBOR, RFC 8259 JSON. Nothing invented here — the trap a hand-rolled codec falls into is round-tripping with ITSELF while being wrong. The decoders are written for hostile input, because a profile string arrives pasted from a forum: every length is checked against what is actually left, nesting is capped, the inflate output is capped at 16 MiB, and a malformed blob answers nil rather than raising or allocating what a crafted header asked for. Tests run the RFCs' own vectors in both directions — including the half-float and tag forms we never write but a real encoder may — a truncated/overlong/deep-nested set, and pfUI's whole export/import pipeline end to end. `C_EquipmentSet` and `C_LootHistory` are subsystems 1.12 does not have at all, and both answer an honest nothing rather than being absent, because pfUI builds both windows and reaches one member of each OUTSIDE its own gate (the equipment manager's `CanUseEquipmentSets` test is one line in one addon; the loot window's Clear button has no test at all). What each would take is written down rather than guessed at. Equipment sets are persistent state 1.12's wire has no concept of — a feature to decide on, not a gap to close, and pfUI already supplies the UI. The loot log is closer than it looks: both packets already carry what the getters answer and `ui_loot_roll.rs` handles them, so it is a bounded list plus three events — but the getters' tuple order is the one thing here that IS a guess, read out of pfUI's own destructures, and building the log would make that guess load-bearing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
65609f6db4 |
classicapi: C_ChatInfo and C_LossOfControl — two namespaces that were raising
Measured the gap rather than trusting the stale sweep in the doc: of 153 `C_<Namespace>.<Member>` names grom's addons reference, 56 are missing — 24 where the namespace exists and the member does not, and **32 where the whole namespace is absent**, which is the dangerous half. An absent namespace is not a missing feature: `C_Foo.Bar(...)` raises. Two of those are indexed with no guard at all: - AceComm-3.0's ChatThrottleLib takes `_G.C_ChatInfo.SendAddonMessage` out of the table before calling it (`:638`, and `:653` for the logged twin), so every addon-message send through the throttler raised. Two of its other reads DO guard, which is what made it easy to miss: the library tests for the namespace in one place and assumes it in another. AI_VoiceOver's comms ride this. - `SuperCleveRoidMacros/ClassicAPI.lua:237` reads the loss-of-control count with `or 0` — which guards the result, not the namespace. Its own comment four lines up says "Returns 0 for a client without C_LossOfControl", which is what the author intended and not what the code does. `API.GetSchoolLockout()` is reached from the conditionals. `C_ChatInfo` delegates to the stock globals, so the distribution whitelist and the usage errors keep one owner. It adds exactly one behaviour: **a lane 1.12 does not have is dropped, not raised.** AceComm's whisper lane calls it with "WHISPER", the stock binding answers the reference's own `Unknown addon chat type` raise, and a raise inside the throttler takes down the queue carrying the message. Of raise / misdeliver to the party / drop, dropping is the only one that neither stops the caller nor sends addon data to an audience that did not ask for it. Which lanes exist is asked of the binding's own table, not copied. `C_LossOfControl` answers an empty list, which is the answer its caller was written to handle. What a real one needs is written down: the only member with content on this wire is the school interrupt, and a lockout arrives as ordinary cooldowns on every spell of the locked school — so recognising one means deciding a batch is a lockout rather than a coincidence, and a wrong guess makes those conditionals refuse to cast. Also the seven missing `C_AddOns` members, all delegations. The enable state takes its arguments in the namespaced order, measured rather than assumed: AI_VoiceOver aliases the two forms and writes the swap out at its own call site. It answers 2 or 0 and never 1, because 1.12 keeps one enable bit per addon with no per-character dimension to be partial over. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
38a6b6f149 |
unitxp: inSight and behind — four macro conditionals that silently never worked
Not a gap: a defect. Defining `UnitXP` at all is what makes
SuperCleveRoidMacros believe the whole family is present —
`hasUnitXP = pcall(UnitXP, "nop", "nop")` (`Init.lua:274`), which
succeeds here because the dispatcher's fall-through does not raise — so
four macro conditionals have been running against commands this client
did not implement: `[insight]`, `[noinsight]`, `[behind]`, `[nobehind]`,
each with a count mode too.
The fall-through answers the stock `UnitXP(unit)` getter's 0, and every
one of those conditionals compares against `true` LITERALLY (`== true`,
`~= true`). So `[insight]` and `[behind]` could never fire while
`[noinsight]` and `[nobehind]` always did, with nothing to notice it by.
That is why both commands answer a boolean rather than 1.12's `1`/nil
flag shape, and why a pair they cannot measure answers nil rather than a
fabricated `false`: the two directions are read separately, so "cannot
say" has to be distinguishable from "no".
`inSight`'s blocker is gone, and it was gone before this: the row said
"needs a real raycast with no current binding-layer access", and the
nameplate-occlusion work built exactly that raycast. This is the same
body-layer ray from the player's chest instead of the camera's eye, fed
into the per-token `UnitReach` record `distanceBetween` already reads —
only the app can trace the world, which is the whole reason the command
sat blocked in the binding.
`behind` this file called *unconfirmed* ("treat as unconfirmed until seen
elsewhere"). It is now seen elsewhere: ten call sites, with the argument
order documented by its own readers, so `behind(A, B)` asks whether A
stands in B's rear arc. The arc is the rear HEMISPHERE, which is what
this realm's server means by behind — vmangos tests `!HasInArc(M_PI,
attacker)`, the same rule that accepts or refuses Backstab — taken flat,
so a ledge above a mob's back counts and directly overhead does not.
Two things my own tests caught, both worth keeping:
- A two-argument `inSight` answered `true`. These are recognised
commands, so they now match on the NAME alone and say nil when the
arguments make no pair; the arity fall-through is right for an
unrecognised command and wrong here, because neither name is a unit
token so the stock getter can only answer a meaningless 0 — which is
truthy in Lua, i.e. a confident "yes, in sight".
- A `#[cfg(test)]` module placed mid-file hid every `fire_event` after it
from `event_shape_gate`, which truncates each file at the first one:
six unit-field bridge events read as unproduced. The test moved to the
file's own tests module, where the one-test-module-per-file convention
that gate assumes puts it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
1e9ef80c41 |
superwow: pin the bubble's seat, the one placement that could be silently wrong
The widget tests proved the frame's size, its alpha and its text; none of them would have noticed an anchor on the wrong point, which would still size and draw — just in the wrong place. The bubble hangs from its bottom centre on the projected point and grows UPWARD (`0x4b0c30`), so the test now reads `GetCenter`, `GetBottom` and `GetTop` back and pins all three. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
434d514879 |
superwow: the chat bubbles are widgets — CREATE_CHATBUBBLE and pfUI's skin
The last real SuperWoW feature, and one piece of work closed three things: `CREATE_CHATBUBBLE` (which could not honestly be fired, because its payload can only be the bubble), `C_ChatBubbles.GetAllChatBubbles` (which answered an empty list, truthfully, for the same reason) and pfUI's whole bubble skin, which walks that list. `benilla-ui`'s `script/chat_bubble.rs` is a pool of real frames — anonymous WorldFrame children with a backdrop (`0x4b0a35` IS `SetBackdrop`), a tail texture and a FontString, which is what the reference's own `CGChatBubbleFrame` is and how addons of the era found bubbles before SuperWoW published the event. The app keeps everything a bubble's existence depends on — the spawn gate, the lifetime, the fade, the range re-test, the stacking sort and the text measurement that sizes the frame — and hands it over as `BubbleState`: the same split `vplates.rs` already makes for the plates, so the engine decides what is on screen, the widget layer decides what Lua sees, and neither can disagree about a bubble because there is one feed. Three things improved by the move rather than being preserved by it: - The bubbles sit at frame levels 2, 3, 4… over the V-plates at level 1, which is the reference's own order (`0x4b12d5`-`0x4b1312`). The quad band had them UNDERNEATH — a deviation `overlay_z` carried in a comment. That band is gone and the lane is down to one tenant. - The art is loaded, tiled and bleed-inset by the UI pass like any other frame's, so this file drops its own texture resource and its own half-texel inset helper. - The system moved BEFORE the paint. It sat in `UiQuadAppend`, the window *after* the extract: right for an appender, a frame late for a feed. The test that matters transcribes pfUI's own `ScanBubbles` loop — it finds the FontString with the line and its chat-type colour, blanks the one Texture region and parents its own frame to the bubble — and then checks the EXTRACT for drawn quads: both backdrop halves, the tail, and the text at the em the app measured the box with. A test that only checked the widgets would have passed with nothing on screen. Two architectural gates caught this and both were answered rather than silenced. The one-shot gate: `drive_bubbles` now holds the VM and drains a queue, so it is EXEMPT with the argument — the queue's product is app-side state, the widget sync is idempotent and binds on the first frame there is a WorldFrame, and gating the system instead would pile lines up and spawn them all at once with fresh lifetimes. The VM wall: its ceiling rises by one with the reason, which is that this file is `vplates.rs`'s exact twin. `CREATE_CHATBUBBLE` fires per APPEARANCE, not per pool slot ever built — the opposite of `NAME_PLATE_CREATED`'s rule, because a recycled frame has been re-anchored, re-sized and re-texted since it was last seen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
bead2cb22b |
superwow: Clickthrough is live — the last function on the DLL's manifest
It had been stored and read by nothing, which is "a knob with two positions and one picture". Now the world pick looks through a dead unit that holds no loot for us, which is what SuperAPI's own description asks for: "click through corpses to loot corpses underneath them" — the pile after an AoE pull, where the one body you want is behind four empty ones. The test is `CanLootUnit`'s own, negated (`clickthrough_skips`), so "lootless" means one thing to that verb, to this pick and to the loot sparkle; and because `UNIT_DYNFLAG_LOOTABLE` is per-viewer on the server, a corpse somebody else may loot is lootless TO US, which is exactly the pile the feature is for. A living unit is never skipped whatever its flags — that would make mobs unclickable — and a unit whose descriptor has not streamed is not known to be empty. The flag is mirrored out of the VM once a frame rather than read in the pick, which is a `SystemParam` with no reach into the script; a one-frame lag on a preference set once is immaterial. The resource is `Option` in the pick, like `factions` beside it: a harness that drives this pick builds the world it needs and nothing more, and absent means off, which is also the default. The stale aggregate row this closes is rewritten rather than deleted, because what it got wrong is the lesson: it called six functions "blocked — each needs new app-side state", and three of those blockers were answered by data the tree already had. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
0e02401325 |
superwow: CastSpellByName at a unit, FoV, NameplateMotion, the raw log
The DLL publishes its own **usage strings**, in one block beside the names it registers, and reading them turned up four things this file had wrong — three of them bugs rather than gaps. `CastSpellByName(name [, "unit"])`. A unit ID, which is what every real call site passes and what `Model::unit` already resolves. It used to fall through to the ordinary queue, which casts at the player's OWN selection — a silent wrong target at six sites in grom's corpus: pfUI's /castfocus (a GUID) and /castmouseover (a token), Decursive's cure-at-guid, BigWigs' spell requests and two encounter scripts. Each asked for a unit and got whatever was targeted. The stock `onSelf` meaning keeps its behaviour: only a string that resolves to a live unit takes the new path. `CombatLogAdd`'s second argument names a FILE, not the event. There is a third log literal in the same pool — `Logs\WoWRawCombatLog.txt` — and it is the GUID-form log, the same lines RAW_COMBATLOG carries as arg2. `ui_chat/logging.rs` keeps it beside the ordinary one now, opened by the same `LoggingCombat`, which also makes the arg2 composition worth paying for when nothing is registered but the log is on (`wants_raw_form`). My first reading, "also fire the event", was an inference from the name alone, and the second literal settles it. `FoV`'s hold-back is resolved, and the answer was already in the tree. The row was held because 1.57 looked irreconcilable with CAM_FOVY's 0.785. It is the same number: 1.57 is π/2, the reference's own `[camera+0x40]` default of 90° in radians, and that field is not a vertical FOV at all — `scoped_view.rs` already divides by it as a DENOMINATOR, because what it expresses is a ratio. So upstream's own default is this client's existing lens by construction, which is what the test asserts; the DLL even registers the literal `1.57079`. Live, where upstream's panel says "Requires UI Reload". `NameplateMotion` asked for an investigation that was also already answered: benilla spreads plates with the reference's own SmartScreenRect solver. 0 (Overlap) bypasses it, 1 (Default spread) is it; 2 and 3 are the mod's own algorithms and fall back to 1, a documented silent-equal rather than a refused row. Also: `GetTradeSkillItemLink(index[, spellLink])`, the only way to link a recipe that makes no item — an enchant, whose product_item is 0 and which answers nothing at all on the stock surface. And four stored-only rows (SelectionCircleStyle, UncapSounds, HealingText, InterfaceVolume), each with its own reason, so SuperAPI's options panel round-trips. Three ghosts retired: outline/shadow text, force-show-auras and XP text are in no part of the DLL's string pool, nor is NameplateVerticalSpacing. They came from the early wiki reading, not from the binary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
323f3ae03c |
superwow: the map pair, GetQuestLinkForID, GetWeaponEnchantID
Four more off the DLL's manifest, and two measured corrections. GetMapPositionWorldLoc is the INVERSE of GetPlayerMapPosition, which is what the test asserts rather than three numbers: `map_proj::zone_world` is the reference's `0x4a7100`, whose own note is that used one axis per call it inverts the `zone_uv` that verb answers from. The law is transcribed into benilla-ui (which cannot depend on benilla-world) and the round trip is welded, which is the thing that actually keeps the two in step. GetMapBoundaries shares one new `worldmap::loc_rect_at` with the hover grid's own windowing, so no two readers of a (continent, zone) selection can disagree about which rect it means. GetWeaponEnchantID is the one thing GetWeaponEnchantInfo cannot say: the reference reads that very dword as its gate and then reports only that there WAS an enchant, so nothing on the stock surface can tell Windfury from Rockbiter. The id was already in hand and discarded as a `?` gate, so this was one field on the snapshot, plus a test that the two functions cannot disagree about which hands are enchanted. GetQuestLinkForID answers for a quest in the log and nil otherwise — which is also what upstream answers on the first call for one it fetches. The gap is one feed, not new protocol: the ask-once CMSG_QUEST_QUERY cache already exists app-side. That row also corrected the LINK FORMAT for both quest getters. The DLL's own formatter is `|cffffff00|Hquest:%d:%d|h[%s]|h|r`; ours was built without the colour wrapper on the reasoning that it matched the stock quest-log shift-click — but 1.12 has no quest links at all, so there was no stock shape to match and the measured string is the only authority. The four sound-driver getters are deliberately NOT built, with the reason recorded: every one of their return shapes is unknowable from here, none is called in the corpus, and this client has one backend and no enumeration to report — the same bargain KEY_DOWN/KEY_UP lost. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
400457ff82 |
superwow: TrackUnit/UntrackUnit, and the four chat-bubble CVars
TrackUnit is the one with a live consumer and a real feature behind it: 1.12 draws party members on the minimap and raid members NOWHERE, and pfUI puts the whole raid on it with these two — for every party and raid token on PARTY_MEMBERS_CHANGED/RAID_ROSTER_UPDATE/PLAYER_ENTERING_WORLD, each inside a pcall, so on a client without the function the feature is simply absent, which is what it has been here. The set is guids, not tokens, so a tracked raid member keeps its dot while the roster shuffles its token out from under it. The dot itself is drawn by a fourth always-show clause beside the reference's own three (`shows_tracking_dot`) — `tracked_creature` stays exactly `0x5ed210`, so the predicate that file documents is still the client's, and a tracked unit gets the ordinary cell, hover name and distance cull with nothing new drawn. The set is read off the VM handle the blip system already holds rather than behind a resource of its own: `BlipInputs` is at the 16-param ceiling and its last element is already a nested pair for that very reason. The bubble four come straight off the DLL's string pool. Three are nampower's `NP_ChatBubbles*` under SuperWoW's unprefixed names, writing the same fields — two mods, one knob, last writer wins, as the nameplate reach already is — each keeping 0 because the reference bubbles none of those kinds. `ChatBubblesCreatures` is SuperWoW's alone and the only behaviour of the four: it NARROWS `ChatBubbles` rather than standing beside it, because the mod hooks the reference's own gate and a switch added at that site can only subtract; the other reading would bubble creatures with the master switch off. It ships 1, so an untouched client bubbles monster say and yell exactly as 1.12 does. All four are welded to their struct literals in the defaults-mirror test. CREATE_CHATBUBBLE is deliberately NOT fired, and the row says why: its payload can only be the bubble, and this client's bubbles are not Lua frames — the same wall `C_ChatBubbles.GetAllChatBubbles` already answers an empty list for. Argless would be worse than silence, since an addon written against upstream reaches for arg1's regions and raises once per bubble. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
ac857ada36 |
superwow: CanLootUnit, GetSpeed and UnitNameplate
Three of the DLL's own manifest, and two of them needed no new feed at all — which is the point worth recording, because the row that called this family "todo, blocked: each needs new app-side state" was wrong about two of the three. `CanLootUnit` is the loot sparkle's test read through the token map: `UnitState` already carries `dead` and raw `dynamic_flags`, so a corpse that sparkles answers 1 and there is one source of truth for "has loot for me". The bit is per-viewer on the server, so "can" means can. `UnitNameplate` goes through the SAME `plate_frame` wrapper as ClassicAPI's `C_NamePlate.GetNamePlateForUnit`, welded by a test that asserts both names hand back one object — an addon may feature-detect either, and two different frames for one plate is the kind of bug that only shows up as an anchor in the wrong place. `GetSpeed` is the one that needed a field: the posture snapshot gained `speed`, filled from `net::current_speed`, the reference's own `CMovement::GetCurrentSpeed 0x7c4c90`. What could not be settled is written down rather than guessed over — upstream publishes no prose and nothing in the corpus calls it, so "current" is a reading, taken because it is the more informative of the two and because it is the only function in the client named after speed that a hook would return. Standing still it is 0, as that getter is. `SelfMovement` loses its `Eq` (an f32 field); nothing compared two snapshots — the feed assigns a fresh one each tick. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
3b6e91be35 |
superwow: RAW_COMBATLOG and CombatLogAdd, the only two names his addons call
Of SuperWoW's whole surface these were the two names grom's corpus calls
that this client lacked (`scripts/dll-surface.py --wanted`), and the first
is load-bearing out of proportion to its size: `SuperCleveRoidMacros`
funnels FIVE subsystems through one `RAW_COMBATLOG` frame — immunity and
resist learning, hidden-CC detection, debuff-duration learning, and the
reactive-proc window that lights Overpower and Revenge. Its own comment
says why the last of those can come from nowhere else: "SPELL_GO only
provides hit/miss COUNTS, not miss TYPES".
`route` is the fire site: the one funnel every combat line already passes
on its way to a window and to WoWCombatLog.txt. `arg1` is the printed
sentence; `arg2` is the same sentence with each unit name spelled as its
GUID, composed a second time through the same `compose_line` so the two
can only differ in the name slots. Fired before the `CHAT_MSG_*`, which is
what "raw" means upstream. Both the fire and the `arg2` composition are
gated on `has_listener` — a raid's combat log is tens of lines a second.
`CombatLogAdd` is also BigWigs' SuperWoW detection (`if CombatLogAdd
then`), so defining it turns that module's path on.
Three corrections came out of reading the DLL's own usage strings, which
it publishes beside the names — the arity and argument meaning of every
remaining SuperWoW function is now measured rather than inferred, and
`docs/ADDON_COMPAT.md` carries the manifest:
- `CombatLogAdd("text" [, addToRawLog])` has a SECOND argument. Nothing
in the corpus passes it; it would have been missed.
- `SUPERWOW_COMBAT_LOG_EVENT` does not exist. The DLL registers exactly
three events, and that name is in no part of its string pool.
- `IsIndoors` is capital-I beside `IsSwimming`/`IsMounted`, not the
lowercase `isIndoors` two files guessed at — all three are already live
off one feed, so that clause of the "blocked" row was already answered.
- The detection pair is `SUPERWOW_STRING="SuperWoW 2.2 by Balake"` and
`SUPERWOW_VERSION="2.2"`, both quoted. Ours had the string without its
author clause and the version as a number.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
5a824f1625 |
app: the press queue was switched off in play — a derived Default is the live config
`cvars.rs` announces a `CvarChanged` only when a value *moves*, and `config.toml`
holds only the rows that left their defaults ("CVar values that moved off their
defaults" — its own header). So a row the player never touched never reaches an
`on_cvar` observer, and the resource's own `Default` is what the client runs with
for the whole session.
`PressRules` and `SpellQueue` both derived `Default`. Every toggle was false, every
window 0 and every buffer 0, so the press-interception queue held nothing, the mount
refusal never fired, and `SpellQueue`'s zero window expired each explicit entry on
the frame it was queued — the day after all of it was committed. Fourteen of the
sixteen rows have a non-zero registered default.
grom's own `WoWCombatLog.txt` is what named it, with no live run to ask for: 147
`Sunder Armor: Not yet recovered` against 163 landed Sunders, in bursts 0.17-0.23 s
apart — every press refused, none held. Sunder Armor rank 1 on this realm is
`rec=0 cat=0 catrec=0 gcdcat=133 gcd=1500`, read off its own `Spell.dbc`: a global
cooldown and nothing else, which is precisely the case the queue must hold.
Both now hand-write `Default` with the registered values, which is the pattern
`cvars.rs::registered_defaults_mirror_the_code_truths` already welds for the engine
knobs; `np_press.rs::the_default_is_the_registry` welds these sixteen the same way.
The test-only `defaults_for_tests()` constructors are gone — they spelled out the
registered numbers for tests to read while the client could not reach them, which is
the whole shape of the bug. A sweep of the other nine CVar-cached resources (list
every `fn on_cvar`, take its `ResMut` types, check for a hand-written `Default`, look
up each CVar it names in `REGISTERED`) found no further victims.
Two things so the next session can speak for itself:
- `a_press_made_during_the_global_cooldown_is_held_and_sent_when_it_ends` — the
end-to-end cycle with the GCD as the only guard, which is what a class with no cast
bar presses into all day. The existing end-to-end test arms `PendingCast` by hand
and so never starts a GCD at all; this one presses through the real ladder, so the
commit arms it the way a cast does.
- one `info!` a minute from the drain, and only when something moved: held, sent,
dropped, retried, fell-through, channels cut — plus the live windows and buffers.
Every other line this half writes is `debug!` and the default filter is INFO, so a
67-minute session could not say one word about the queue. A line of zeroed knobs
now diagnoses this class of fault by itself.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
|
||
|
gromandClaude Opus 5
|
fcdf14c0dc |
app: a held press cannot time out while something still guards it
The queue stamped each entry's deadline when the press was made. That reads right and has a hole in it: a cast pushed back twice (`SMSG_SPELL_DELAYED`) ends after the window a press made at its start was given, so the entry times out while the thing it is waiting for is still running — and the press vanishes with no packet, no error line and no cast. That is the one failure mode a player cannot describe, and it is exactly the class of bug the press queue was supposed to remove rather than introduce. The same hole swallows a press held through a cast whose global cooldown then outlasts the window, which is the ordinary case rather than an edge one: a 500 ms window, a cast that ends, and 1.5 s of global cooldown behind it. So a slot now keeps the window and the base it is measured from, and the drain ratchets that base forward — only ever forward — past every live guard: the cast's reopening, the channel's trimmed end, and the global cooldown's. An entry gives up when nothing is holding it back any more and the window has run out since, which is what "how long past the guard's end" meant all along. Scripts keep a fixed deadline. Their contract is "after queued spells", and a script whose spells never fired has nothing left to run after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
39932b12da |
app: nampower's channel latency trim — the last default-on queue row
`NP_ChannelLatencyReductionPercentage` ships 75, not 0, so a real install really does take three quarters of the round trip off a channel's end. Without it a press held through a channel is sent a round trip late, which is the cost the whole press queue exists to remove — leaving this stored would have left the queue's worst case exactly where it started. **Why sending early here is conservative and not reckless**, which is the part worth writing down. The server starts the channel at T and ends it at T + duration; we only learn of it half a trip later, so our own end sits at T + L/2 + duration — half a trip *after* the server is already free. Trimming 75% of L puts our end at T + duration − L/4, and a packet sent then reaches the server at T + duration + L/4: still after its own end, by a quarter of a trip. Same arithmetic as the cast buffer, and the round trip it removes was never real work. It reads the channel's **named** end, not the slack-inflated one. That slack is two seconds of belt-and-braces for a lost `MSG_CHANNEL_UPDATE(0)`, and reading it here would make every channel look longer than it is — the pop gate was doing exactly that, so the press would have fired a round trip and then some after the channel finished. With no pong measured yet nothing is trimmed, which is the safe direction to be wrong in. The lock is recovered rather than unwrapped, as every reader of it is, and that matters more here than at the latency meter: the cast path reads it now, and a poisoned lock must not take the ability to cast with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |