-
Notifications
You must be signed in to change notification settings - Fork 787
feat: Nuke util layer added; SAMs now change color and transparency based on nuke prediction. #2944
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughA new NukeRenderUtilLayer centralizes nuke trajectory, spawn selection, interceptor detection, and alliance-impact state. GameRenderer creates and injects it into dependent render layers (StructureIconsLayer, SAMRadiusLayer, NukeTrajectoryPreviewLayer), which now consume its state instead of recomputing trajectories or alliance checks. Changes
Sequence Diagram(s)sequenceDiagram
participant GR as GameRenderer
participant EB as EventBus
participant NR as NukeRenderUtilLayer
participant CV as CanvasLoop
participant SR as SAMRadiusLayer
participant SI as StructureIconsLayer
participant NT as NukeTrajectoryPreviewLayer
GR->>NR: new NukeRenderUtilLayer(game, eventBus, uiState, transform)
GR->>SR: construct(..., nukeRenderUtilLayer)
GR->>SI: construct(..., nukeRenderUtilLayer)
GR->>NT: construct(game, nukeRenderUtilLayer)
EB->>NR: MouseMove / GhostStructureChanged / SwapRocketDirection
NR->>NR: compute trajectory, spawnTile, interceptors, stressedPlayers
CV->>NR: tick() / renderLayer()
CV->>SR: render() -> NR.getInterceptingPlayers(), NR.isNukeGhostActive()
CV->>SI: render() -> NR.getAllianceStressedPlayers()
CV->>NT: render() -> NR.getTrajectoryInfo()
SR->>CV: draw SAM arcs (theme + nuke-mode)
SI->>CV: draw ghost icons (ally-stressed)
NT->>CV: draw trajectory preview
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/client/graphics/layers/NukeRenderUtilLayer.ts`:
- Around line 255-259: The early return in renderLayer (after checking
this.findTargetTile() === null || !this.spawnTile) leaves stale state in
this.allianceStressedPlayers and this.interceptingPlayers; update the block so
that before returning you also clear this.allianceStressedPlayers = [] and
this.interceptingPlayers = [] (and keep clearing this.trajectoryPoints = [] as
already done) so no old colors or intercept markers remain when there is no
valid target or spawnTile.
- Around line 54-64: When ghost structure changes (in the
GhostStructureChangedEvent handler) reset the cached target by setting
lastTargetTile = null (like SwapRocketDirectionEvent does) and also make
trajectoryPreviewTick resilient to stale async results by recording the
currentGhostStructure (or a request ID) before awaiting player.actions() and,
after the await, verify that currentGhostStructure (and any relevant request ID)
still matches before assigning spawnTile; update both the
GhostStructureChangedEvent handler (references: currentGhostStructure,
nukeGhostActive, lastTargetTile) and the async logic inside
trajectoryPreviewTick (references: player.actions(), spawnTile) — apply the same
guard for the other similar block noted (lines ~105-127).
🧹 Nitpick comments (1)
src/client/graphics/layers/SAMRadiusLayer.ts (1)
274-349: Check intercepting opacity for allies/self.
In nuke mode, friendly/self inline strokes stay translucent unlessstressedis true. IfgetInterceptingPlayers()can include allies or self, the intercepting cue becomes too subtle. Please confirm the set excludes them; if not, makeinterceptingforce full opacity. Also,nukeMode / stressed / interceptingare constant per SAM, so you can compute them once outside the arc loop to avoid repeated lookups.Possible tweak if intercepting can include allies/self
- if (a.owner.isMe()) { - ctx.strokeStyle = lineColorSelf.alpha(0.3).toRgbString(); - } else if (this.game.myPlayer()?.isFriendly(a.owner)) { - if (stressed) { - ctx.strokeStyle = lineColorStressed.toRgbString(); - } else { - ctx.strokeStyle = lineColorFriend.alpha(0.3).toRgbString(); - } - } else { - if (intercepting || stressed) { - ctx.strokeStyle = lineColorEnemy.toRgbString(); - } else { - ctx.strokeStyle = lineColorEnemy.alpha(0.3).toRgbString(); - } - } + if (a.owner.isMe()) { + ctx.strokeStyle = ( + intercepting ? lineColorSelf : lineColorSelf.alpha(0.3) + ).toRgbString(); + } else if (this.game.myPlayer()?.isFriendly(a.owner)) { + if (stressed) { + ctx.strokeStyle = lineColorStressed.toRgbString(); + } else if (intercepting) { + ctx.strokeStyle = lineColorFriend.toRgbString(); + } else { + ctx.strokeStyle = lineColorFriend.alpha(0.3).toRgbString(); + } + } else { + if (intercepting || stressed) { + ctx.strokeStyle = lineColorEnemy.toRgbString(); + } else { + ctx.strokeStyle = lineColorEnemy.alpha(0.3).toRgbString(); + } + }
… into sam-nuke-colors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/client/graphics/layers/NukeRenderUtilLayer.ts`:
- Around line 273-276: When the early return after calling this.game.myPlayer()
executes, ensure you clear any lingering state to avoid stale data: reset
this.allianceStressedPlayers and any trajectory-related state (e.g.
this.trajectory*, trajectoryPoints, collision/preview state) before returning.
Update the method in NukeRenderUtilLayer where you call this.game.myPlayer() so
that on !player you explicitly clear those fields and then return.
🧹 Nitpick comments (1)
src/client/graphics/layers/NukeRenderUtilLayer.ts (1)
267-267: Prefix unused parameter with underscore.The
contextparameter is intentionally unused since this is a utility layer that doesn't draw. Prefix it with underscore to signal intent and silence linter warnings.Suggested fix
- renderLayer(context: CanvasRenderingContext2D) { + renderLayer(_context: CanvasRenderingContext2D) {
evanpelle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make the friendly SAMs yellow, since that's what we use for friendly trade ships
| new SAMRadiusLayer(game, eventBus, uiState, nukeRenderUtilLayer), | ||
| new NukeTrajectoryPreviewLayer(game, nukeRenderUtilLayer), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be a bit cleaner to merge these two layers into one layer, and then that layer has a nukeRenderUtil layer that it calls into?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Structure Icon Layer was also running more or less the same logic, so there are three layers that all require the same calculation. This also makes it easy if I want to add more layers as well needing it as well. (such as I might wanna work on showing which structures in StructureLayer get destroyed by the nuke).
Description:
Adds a new layer, NukeRenderUtilLayer, which does not draw anything but consolidates all nuke prediction code to be used by trajectory drawing layer, SAM radii drawing layer, and structure icon layer rather than recalculated each time. The layer is calculated first, and a reference of this layer is passed to all layers needing it.
pr_ex.1.mp4
SAM radii now change color/transparency when in "nuke mode":
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
bibizu