unit price#3989
Merged
Merged
Conversation
Contributor
WalkthroughBuild preview now displays an optional cursor cost label beneath the build site. The feature adds showCost and canAfford fields to GhostPreviewData, computes them in BuildPreviewController using user settings, passes them through the rendering system, and renders the label via a renamed WorldTextPass class with affordability-based coloring. ChangesGhost cost labeling
Sequence DiagramsequenceDiagram
participant BuildPreview as BuildPreviewController
participant Renderer as GPURenderer
participant WorldText as WorldTextPass
participant Screen as Screen
BuildPreview->>BuildPreview: compute cost, canAfford
BuildPreview->>Renderer: return GhostPreviewData with showCost, canAfford
Renderer->>Renderer: check showCost && cost > 0
Renderer->>WorldText: setGhostCostLabel({ tileX, tileY, cost, canAfford, canPlace })
WorldText->>WorldText: compute color (red/gray/white)
WorldText->>WorldText: tick() - layout cost text
WorldText->>WorldText: build glyph instances
Renderer->>WorldText: draw()
WorldText->>Screen: render cost label with affordability color
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client/render/gl/passes/ConquestPopupPass.ts (1)
495-498:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
clear()should also clear persistent ghost label state.Line 495 clears popup entries, but
ghostCostLabelremains set, so stale label state can surviveclear()calls.Proposed fix
clear(): void { this.active.length = 0; + this.ghostCostLabel = null; this.instanceCount = 0; }🤖 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/passes/ConquestPopupPass.ts` around lines 495 - 498, The clear() method currently zeroes active entries and instanceCount but leaves the persistent ghost label state (ghostCostLabel) intact; update ConquestPopupPass.clear() to also reset ghostCostLabel (e.g., set ghostCostLabel to undefined/null or its initial default) so stale ghost label state cannot persist after clear() is called.
🤖 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/ConquestPopupPass.ts`:
- Around line 40-45: Move the per-pass tuning constants GHOST_COST_Y_OFFSET,
GHOST_COST_SCALE, and GHOST_COST_OUTLINE_WIDTH out of ConquestPopupPass and into
render-settings.json as new keys (e.g., ghostCostYOff, ghostCostScale,
ghostCostOutlineWidth), then access them through the RenderSettings accessor
used elsewhere (e.g., RenderSettings.<property> or the existing getter pattern)
inside ConquestPopupPass instead of the hardcoded values so the pass reads
RenderSettings.ghostCostYOff, RenderSettings.ghostCostScale and
RenderSettings.ghostCostOutlineWidth at runtime.
---
Outside diff comments:
In `@src/client/render/gl/passes/ConquestPopupPass.ts`:
- Around line 495-498: The clear() method currently zeroes active entries and
instanceCount but leaves the persistent ghost label state (ghostCostLabel)
intact; update ConquestPopupPass.clear() to also reset ghostCostLabel (e.g., set
ghostCostLabel to undefined/null or its initial default) so stale ghost label
state cannot persist after clear() is called.
🪄 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: 849ca81b-c1c4-4244-b03e-d7ac7b76a3be
📒 Files selected for processing (5)
src/client/controllers/BuildPreviewController.tssrc/client/hud/GameRenderer.tssrc/client/render/gl/Renderer.tssrc/client/render/gl/passes/ConquestPopupPass.tssrc/client/render/types/Renderer.ts
Comment on lines
+40
to
+45
| /** Tiles below the ghost icon center for the cost label. */ | ||
| const GHOST_COST_Y_OFFSET = 3; | ||
| /** World-space font size — smaller than popups so it sits unobtrusively under the icon. */ | ||
| const GHOST_COST_SCALE = 4; | ||
| /** Matches player-name outline width for a consistent UI look. */ | ||
| const GHOST_COST_OUTLINE_WIDTH = 1.4; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Move ghost-cost tuning values into RenderSettings.
The new constants on Line 41, Line 43, and Line 45 are per-pass tuning values but are hardcoded in the pass.
As per coding guidelines All per-pass tuning constants must be defined in render-settings.json and accessed through RenderSettings, not hardcoded in pass classes.
🤖 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/passes/ConquestPopupPass.ts` around lines 40 - 45, Move
the per-pass tuning constants GHOST_COST_Y_OFFSET, GHOST_COST_SCALE, and
GHOST_COST_OUTLINE_WIDTH out of ConquestPopupPass and into render-settings.json
as new keys (e.g., ghostCostYOff, ghostCostScale, ghostCostOutlineWidth), then
access them through the RenderSettings accessor used elsewhere (e.g.,
RenderSettings.<property> or the existing getter pattern) inside
ConquestPopupPass instead of the hardcoded values so the pass reads
RenderSettings.ghostCostYOff, RenderSettings.ghostCostScale and
RenderSettings.ghostCostOutlineWidth at runtime.
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/client/render/gl/passes/WorldTextPass.ts (1)
41-46: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftMove ghost-cost tuning values to RenderSettings.
Line 42, Line 44, and Line 46 hardcode per-pass tuning values, and those same constants are consumed at Line 331 and Line 431-Line 432. Please source them from
render-settings.jsonthroughRenderSettingsinstead.Suggested direction
-const GHOST_COST_Y_OFFSET = 3; -const GHOST_COST_SCALE = 4; -const GHOST_COST_OUTLINE_WIDTH = 1.4; +// from RenderSettings.worldText.ghostCost*- y: label.tileY + GHOST_COST_Y_OFFSET, + y: label.tileY + this.settings.worldText.ghostCostYOffset,- this.instanceData[off + 8] = GHOST_COST_SCALE; - this.instanceData[off + 9] = GHOST_COST_OUTLINE_WIDTH; + this.instanceData[off + 8] = this.settings.worldText.ghostCostScale; + this.instanceData[off + 9] = + this.settings.worldText.ghostCostOutlineWidth;As per coding guidelines
All per-pass tuning constants must be defined in render-settings.json and accessed through RenderSettings, not hardcoded in pass classes.Also applies to: 331-332, 431-432
🤖 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/passes/WorldTextPass.ts` around lines 41 - 46, The three hardcoded tuning constants GHOST_COST_Y_OFFSET, GHOST_COST_SCALE, and GHOST_COST_OUTLINE_WIDTH in WorldTextPass must be removed and sourced from RenderSettings/read from render-settings.json; add corresponding keys (e.g. ghostCostYOffset, ghostCostScale, ghostCostOutlineWidth) to render-settings.json, expose them on the RenderSettings API, and replace all uses of the local constants (declaration site and the usages inside WorldTextPass where the constants are referenced) with lookups on RenderSettings (e.g. RenderSettings.ghostCostYOffset / RenderSettings.get('ghostCostYOffset')) so the pass no longer contains hardcoded tuning values.
🤖 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.
Duplicate comments:
In `@src/client/render/gl/passes/WorldTextPass.ts`:
- Around line 41-46: The three hardcoded tuning constants GHOST_COST_Y_OFFSET,
GHOST_COST_SCALE, and GHOST_COST_OUTLINE_WIDTH in WorldTextPass must be removed
and sourced from RenderSettings/read from render-settings.json; add
corresponding keys (e.g. ghostCostYOffset, ghostCostScale,
ghostCostOutlineWidth) to render-settings.json, expose them on the
RenderSettings API, and replace all uses of the local constants (declaration
site and the usages inside WorldTextPass where the constants are referenced)
with lookups on RenderSettings (e.g. RenderSettings.ghostCostYOffset /
RenderSettings.get('ghostCostYOffset')) so the pass no longer contains hardcoded
tuning values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e76a8a43-a7a3-4201-ba50-60b695d2405a
⛔ Files ignored due to path filters (2)
src/client/render/gl/shaders/world-text/world-text.frag.glslis excluded by!**/*.glslsrc/client/render/gl/shaders/world-text/world-text.vert.glslis excluded by!**/*.glsl
📒 Files selected for processing (5)
src/client/controllers/BuildPreviewController.tssrc/client/hud/GameRenderer.tssrc/client/render/gl/Renderer.tssrc/client/render/gl/passes/WorldTextPass.tssrc/client/render/types/Renderer.ts
github-project-automation
Bot
moved this from Development
to Complete
in OpenFront Release Management
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
Ghost structure cost label
Renders the gold cost of the currently-selected build under the ghost
structure cursor, with color-coded affordability/placement state. Honors the
existing
cursorCostLabeluser setting (legacy nameghostPricePill, alreadyshipping ON by default).
Behavior
The number is formatted via
renderNumber(project-wide convention —1.5K,1.23M, etc.) and rendered as MSDF text at a fixed world-space scale, centeredunder the ghost icon.
Implementation
The cost was already plumbed end-to-end on
GhostPreviewData.costbut nevervisualized. This PR:
GhostPreviewDatawithshowCost(from setting) andcanAfford(goldvs. cost check, computed in BuildPreviewController).
setGhostCostLabel(...)channel to the MSDF text pass — one persistent,non-animated text instance alongside the existing ephemeral popups. No new
pass, no new shader.
Renderer.updateGhostPreviewto push the label whenever a ghost is active.ConquestPopupPass→WorldTextPass(and its shader dirconquest-popup/→world-text/) since it now handles conquest popups,bonus popups, and the ghost cost label. Done with
git mvso history ispreserved.
Screen.Recording.2026-05-22.at.5.21.43.PM.mov
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
evan