feat(client): highlight small players with a pulsing glow#4525
Conversation
Boolean lobby option in host and single-player modals, whitelisted for live lobby updates. Client-only visual flag; the sim never reads it. Rendering follows in a later commit.
New SmallPlayerGlowPass draws a pulsing additive glow that radiates from the tiles of players in a highlight set (per-owner lookup texture + neighborhood falloff, so scattered fragments merge into a clean halo). WebGLFrameBuilder computes the set each tick: alive humans holding <=1% of the map, once past halftime (or 10 min uncapped). Gated fully off otherwise. Tunable via render-settings.json (radius/color/alpha/pulseSpeed).
…obby toggle Reverts the lobby GameConfig toggle (schema/server/modals) and instead exposes 'Highlight small players' as a client UserSetting in the in-game settings modal, toggleable live. Drops the halftime gate (a personal view shows immediately); still humans-only and <=1% of the map. The glow render pass is unchanged.
Threshold tightened to <=0.1% of the map, glow color changed to red, and the glow is suppressed for the first 30s of play (replaces any time-of-game tracking; avoids everyone's tiny starting territory glowing).
Glow now breathes: the reach grows/shrinks and the alpha fades fully to 0 at the trough for clear on/off contrast; base radius 5->8 (shader MAX_R 10), pulse a touch slower. Settings row uses a new distinct aura icon (HighlightIconWhite.svg) instead of the reused sword icon, and the description reflects the 0.2% threshold.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new “highlight small players” setting with translations, settings UI, render configuration, a WebGL glow pass, and per-tick logic that selects small players and forwards them to the renderer. ChangesSmall player glow highlight
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/client/render/gl/RenderSettings.ts (1)
383-388: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
colorbreaks from the file's established RGB convention.Every other color in this interface (e.g.
terrain,affiliation,bar,nukeTrajectory) uses separatexxxR/xxxG/xxxBscalar fields instead of an array.color: number[]is untyped (no length guarantee — could be[]or have 5 entries) and is the only array-shaped color in the whole struct.♻️ Suggested fix for consistency
smallPlayerGlow: { radius: number; // glow reach in tiles (shader MAX_R caps this at 6) - color: number[]; // RGB, each 0–1 + colorR: number; // RGB, each 0–1 + colorG: number; + colorB: number; alpha: number; // peak opacity (0–1) pulseSpeed: number; // breath animation speed };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/render/gl/RenderSettings.ts` around lines 383 - 388, `smallPlayerGlow` is the only RGB value in `RenderSettings` that uses an untyped array, unlike the rest of the interface’s `xxxR`/`xxxG`/`xxxB` scalar pattern. Update `smallPlayerGlow` to match the existing color convention used by fields like `terrain`, `affiliation`, `bar`, and `nukeTrajectory`, and adjust any corresponding consumers of `smallPlayerGlow` so they read separate RGB components instead of `color: number[]`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/client/render/gl/RenderSettings.ts`:
- Around line 383-388: `smallPlayerGlow` is the only RGB value in
`RenderSettings` that uses an untyped array, unlike the rest of the interface’s
`xxxR`/`xxxG`/`xxxB` scalar pattern. Update `smallPlayerGlow` to match the
existing color convention used by fields like `terrain`, `affiliation`, `bar`,
and `nukeTrajectory`, and adjust any corresponding consumers of
`smallPlayerGlow` so they read separate RGB components instead of `color:
number[]`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5123c304-d55c-43d0-8d94-aeb9bc29a46c
⛔ Files ignored due to path filters (3)
resources/images/HighlightIconWhite.svgis excluded by!**/*.svgsrc/client/render/gl/shaders/small-player-glow/small-player-glow.frag.glslis excluded by!**/*.glslsrc/client/render/gl/shaders/small-player-glow/small-player-glow.vert.glslis excluded by!**/*.glsl
📒 Files selected for processing (10)
resources/lang/en.jsonsrc/client/WebGLFrameBuilder.tssrc/client/hud/layers/SettingsModal.tssrc/client/render/gl/MapRenderer.tssrc/client/render/gl/RenderSettings.tssrc/client/render/gl/Renderer.tssrc/client/render/gl/passes/SmallPlayerGlowPass.tssrc/client/render/gl/render-settings.jsonsrc/core/game/UserSettings.tstests/UserSettings.test.ts
… -> composite) Replaces the per-fragment O(N^2) neighborhood search with the codebase's FalloutBloom-style pipeline: a sub-tile extract (block-scanned so sparse single tiles aren't missed), a shared separable Gaussian blur, and a camera-projected additive composite. Camera-independent (no shimmer), far cheaper, and scales to any aura size. Removes the dead single-pass shaders and the now-unused radius setting; reuses blur.frag.glsl and createMapQuad.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/render/gl/passes/SmallPlayerGlowPass.ts`:
- Around line 203-208: Clamp the elapsed time used in SmallPlayerGlowPass.draw
so a long pause does not make animTime jump; the issue is in the now/lastTime
update that feeds the pulse calculation. In SmallPlayerGlowPass, limit the delta
before adding it to this.animTime, using the existing pulse speed logic, so
resumes after the grace period or background tab stay smooth and the pulse
remains stable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ba1e196-b903-4af2-a40f-1daf06db19d9
⛔ Files ignored due to path filters (3)
src/client/render/gl/shaders/small-player-glow/composite.frag.glslis excluded by!**/*.glslsrc/client/render/gl/shaders/small-player-glow/composite.vert.glslis excluded by!**/*.glslsrc/client/render/gl/shaders/small-player-glow/extract.frag.glslis excluded by!**/*.glsl
📒 Files selected for processing (3)
src/client/render/gl/RenderSettings.tssrc/client/render/gl/passes/SmallPlayerGlowPass.tssrc/client/render/gl/render-settings.json
💤 Files with no reviewable changes (2)
- src/client/render/gl/render-settings.json
- src/client/render/gl/RenderSettings.ts
draw() now uses GlUtils toTarget/toScreen + RenderTarget instead of hand-rolled FBO bind/viewport/clear, DRYing to existing machinery. Trimmed excessive comments in the pass and extract shader. Deliberately not coupled to FalloutBloomPass: only ~15 lines of plumbing overlap, its extract/composite are domain-specific, and sharing would risk regressing a tuned, complex pass.
The extract+blur (map-size-dependent) now run only when the set changes (~1/s) via a dirty flag; the cached blurred aura is composited every frame for the pulse. Per-frame cost drops to just the screen-size composite, independent of map size.
evanpelle
left a comment
There was a problem hiding this comment.
from CLAUDE:
Correctness
- src/client/render/gl/passes/SmallPlayerGlowPass.ts:222 — Additive blend washes the glow out to white over bright terrain, exactly where a visibility aid is needed. draw() composites with gl.blendFunc(gl.ONE, gl.ONE) under a comment saying "(premultiplied)", but the shader's premultiplied alpha is ignored by pure additive blending. With the default color [1.0, 0.12, 0.1], the glow over stock near-white mountain terrain [230, 230, 230] clamps to (1.0, 1.0, 0.99) — a faint hueless brightening with no red at all, and fully invisible over anything at white. FalloutBloomPass uses (ONE, ONE_MINUS_SRC_ALPHA) (premultiplied-over), which with the identical shader output stays clearly red over the same terrain and is identical to additive over dark backgrounds — a strict improvement, and a one-line change since the shader already outputs premultiplied alpha.
- src/client/render/gl/passes/SmallPlayerGlowPass.ts:112 — Floor-sized bloom buffers never scan the last mapW%4 columns / mapH%4 rows, so an edge-dwelling small player can get zero glow. bw = floor(mapW/4) means the extract shader's block scan tops out at tile 4*bw - 1; its bounds check only clips overshoot, nothing covers the undershoot strip. All Normal-size maps happen to be multiples of 4, but Compact games play on map4x variants and dozens of those aren't (yellowsea 750×576, arctic 914×914, montreal 764×750, luna 654×1754, …). A small player whose remaining tiles sit in that 1–3-tile edge strip — the exact near-dead player this feature exists to find — silently gets no highlight, and the vUV = aPos/uMapSize composite is slightly stretched on those maps. Fix is Math.ceil for bw/bh. (FalloutBloomPass has the same floor, but a missing bloom strip there is cosmetic; here it defeats the feature.)
- src/client/WebGLFrameBuilder.ts:358 — Toggling the setting off while the game is paused leaves the glow pulsing indefinitely. The off-switch is only delivered through WebGLFrameBuilder.update(), which runs once per simulation tick, while the rAF loop keeps calling smallPlayerGlowPass.draw() every frame. Singleplayer pause stops ticks (LocalServer.endTurn returns while paused), and when the game was already paused before the settings modal opened, the modal's own pause/unpause is a no-op — so the toggle appears broken until unpause. Low severity (self-corrects on the first tick), but the code comment claiming the per-tick check keeps "toggling the setting off responsive" presumes ticks that aren't happening.
Cleanup
- src/client/render/gl/shaders/small-player-glow/composite.vert.glsl:1 — Byte-for-byte functional duplicate of fallout-bloom/composite.vert.glsl. The repo's shaders/shared/ directory exists for exactly this (this pass already imports three shared shaders). It's actually the third copy — day-night/fallout-composite.vert.glsl is another — so moving one to shared/ would consolidate all three.
- src/client/render/gl/passes/SmallPlayerGlowPass.ts:120 — createTarget() is a third near-verbatim copy of the RGBA8 texture+FBO boilerplate. The new code imports RenderTarget and toTarget() from GlUtils but hand-assembles the struct GlUtils owns; FalloutBloomPass and Renderer's sceneTarget do the same. A ~20-line createRenderTarget(gl, w, h) in GlUtils would serve all three exactly (migrating the old two sites is optional).
- src/client/render/gl/Renderer.ts:1263 — The glow pass has no passEnabled gate, unlike the FalloutBloomPass it explicitly mirrors. Minor consistency point: pe.falloutBloom exists so bloom cost can be disabled (and toggled in the debug layout panel); the new bloom pipeline can't be. Several overlay passes are similarly ungated, so this is parity polish, not a bug.
- src/client/render/gl/passes/SmallPlayerGlowPass.ts:29 — BLOOM_TILE_SCALE = 4 allocates 4× the buffer pixels of FalloutBloom's scale 8, eagerly, for a default-off feature. That's ~2–4 MB VRAM on large maps paid by every client at load. Eager constructor allocation is the codebase-wide pattern (fine as-is), and the fill-rate argument for scale 8 doesn't fully transfer since extract/blur only run ~1/s here — so this is just a question worth asking: does the heavily-blurred output actually need the finer mask?
Refuted along the way (no action needed)
- "Alive human with 0 tiles keeps the pass active" — impossible; isAlive() is literally _tiles.size > 0.
- "Aura rebuilt every second even when nothing changed" — the rebuild is necessary (territory shape comes from tileTex, which moves even when the set doesn't), and it's already ~60× cheaper than FalloutBloom's every-frame pipeline.
- "Rescan counter delays glow after grace period" — counter starts at 0, so the first eligible tick scans immediately; only an off→on re-toggle can see a ≤0.9s delay, mostly hidden behind the settings modal.
The two findings I'd act on before merge are #1 and #2 — both are one-line fixes (blendFunc and Math.ceil) that materially affect whether the feature works in its stated scenario.
…mplify copy Addresses maintainer review: - Composite uses premultiplied-over (ONE, ONE_MINUS_SRC_ALPHA) so the glow keeps its color over bright terrain instead of washing to white. - ceil (not floor) the bloom buffer so edge players on non-multiple-of-scale maps still glow. - Consolidate the 3 identical composite verts into shaders/shared/map-quad.vert.glsl (small-player-glow, fallout-bloom, fallout-light). - Extract GlUtils.createRenderTarget and use it in the glow pass. - Simplify the setting description; correct the throttle comment (toggle-off is tick-gated, deferred while paused).
|
Fixed the blend (premultiplied-over so it stays colored over bright terrain) and the ceil edge bug, consolidated the shared vert + added createRenderTarget, and corrected the paused-toggle comment. Left the passEnabled gate and BLOOM_TILE_SCALE=4 as-is parity polish, and the mask is nearly free now that extract/blur run ~1 Hz. Also took your description wording. |
Description:
Adds a "Highlight small players" client setting (in the in-game Settings menu). When it's on, human players holding 0.2% or less of the map get a soft red glow that breathes (a pulsing aura) around their territory, starting one minute into the game. This makes near-eliminated players easy to spot on a busy map, even when their territory is scattered in fragments or sitting under structures.
How it works:
SmallPlayerGlowPassrenders a tile-space bloom: extract a sub-tile mask of the small players' tiles, run a separable Gaussian blur, then composite the soft aura over the map additively. It's camera-independent (no shimmer when panning/zooming), so scattered tiles blur into one clean halo. The aura breathes: its intensity fades fully to 0 at the trough for clear contrast.WebGLFrameBuilder: alive human players withtilesOwned / (landTiles - fallout) <= 0.002(0.2%), suppressed during the spawn phase and the first minute.UserSettingsflag, toggleable live from the in-game Settings modal (with a new, distinct icon).render-settings.json(smallPlayerGlow: color / alpha / pulseSpeed).UI (glow fades in and out):


Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
zixer._