Smooth nuke point-light position per frame in ambient mode - #4311
Conversation
Follow-up to #4255: nuke sprites glide per render frame (UnitPass lerps lastPos→pos), but the glow behind them comes from PointLightPass, whose instance buffer is packed once per tick from raw unit.pos. Its per-frame draw() never repositioned the lights, so in ambient/night mode the light jumped once per tick while the sprite moved at 60fps. Mirror UnitPass's smoothing in PointLightPass: record (lightIdx, lastX, lastY, x, y) segments for SMOOTHED_NUKE_TYPES in updateLights(), then lerp them by wall-clock tick progress in draw() and re-upload only the affected instances (the light buffer holds thousands of static structure lights, so a full per-frame re-upload would be wasteful). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
WalkthroughA new ChangesNuke and light position smoothing refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/client/render/gl/UnitPassSmoothing.test.ts (1)
34-74: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftUse
setup()helper instead of manualUnitPassprototype wiring in tests.This harness directly constructs
UnitPassviaObject.create(...)and custom field injection, which bypasses the required test setup path fortests/**/*.test.ts. Please migrate this harness to usetests/util/Setup.tsand drive smoothing through the simulation-oriented setup flow.As per coding guidelines: “
tests/**/*.test.ts: Use thesetup()helper fromtests/util/Setup.tsto create test game instances and exercise core simulation directly.”🤖 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 `@tests/client/render/gl/UnitPassSmoothing.test.ts` around lines 34 - 74, The makeSmoothingHarness function manually constructs a UnitPass instance using Object.create(UnitPass.prototype) and Object.assign to inject properties, which bypasses the required test setup path. Replace this manual prototype wiring with a call to the setup() helper from tests/util/Setup.ts to create a properly initialized test game instance. Update makeSmoothingHarness to use the simulation-oriented setup flow, which will ensure the UnitPass is correctly initialized through the standard testing infrastructure rather than bypassing it with direct property injection.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@tests/client/render/gl/UnitPassSmoothing.test.ts`:
- Around line 34-74: The makeSmoothingHarness function manually constructs a
UnitPass instance using Object.create(UnitPass.prototype) and Object.assign to
inject properties, which bypasses the required test setup path. Replace this
manual prototype wiring with a call to the setup() helper from
tests/util/Setup.ts to create a properly initialized test game instance. Update
makeSmoothingHarness to use the simulation-oriented setup flow, which will
ensure the UnitPass is correctly initialized through the standard testing
infrastructure rather than bypassing it with direct property injection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab1f08d2-033f-417b-993d-ba26b0444a01
📒 Files selected for processing (4)
src/client/render/gl/passes/PointLightPass.tssrc/client/render/gl/passes/UnitPass.tssrc/client/render/gl/utils/NukeSmoothing.tstests/client/render/gl/UnitPassSmoothing.test.ts
ebb1b08 to
af930dc
Compare
Summary
Follow-up to #4255. That PR made nuke sprites glide per render frame —
UnitPass.drawMissileslerps each nuke'slastPos→posby wall-clock progress through the current tick. But in ambient/night mode the glow behind a nuke comes from a separate pass,PointLightPass, whose instance buffer is packed once per tick inupdateLights()from the rawunit.pos. Its per-framedraw()(run every frame viaLightmapPass) only set uniforms and issued the instanced draw — it never repositioned the lights. So the sprite moved at 60fps while its light jumped once per 100ms tick.Fix
Mirror
UnitPass's smoothing inPointLightPass:updateLights()records asmoothSegstuple(lightIdx, lastX, lastY, x, y)for eachSMOOTHED_NUKE_TYPESunit whoselastPos !== pos, and stampslastUnitsUpdateMs.applySmoothing(), called at the top ofdraw(), lerps those lights by wall-clock tick progress ((now - lastUnitsUpdateMs) / tickIntervalMs, clamped to 1) and re-uploads only the affected instances. UnlikeUnitPass(which re-uploads its tiny missile buffer wholesale), the light buffer can hold thousands of static structure lights, so a full per-frame re-upload would be wasteful.tickIntervalMscomes from a newconfigconstructor param, wired through inRenderer.ts(the sameconfigalready passed toUnitPass).The light now uses the exact same
lastPos→posendpoints and alpha as the sprite, so the two track together.Test plan
npx tsc --noEmit, eslint, and prettier all clean.npx vitest tests/client/render --run— 40 passed.🤖 Generated with Claude Code