ee9cfdcacf63f2b048b1ba0ff19d43f3fe4fa605
1223
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  gromandClaude Opus 5
|
ee9cfdcacf |
Two settings for things arriving: a foliage distance row, and a ramp for lights
**Foliage Distance** (`foliageDistance`, Advanced Graphics, 70..300 yd). The reference draws ground clutter to a hardcoded 70 yd (`[0x867958]`) and we inherited it, so grass, reeds and small bushes arrive at a fixed ring around the player — which is exactly where the eye is drawn to them appearing. The row moves the ring; it changes nothing about what grass looks like. Measured on an Elwynn fence line (`fence-shadowline-day`, clutter density 2), p50 frame time / main thread: 70 yd **3.25/3.26 ms**, 140 **3.32/3.24**, 200 **3.48/3.38**, 300 **3.69/3.42**, with the GPU flat throughout. Doubling it is free; past 140 it costs main thread, which is the budget this client is short of. The registered default IS the reference's own 70 and the row is in no preset ladder, so nothing moves until a player asks it to. It commits on Apply: `fade_far` is baked into the clutter material key, so a change drops every built patch to be re-cut (338 meshes on one measured write) and a drag would do that thirty times a second. `clutter::remesh_on_cutout_change` is now `remesh_on_material_change` and watches both the alpha ref and the fade for that reason. What it buys in a STILL is small — 70 -> 300 moves 0.6% of a frame, the far tufts being sub-pixel — and that is the wrong instrument: the complaint is grass ARRIVING, which no static capture can show. **Light arrival fade** (`lighting::global_light`). No setting: a correction, not a taste. A fixture enters the packed light table for several unrelated reasons — the portal walk reaching its room, its WMO group streaming in, the camera crossing `INTERIOR_NEAR_ADMIT` or `POINT_PACK_RADIUS`, the 255-entry capacity cut re-sorting — and every one of them was a hard edge that switched a light on at full brightness between one frame and the next. Measured in Ironforge by diffing the admitted set between static vantages: walking **3 yd exchanged 2 of 172 admitted lights, and 10 yd exchanged 8** — about one arriving per yard, which at a walk is several a second. The table ran 110-196 across five vantages and never reached the cap, so this is admission churn and NOT capacity; the cap does bind transiently during a world load (305 candidates seen once), which is why the boundary fade below is kept. Arrivals now ramp over 0.35 s, whichever rule admitted them. The scale lands on the light's colour before the pack, so the GPU table, the CPU mirror, the fog lamps and the froxel volume all follow from one write. Deliberately NOT symmetric: a departing fixture still goes at once, because keeping it alive to dim means keeping its room claim alive with it, and a stale claim gates a live light with a dead building's identity. **Spell lights are exempt**, and that is the exemption that matters: a fireball's light lives about as long as the ramp does, so ramping it would spend the whole effect at half strength. Beside it the capacity cut itself is softened — the last 24 admitted ranks scale toward zero across the distance band they span — and nothing fades while the table fits, which is what makes the control possible: with the ramp on against off, the Goldshire inn and the Great Forge both settle BYTE-IDENTICAL. Also recorded in docs/GRAPHICS.md: captures force `WOW_CLUTTER_DENSITY=0` for byte-stable baselines, so a run that sets only `WOW_CLUTTER_FADE` measures a scene with no grass in it and reports every horizon as free. Three rounds of this measurement were worthless before the starved control caught it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
33fc0c2023 |
Froxel volumetrics: the haze stops painting shadowed air the colour of sunlit air
`volumetricFog` gains tier 3, "Volume", with a "Shadowed Air" dial beside it; the Ultra rung of Lighting Quality takes it. A view-space 160x90x64 froxel grid — a fixed BUDGET, so every resolution costs the same — marched one compute thread per screen COLUMN, front to back, with a reprojected eight-frame history. Inject and integrate in one pass, because a froxel past the last shadow cascade has no evidence either way and the honest answer is the mean of what the rest of its column could see; a thread that owns the column can carry that and a froxel-per-invocation dispatch cannot. It publishes two things: - **the lit share** — how much of the air in front of each pixel the sun actually reaches, weighted by each slice's own contribution to what the eye sees through it. `volumetric_fog`'s haze multiplies its colour by it. The haze below this rung is the reference's: a zone colour mixed in by distance, the SAME colour whether the air it tints stands in open sun or a mountain's shadow, and no screen pass can tell it otherwise because a shaft march only ever ADDS light. On an Elwynn overlook at noon, 74% of the frame is air the sun does not fully reach. - **the lamps' in-scatter**, free once a volume exists: the phase lobe evaluated at every point of the air rather than once per ray at its closest approach, attenuated by the medium it crosses and strengthened when the medium is. `lampFog`'s analytic halos retire into this solve at tier 3, and that row still chooses which lamps and how many. The gain is derived, `0.9 / density(clear night)`, so the two lanes match on a clear night. At the Goldshire inn, isolating each by rendering lampFog 0 against 2 on both (the bases are byte-identical, which is what makes it a comparison): the volume's are 1.30x and wider. The sun's SHAFTS deliberately stay on `volumetric_light`'s per-pixel march, and that is a measurement. A volume bands in hard horizontal steps at 160x90, 320x180 and 534x300 alike, because one froxel column hands the same value to about a hundred pixels and a canopy's shadow is finer than any grid worth paying for. Spreading each column's samples across its own screen footprint (R2 down the column) turns the steps into a gradient and is worth knowing for any volume, but the march still wins on detail. docs/GRAPHICS.md has the sweep. Safety is one number: fully lit air multiplies by EXACTLY 1, so at Shadowed Air 0 the lane renders byte-identical to High over all 1,440,000 pixels with every pass still running. Getting that exact needed `1 - (1-floor)*(1-lit)` and not the `mix(floor, 1, lit)` it equals — `mix` is a fused multiply-add and lands a last ulp below 1.0, which moved five of nine tier 0-2 control pairs with the rung switched OFF. The shipped floor, 0.85, is deliberately half the depth of the first one tried, and the dial reaches twice it. Control: nine tier 0-2 pairs against the previous build, five byte-identical and four differing by one or two PIXELS out of 1,440,000 at 1-2 of 255 — every tier-0 pair identical, and tier 0 is the tier that switches the fog pass off, so the remainder is the fog shader's instruction schedule changing because the shader did. Cost is under the noise of both instruments: the frame probe puts Volume inside High's own run-to-run spread, and the GPU pass timer puts `gpu_other` at 0.75 -> 0.76 ms. Also recorded: `water-lake` leaves the deterministic row once shadows are forced on — it differs from a build of ITSELF by up to 88/255 on 0.03% of pixels. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
848c840ecf |
SSILVB: the AO lane stops only darkening and starts bouncing light
`ambientOcclusion` 3 (Indirect Light), the Ultra rung. Tier 3 of the AO row rather than a row of its own, because it is one trace: the bitmask that measures occlusion properly is what produces the bounce, so a player choosing between them would be choosing between two halves of the same thing. Classic SSAO sums a cosine term per tap, so two occluders covering the same direction count twice and a cluttered corner over-darkens. A bitmask counts DIRECTIONS: each slice owns a 32-bit word of angular sectors, a tap sets the run it spans, overlap is a no-op. And the bits a tap sets that were NOT already set are exactly the share of the hemisphere it is the nearest thing in — i.e. the fraction of the receiver's light it alone is responsible for. Its own pixel colour, weighted by that, IS the one-bounce indirect term. No second trace, no probes, no extra pass. ## The lane was already shaped for it - `STAGE_AO` already bound the resolved scene colour, for the bright-pixel protection mask. That buffer is exactly what a screen-space gather reads. - The apply needed no new pass and no dual-source blending: visibility in `src.a`, bounce in `src.rgb`, blend `One, SrcAlpha` = `bounce + dst*vis`. - Only the half-res stages grew, by one attachment each, and the uniform's two spare floats carried the strength and thickness, so its layout never moved. ## The one part that is ours and not the paper's **The bounce is scaled BY THE OCCLUSION.** A physically-measured bounce is wrong here: 1.12's lighting already carries an ambient term standing in for indirect light, so adding a measured one double-counts it. Unscaled it lifted a daylight exterior's median from 85.7 to 101.3 and washed the frame flat. Scaled by `occ`, the lane puts back only the share of the ambient the occlusion just took away, in the occluder's colour. An open field gets nothing, because nothing was removed there; a corner gets the colour of the wall shading it. Redridge overlook noon median 85.7 -> 86.7 Goldshire inn noon median 26.7 -> 29.0 Neutral in daylight, +8.6% in the shaded room — the same shape as the auto-exposure lane's floor of 1.0, for the same reason. The gather runs in LINEAR (`c*c` in, `sqrt` out): the scene is stored in gamma, and 0.5 gamma is 0.22 of the light, not half of it. ## Cost Frame time is unchanged (inn 2.22 -> 2.21 ms over 600 frames; overlook 2.25 -> 2.11). That is the frame being CPU-bound, not the lane being free. The GPU meter is the honest instrument: whole-frame GPU 1.68 -> 1.81 ms, and the bucket these passes land in 0.40 -> 0.54. So **+0.14 ms of GPU over High and nothing on the main thread** — which is why the Options text says "a fraction of a millisecond" rather than what I first wrote before reading the meter. ## Two bugs, both found in the intermediate rather than the frame - **Screen +y is view -y.** `view_at` flips clip y, so a slice direction built as `vec3(omega, 0.0)` measures its taps against a mirrored normal. One slice of two was right and the other inverted, averaging to A FLAT FLOOR READING 50% OCCLUDED. `WOW_AO_DEBUG=4` showed it in one run; the composited frame never would have. - **`a / b` and `a * (1.0 / b)` are not the same number.** Factoring the blur's divisor into one reciprocal — a tidy-up, not a change — moved `water-lake` at tier **1** by up to 53/255 on 0.021% of its pixels, because the AO is multiplied into the colour the water lane's depth-march then refines by bisection, and one ulp picks a different step. Restored to two divides. ## Control Tiers 0-2 byte-identical to the build before this, on three deterministic scenarios at three tiers each, nine of nine. `fence-shadowline-night` had to be dropped mid-control, and it sharpens the determinism table added last commit: it screens as deterministic at the DEFAULT tier and is not at tier 1. Screen a scenario at the setting you are about to measure it at, not at its default. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
11dda4f9cf |
Two cheap lanes: RCAS sharpening, and a field of view a player can move
Both were named in docs/GRAPHICS.md as nearly free and neither had been built. They are unrelated to each other; they land together because each is small. ## Sharpening (`sharpening`, 0 Off / 1 Low / 2 Medium / 3 High) RCAS, FSR 1.0's sharpening half taken on its own. It does not resample, so it needs none of the upscaler's motion vectors, jitter or history — which is the whole reason it can land while §7 cannot. One 5-tap cross; the lobe is not a constant but the widest one that cannot push the result outside the ring's own range, solved per channel against a peak of 1.0 and taken as the most conservative of the three. Flat regions get nothing and nothing rings. The seat is most of the design. `Core3d`, after `FfxGlowLabel` and before `Node3d::Bloom`: - After the glow, not before. `FfxGlowNode` builds the blur chain the glow and the underwater haze read, and a sharpened bloom source is how an unsharp mask turns a lit window into a ringed halo. - The world view only. The world's last reader is the FFXGlow combine drawn inside the UI camera's main pass, which binds `main_texture_view()` at node-run time per frame, precisely so a `post_process_write` upstream lands. The interface is drawn afterwards into a different target — and sharpening text is the one thing this must never do. - Every tap saturates, as the combine's own scene read does one pass later: the limiter is solved against a peak of 1.0, so a super-white tap would invert the bright side's sign. Nothing is lost, the blur chain already took its copy. - No denoise term. The deband dither it would exist to suppress is added downstream, in the combine's exit. Measured, 600 frames: Redridge overlook 2.95 -> 3.02 (Medium) -> 3.01 (High); Goldshire inn 3.46 -> 3.51 -> 3.29. Inside the noise, the inn reading *faster* at High being how you know. CPU flat. What it buys: 3.8% / 19.9% / 48.6% of the frame moved by more than 2/255. **The first run hit the trap this repo already has a note about.** `max(4.0 * mx, 1e-5)` where `mx` is a `vec3` is `error: inconsistent type passed as argument #2 to max`. naga refused the shader, the pass drew nothing, and the capture came out looking perfectly normal. One `grep -ciE 'error:'` on the capture log answered it before a single image was diffed. ## Field of view (`fieldOfView`, vertical degrees, 40..70, default 45) Field of View on the Graphics page under Render Scale, live as you drag. It is the BASE lens, not a fourth factor. `player::scoped_view` already multiplied two things in and neither is a field of view: SuperWoW's `FoV` is the reference's `[camera+0x40]` as a ratio, the spyglass is a zoom off normal. Both answer "how much wider than normal"; nothing a player could move answered what normal IS, and the answer was a `const` in `benilla-world`. So the term order is `base × FoV × zoom`, and an addon writing `FoV` still means the same fraction of the lens the player chose. Vertical degrees, not horizontal, which keeps the reference's Hor+ framing: a fixed vertical lens widens horizontally as the window does, so an ultrawide player gains picture at the sides instead of losing it off the top. The default is `CAM_FOVY` to the bit — `45f32.to_radians()` and `FRAC_PI_4` are both `0x3f490fdb` — which the defaults-mirror test now asserts on `to_bits()` rather than against a tolerance. An ulp of projection is a pixel in a capture diff, and a row that reframes the world for a player who never opened Options is not a row, it is a regression. ## Both lanes off: byte-identical Against a `git stash`ed build of the same tree: `overlook-noon` and `water-lake` identical, and `fieldOfView=45` written explicitly identical to the row never touched (40 and 65 move 97.6% and 99.3% of the frame). `inn-noon` is excluded WITH PROOF rather than quietly: two runs of the *same* binary differ on 37.8% of their pixels by up to 199, because that scene's flame emitters do not reproduce. Its own before/after delta is 0.00% of pixels at a maximum of 1. A scenario that cannot reproduce itself cannot be a control, and the number to check first is always the baseline against itself. ## Seats Six Options edits each, per the recipe, including the silent one (`OPTIONS_PAGE_ROWS`). Sharpening is a dropdown on Advanced Graphics with the other screen effects and reuses the existing `qOff/qLow/qMedium/qHigh` labels; Field of View is a `raw`-format slider on Graphics, whose label is literal because that page is not localised. Preset ladder: `sharpening` takes `ambientOcclusion`'s shape (`0 0 1 2 2`) because a look change starts at Medium in this file's convention; `fieldOfView` is on no rung at all, because it changes how the game feels to play and that is the player's call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
280c2e886f |
Planar water: the mirror works, and ladders in at waterQuality 3 (Ultra)
The starburst was three defects, and none of them was the oblique projection I blamed it on in the last commit. What found them was looking at the mirror's own render target instead of reasoning about the composited frame — the water reads it through a fresnel weight, a rippled UV, a plane-match band and an SSR march that paints over it, so from the outside a broken mirror and a broken water shader look identical. WOW_MIRROR_DUMP=1 now writes that target beside the capture. (Bevy's own save_to_disk cannot: try_into_dynamic knows four 8-bit formats and answers UnsupportedFormat for Rgba16Float, logging and writing nothing.) - The oblique row's scale. Replacing the near plane with the water plane costs the FAR plane: `z >= 0` stops meaning "nearer than CAM_FAR" and becomes `depth >= a*d`, a plane through the eye tilted by a. Lengyel's own scale puts that plane exactly through the frustum's far corner, and the sky dome is pinned just inside the far plane, so it straddled the boundary and was sliced along its own tessellation. Scale by the supremum of d/depth over the frustum instead, with 2% of margin. - The water surface lay ON its own clip plane. This was the starburst and it was fatal rather than untidy: depth now means "distance above the plane", so a fragment at d=0 reads z/w=1, the nearest value reverse-Z has, and beats every other surface in the frame. The mesh is wave-displaced, so its triangles straddled the plane instead of lying in it, and each was clipped to a sliver that kept the winning depth. SURFACE_CLEARANCE lifts the plane a yard: the swell sums to 0.34 and the probed height answers for the surface under the eye, not this fragment. - The mirror was stored upside down. A true reflection is what a camera on the far side sees with its up vector reflected too — an improper basis no Transform can hold. The honest right-handed camera sees the same surfaces through the same frustum and stores the image flipped, so the shader samples 1 - v; that costs no winding flip, so backface culling stays correct. It read as a lake reflecting the ridge the RIGHT way up. Measured: Lake Everstill 2.87 -> 3.36 ms mean over 600 frames (+17%), the Stormwind canal inside the noise, CPU +0.2 to +1.1 ms. It is the first lane in docs/GRAPHICS.md that spends main thread, so it is a rung of its own above High and the default stays 2. Seated as a setting, per the standing rule that no graphics lane hides behind an env var: waterQuality gains tier 3, the Ultra preset takes it, and the Advanced Graphics dropdown gains a fourth entry in both locales. With the lane off, both scenarios are byte-identical to the build before it — which took two findings of its own: - The system could not live in Update at all. `&mut Transform` on a camera is shared with everything that moves anything; declaring it put 59 undeclared orders into Update and tripped the schedule ratchet, which is what kept this behind an env var. It belongs in PostUpdate with the skybox and the billboards, after propagation and before visibility, where it also reads the eye's settled pose instead of a frame-old one. (The comment blaming ResMut<Assets<Image>> for those 59 was wrong — dropping the asset handle changed the count by zero.) - A system that runs and returns on its first line is still not free. Holding &mut Camera and &mut GlobalTransform for a frame is enough to change which undeclared orders the executor picks: the canal moved 0.75% of its pixels by up to 96/255 with the lane switched off. So the whole system sits behind a run condition rather than an early return — and the same effect killed a .before(UpdateDirectionalLightCascades) order that was correct in itself. PostUpdate's ambiguity ceiling goes 466 -> 495 with the reason (the pairs are structurally impossible: nothing else in the schedule names the mirror camera); Update's ratchets down 4973 -> 4952. Also restores scripts/gates.sh's `where` example, which had gone missing. Its absence was silent — cargo's error went to /dev/null and `|| true` made it read as "no install" — so the memo key and the player-tests gate saw none even when WOW_DATA was set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
77fce9210e |
The water already reflected the scenery; it was switched off
`docs/GRAPHICS.md` said we had refraction and foam but no mirror. That was
wrong. `waterQuality` 2 has carried a depth-marched screen-space
reflection of the scenery all along (`enhanced_water.wgsl`, "HIGH"), with
bisection refinement against the real scene depth rather than the fixed
probe its WarcraftXL source took. What was missing was the switch: the
registered default was Enhanced (sky only), and of the five lighting rungs
only Ultra reached High.
The line holding it back said exactly what it was waiting for —
"Mirror reflections stay opt-in until their cost is measured." So I
measured it, `WOW_FPS_PROBE=300`:
Classic Enhanced High
Lake Everstill 1.91 ms 1.84 ms 1.90 ms
open ocean 1.89 ms 1.88 ms 1.89 ms
Every tier is inside the run-to-run noise — Enhanced lands FASTER than
Classic on the lake, which is how you know you are reading noise rather
than a cost. What it buys is not noise: the far bank and the mountains
appear in the water, 18.5% of the lake frame and 35.1% of the ocean frame.
So High becomes the default and the High rung. That is a weld across four
places the tests hold together, and each one bit in turn and was fixed as
it asked: `REGISTERED`, `VideoConfig`'s literal, `WaterQuality::default`,
and the rung itself — the High rung IS the registered defaults, which
`the_high_preset_is_the_registered_defaults` states and enforces.
Low and Medium keep Enhanced deliberately, and that needed saying in the
one test that assumed every rung between Off and Ultra sits AT its
default: free on the machine it was measured on is an argument for the
default, not for the rung whose whole job is hardware that cannot afford
what this one can. `water_and_lava_observers_clamp_and_water_high_is_opt_in`
asserted the old decision in its name and its body, so it is renamed and
now pins the pairing the other way: High reflects, Medium does not.
The honest remaining gap is written into the doc rather than claimed as
done: SSR gives up when the ray leaves the screen and fades toward the top
edge, which is where a shallow-angle lake wants its far bank. A planar
pass would fill that — and it is the first item on the list that draws the
scene twice, so it costs main thread and gets priced against item 1's
view-distance table before anyone builds it.
Verified: ALL GATES GREEN (492 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
|
113572b8e2 |
Fix: auto exposure was manufacturing bloom on ordinary bright surfaces
grom's play test, in Ironforge: "some edges, and seems to be only in
interiors, are shining really bright, like flashing colors". His config
has `autoExposure = "1"`, so the lane I landed last commit was live, and
"interiors only" is its exact signature — the gain is 1.0 outdoors by
construction and only ever rises indoors.
The apply pass was a plain multiply, and a plain multiply pushes ordinary
bright surfaces past the white point. Everything downstream reads a value
above 1.0 as EMISSIVE: `post/bloom.wgsl`'s extract is literally
`max(c - 1, 0)`, and FFXGlow squares its blur. So a lamp surround, gold
trim or a window frame sitting just under white crossed the line and grew
a bloom halo it had no business having — and because the gain drifts with
what is on screen, the set of pixels across that line kept changing and
the haloes FLASHED. It also scaled the HDR headroom itself, doubling the
bloom of everything that genuinely was emissive.
Measured, pixels at or above 250/255 — which is the bloom's fuel:
off Subtle Full
vista 0.338% 0.338% 0.338% (gain is 1.0: untouched)
inn, noon 0.024% 1.930% 9.779% ( 80x / 407x)
inn kitchen 0.001% 0.312% 1.549% (312x / 1549x)
Interiors only, exactly as reported.
The fix is a tone curve instead of a multiply: extended Reinhard with its
white point AT the gain. Three properties, each load-bearing — it is the
exact identity at gain 1 (`e(1+e)/(1+e) = e`), so a daylight exterior is
untouched; it maps the white point to white at every gain, so it can NEVER
produce a value above 1.0 and cannot manufacture an emissive pixel; and it
is monotonic, so highlights roll off toward white instead of flattening
onto it, which a hard clamp would have done. The HDR headroom now passes
through unscaled: it belongs to whatever was actually emissive, and how
bright the room is has no bearing on it.
After, same measurement at his tier: the inn goes 1.930% -> 0.388% and the
kitchen 0.312% -> 0.061%, while the brightening is kept whole (kitchen
median 35.8 -> 35.4, pixels under 32 40.3% -> 41.0%). What remains is
genuinely bright pixels rather than manufactured ones — a surface that was
never emissive now cannot bloom at all, because the picture path cannot
reach 1.0 from below.
`the_tone_curve_cannot_manufacture_an_emissive_pixel` pins all three
properties over the tier range, and a second test pins the Rust mirror to
the shader text so they cannot drift.
The exterior is a no-op to within 0.12% of pixels at mean |d| 0.16 — a few
dozen shadowed-foliage pixels where the gain lands a hair over 1.0. Below
anything visible, and stated rather than rounded away.
Verified: ALL GATES GREEN (290 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
|
88b6998453 |
Auto exposure: dark rooms read at their own level
The programme's item 1 was terrain horizon occlusion, ranked first on the reasoning that it would make the frame cheaper on the thread that is actually full. **I measured it before building it, and the premise does not hold for this renderer**, so this is item 2 instead. The numbers are in `docs/GRAPHICS.md`: cutting the view distance from 777 yd to 177 — removing four fifths of the visible world — saves 0.44 ms of a 3.6 ms frame, p95 is flat at 6.4 ms at every distance, no main-world system in a steady vista exceeds 0.12 ms/run, and the model cull already takes 6,016 submeshes to 29 drawn. Horizon occlusion can only remove the fraction of that which sits behind a hill. The reference needed it on a 2004 CPU with no GPU-side culling; we do not. What the same harness said about item 2 was the opposite. Before anything was written: the Elwynn vista renders at a gamma median of 87.6/255 with 1.4% of its pixels under 32, the Goldshire inn at noon at 31.9 with 50.1% under 32, and the inn's kitchen at **12.8 with 94.5% under 32**. That is not a dim room, it is a black screen — and the reference has the same problem, because it has no metering either. A histogram-and-reduce over the HDR view target, smoothed in the log domain, applied ahead of the shafts, bloom and glow chain so FFXGlow's blur — which is built from the scene — sees the same exposed image everything else does. Two departures from the textbook version, both forced by what this renderer's pixels mean (the world is drawn in the reference's gamma convention, which the combine clamps to 1.0): - The gain has a **floor of 1.0**: it only ever brightens. Aiming at the exterior's own measured level is what lands it at 1.0 outdoors with no special case — the vista's median moves 87.6 -> 87.5, and 0.12% of its pixels with it. Daylight is the one look this client is measured against and nothing here was worth moving it. - It has a **ceiling**, which is the tier. The kitchen meters to roughly 24x; uncapped it comes out as daylight, which erases the room instead of making it readable. Subtle and Full are 2x and 4x: "a dark room you can see in" and "one you can read in", neither "a lit room". Measured after: inn noon goes 50.1% -> 4.6% of pixels under 32, the kitchen 94.5% -> 40.3%, and the vista does not move. **+0.08 ms of GPU**, nothing on the main thread. The calibration is the one thing to get right if this is ever re-derived, and it is written down because I got it wrong first: the aim level is the level of the scene THIS PASS SEES, not of the screenshot. The glow adds `w·blur²` downstream, so the finished vista log-averages to 0.0931 linear and the pass's own input to 0.060. Calibrating against the former aimed the lane 1.5x high and brightened the daylight exterior it exists to leave alone — which the vista capture caught immediately. Two rows on Advanced Graphics, both locales, the blend greyed while there is no gain to blend; on the Graphics Preset ladder from Medium, in the shape the other post lanes take. Classic keeps it off, as the reference has no metering at all. Verified: ALL GATES GREEN (550 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
|
50fd1f158e |
The interior lighting rig gets its ten rows, and the audit closes
The last ten CVars `video::on_cvar` answers that a player could not reach. With these the sweep is done: every graphics and video row the observer takes now has a seat, bar the three that are not settings — `frillDensity` (World Detail already drives it, and two dials on one knob is worse than none), `nearclip` (the projection near plane: changing it is a bug, not a preference) and `interiorDebug` (a visualiser). Seven shape the room — the light budget, the ambient floor under every indoor surface, the indoor daylight floor, the bounce gain, the reach past each fixture's authored window, the share of painted-in lighting kept where nothing reaches, and the room gate — and three are the indoor shadow budgets. Named for what they do to a room, not for the quantity they scale, because the quantity is the renderer's word and the room is the player's: "Unlit Room Floor", not "interior bake floor"; "Lights Stay in Their Room", not "room gate". Each description says what the setting is FOR rather than what it multiplies, and three of them carry the thing a player would otherwise misread: - `interiorAttenScale` at 0 is not darkness, it is the authored window switched OFF, so every fixture reaches the whole room. A left end that looks like "no light" and means "all light" needs saying. - `interiorRoomGate` is the one setting whose job is diagnosis: a room the gate leaves on ambient alone looks exactly like a bug, and one click tells the two apart. That belongs in the options window and not only in `/console`. - `interiorShadowEntityRate` is the only row in the indoor shadow block that actually costs frame time — neither count dial above it moved it, which is why it exists as its own row — so its description says to lower it first. Two grooves deliberately offer less than their CVar's clamp, as `RowRenderScale` already does (50-200% inside a 0.25-4 clamp): ambient and indoor daylight run 0..0.1 of a 0..1 clamp, because the shipped value is 0.015 and past about 0.1 a room is not lit, it is fogged. `/console` still reaches the rest. One offers less than its *registration* says: `interiorShadowDynamic`'s help line reads 0..16, but the observer raises 0 to 1 and clamps to `MAX_TORCH_DYNAMIC` (8), so a groove to 16 would be seven stops that all mean 8 and a zero that means 1. The gating is two-level: every row needs the live-fixture lane, and the three shadow budgets need the shadows on top of it. A budget row left live while its shadows are off is a dial that does nothing, which is how settings earn the reputation these ones had. Russian for all ten rows and their descriptions is written to match the register of the column already there; a native reader would still improve it. Verified: ALL GATES GREEN (346 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
|
2d1dbd5639 |
Seven more graphics settings get a seat: the filter pair, MSAA, the shadow rig
The sweep behind this: every CVar `video::on_cvar` answers, against every CVar the Options window names. Twenty of fifty-five had no seat at all, so the only way to them was `/console` — which is not a setting. Seven of them here; the ten interior-lighting rows follow. **Graphics page.** Trilinear Filtering under Anisotropic, because they are one policy: aniso at 2 or more writes filter mode 5 and trilinear writes mode 4 only while that bit is clear (`0x688d0d`), so the row is GREYED while anisotropy is on — not hidden, and not rewritten. It keeps the value a player set, and turning anisotropy off hands it straight back. The gating reads the STAGED anisotropy where there is one, since both rows defer: otherwise a player who moves one and has not pressed Apply sees the row it is about to gate stay live for the rest of the visit. Multisampling beside Vertical Sync and Resolution, the latched `gx*` rows it commits with. 1.12 had this row and we did not. Backed by the index verbs its own dropdown used (`0x48c580` / `0x48c640`) rather than by `gxMultisample`, because picking a format writes the colour and depth rows with it as one record — and it rebuilds its list on open, like Resolution, since at OnLoad the engine has published no formats yet. Only the sample count is named: the colour and depth halves of each triple describe our swapchain and steer nothing, so offering them would offer a choice that does not exist. Its description says plainly that multisampling does less here than in a modern game, because it misses the alpha-tested cutouts most of this game's foliage and fencing is made of. **Advanced Graphics.** The shadow rig's four unreachable rows — Soft Shadow Edges (the 9-tap gaussian against the card's own 2x2; Bevy's third mode is deliberately not offered, since it only resolves under TAA this camera does not run), Shadow Caster Range, and the two update-rate caps — plus Torch Shadow Darkness beside the softness dial it shares a shadow with. The rate caps needed their own readout. `0` there is **every frame**, the opposite of what it looks like, so a bare "0 per second" would read as the shadows being switched off — which is what the two toggles above them are for. They now say "Every frame", in both locales. Gating is per-lane where the lane is what matters: each rate cap is dead only while its own lane is, while the filter and the reach ride the cascade and need just one sun lane left. Russian for all five rows and their descriptions is written to match the register of the column already there; it would still benefit from a native reader. One thing worth recording for the next row: `OPTIONS_PAGE_ROWS` has positional contracts in its tests, and the list's own comments say "appended so the other rows keep their indices". It drives refresh, search and Restore Defaults — never layout, which is the frames' own anchors — so appending costs nothing and seats each row where it belongs. Verified: ALL GATES GREEN (322 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
|
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
|
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
|
||
|
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 |
||
|
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 |
||
|
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) |
||
|
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
|
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
|
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 |
||
|
gromandClaude Opus 5
|
a9bb9624e9 |
app+ui: nampower's press-interception queue — the half that changes how keys feel
The half of upstream's queue that `queue.rs` has said for three rounds it deliberately left out. A press the client would refuse for a **timing** reason is now held in the same slot an explicit `QueueSpellByName` uses, and the drain sends it the moment the guard clears: no packet, no global cooldown, no error line while it waits. One queue, because upstream has one — a second set of slots beside the first would be a different feature wearing the same name. **The guard reopens early, and that is the headline.** `PendingCast` kept one timestamp and now keeps two. The guard reopens at the cast's own already-known length plus `NP_MinBufferTimeMs`; the record still lives for the cast plus two seconds, so a resolution arriving a round trip later is still recognised as the answer to that cast. That split is not tidiness — the `modalNextSpell` chain that starts a hunter's Auto Shot reads the record, and a record that died with the guard would break shooting. The buffer is not a latency figure, which is why upstream's default is 55 ms and not a ping: our packet reaches the server half a trip after the send, the server finishes its cast half a trip after our own cast time elapses, so a next cast sent at exactly `cast_time` arrives just as the server frees up and the latency cancels out of the arithmetic. The buffer is jitter margin on top. `NP_NonGcdBufferTimeMs` is wider because a non-GCD spell has nothing *but* the guard between two presses. **`NP_RetryServerRejectedSpells` is the mitigation for that**, and that is why upstream defaults it on. If the margin is ever wrong the server refuses the next cast and the press is simply gone — the dropped cast the early guard buys its speed with. It goes back once, held a buffer's worth, on exactly two codes: 0x3c not-ready and 0x61 another-action. Every other refusal is the server telling the player something true, and re-sending one would be a packet a second that never succeeds. **What a window measures, and how that was decided.** Upstream publishes four and no sentence saying. Two readings fit: how long an entry survives, or how close to the guard's end a press must land. This takes the first, because `queue.rs` already reads `NP_SpellQueueWindowMs` that way and one number meaning two things in one client is a trap nobody finds twice; because `Replacing queued nonGcd spell X with Y` means the last press wins, so holding every press costs nothing in staleness; and because the other reading would make the 250 ms cooldown window refuse almost every press made during a global cooldown, which is the complaint nampower exists to answer. Two rows do read a window as remaining time, and both say so themselves. `NP_InterruptChannelsOutsideQueueWindow`'s name states one. And `NP_CooldownQueueWindowMs` has to be one: a cast ends in a second, a Hearthstone cooldown is half an hour, and holding that press means either firing it half an hour later or swallowing it with no error line. A quarter of a second is an allowance for pressing a touch early, not a queue. **Where the rung sits.** Five rungs stay above it — the profession opener, the auto-repeat re-press, the dead caster, the targeting cursor's clear and the on-swing re-press — because each is what the press *means* whatever is in flight. Everything below is a real refusal a player should read: no reagent, out of range, no power, stunned, moving, wrong form. Upstream hooks the cast entry and holds those too, so its error arrives a second later about something the player stopped doing. Refusing them now is the deviation and the better half of the trade. Fourteen CVar rows went live. What stays stored is named with its cost: the dynamic buffer optimizer and its ceiling (off in upstream's own defaults), `NP_OnSwingBufferCooldownMs` (nothing to rate-limit — a re-press of the queued strike already takes the reference's silent bail), the channel latency trim, and the three quickcast rows, all off by default and all needing the world point under the mouse in the cast path. `CastSpellNoQueue`/`CastSpellByNameNoQueue` became meaningful: they take `CastLadder::send_now`, which skips the rung. The queue's own pop takes it too — a pop that could be re-held is a loop that never casts anything. The test worlds now seed the **registered** defaults rather than `Default`'s all-zeroes, which is a configuration nobody runs: three ladder tests were passing only because the queue was off in them. Each now asserts the queue behaviour and keeps the reference's 0x61 in a world where a player unticked the queue. And one test drives the whole cycle through the real drain — press held, nothing on the wire, guard cleared, one packet out — because the failure this guards against is a keypress that produced no packet, no error and no cast, which is the one a player cannot describe. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
94c571e3ea |
app: nampower's mount block at the buff cap — the first live press refusal
`NP_PreventMountingWhenBuffCapped` was the one gap in the nampower table named as a gap rather than a shrug: upstream defaults it ON, so a real install refuses a mount cast with all 32 positive aura slots taken instead of sending one the server accepts and then drops on the floor. With the slots full the mount aura has nowhere to land; vmangos runs the cooldown and the global cooldown and applies nothing, so the player stands still having spent a press, with no error to read because from the server's side nothing failed. The rule is `spell/np_press.rs`, a new module for nampower's press side — the rules that decide what a keypress does when the client would refuse it. It is separate from `queue.rs` because its inputs are not the queue's, and it is where the press-interception half will land. Three things worth naming: **The aura number is measured, not quoted.** `SPELL_AURA_MOUNTED = 78` is asserted against this realm's own `Spell.dbc` for Brown Horse, Summon Warhorse and Summon Felsteed, plus the converse on Frostbolt. A wrong constant here makes the refusal silently never fire, which no amount of play distinguishes from the row being off. **One deviation, and upstream names the bug itself.** Its settings label reads "Preventing mounting due to buff cap (breaks dismount)" — in 1.12 you dismount by casting the mount spell again, so a blanket block strands a buff-capped player on their mount. A press while `UNIT_FIELD_MOUNTDISPLAYID` is live is exempt here. One descriptor read, and it removes the only way this rule can make things worse. **Refused silently**, as upstream is: its own report is a line in its debug log, and 1.12's `SpellCastResult` has no code for "too many auras", so the only error line available would be a sentence about something else. What the player gets instead is the GCD and the packet both unspent, which is the whole point of refusing above the commit rather than below it. The ladder-level test asserts the fixture really reaches the cap before it asserts anything about the refusal — the id dwords alone leave every slot reading empty without the `AURAFLAGS` liveness nibble, and a fixture that never filled the slots would have passed for the wrong reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |
||
|
gromandClaude Opus 5
|
152788d894 |
app: seal a password only if it reads back, and keep tests out of the store
This cost grom his stored password once. Two unit tests ran at the same time: one redirected the config directory through `BENILLA_HOME`, which is process-wide, while the other stood up `LoginPlugin`, whose `build` had just gained a call to `migrate_plaintext`. So his real `autologin.toml` was upgraded against a key file that was not the one beside it — the plaintext was cleared and the blob replacing it does not decrypt. He types the password once more and `remember` stores it properly, since a blob that answers empty does not match and is resealed. Two fixes, and the second is the one that matters. The migration is now `#[cfg(not(test))]`: it was the only line in that plugin's build that writes to the player's own config directory, and no test has any business there. `autologin::sealed` encrypts and then **reads the blob back**, and refuses to clear the plaintext unless the round trip holds. That would have prevented this on its own, and it closes the general hole rather than this instance of it: `credential.rs` already carries a note about a scheme that round-trips with itself while being wrong, and this is the same mistake one level up — the cipher was right, the *key* was what disagreed, and the only readable copy of the password was thrown away before anything checked. A test drives the keep-it branch through the hermetic capture flag, the one state where sealing honestly cannot work, and asserts the plaintext is still there afterwards. An ignored `live_check` test reads the real store by hand and prints lengths only, never the password; it is what caught this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn |