Clear ghost structure when launching atom/hydrogen bomb if user runs out of gold#3380
Clear ghost structure when launching atom/hydrogen bomb if user runs out of gold#3380wozniakpl wants to merge 5 commits intoopenfrontio:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds state and affordability checks to clear ghosted nuclear structures when the player cannot afford them, integrates those checks into render and placement flows, and adds tests for the new helper functions and scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant StructureIconsLayer
participant BuildableUnit
participant Player
Client->>StructureIconsLayer: renderGhost(ghostType, unit, player)
StructureIconsLayer->>BuildableUnit: read unit.canBuild, unit.cost
StructureIconsLayer->>StructureIconsLayer: shouldClearNukeGhostInRenderWhenOutOfGold(hasPlacedNukeWithCurrentGhost, ghostType, unit, player)
StructureIconsLayer->>StructureIconsLayer: shouldClearNukeGhostWhenOutOfGold(ghostType, unit, player)
StructureIconsLayer->>Player: player.gold() (if player)
alt unaffordable nuke & hasPlacedNukeWithCurrentGhost
StructureIconsLayer->>StructureIconsLayer: clearGhostStructure()
StructureIconsLayer-->>Client: stop rendering ghost
else affordable or non-nuke or not placed
StructureIconsLayer-->>Client: continue ghost rendering (maybe red outline)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/client/graphics/layers/StructureIconsLayer.test.ts (1)
98-106: Good edge case test for TileRef vs false.Using
0 as BuildableUnit["canBuild"]correctly tests that strict equality!== falsedistinguishes between a valid TileRef (even if 0) and the literalfalse. A brief inline comment could make this clearer for future readers:💡 Optional: add clarifying comment
test("returns false when unit canBuild is not false (e.g. can place)", () => { const unit = nukeBuildable(UnitType.AtomBomb, { + // 0 is a valid TileRef; strict equality (canBuild !== false) distinguishes it from literal `false` canBuild: 0 as BuildableUnit["canBuild"], });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/client/graphics/layers/StructureIconsLayer.test.ts` around lines 98 - 106, The test correctly uses 0 as BuildableUnit["canBuild"] to ensure strict inequality against false; update the test surrounding shouldClearNukeGhostWhenOutOfGold (the case using nukeBuildable and UnitType.AtomBomb) to include a short inline comment explaining that 0 is a valid TileRef and not the boolean false, referencing the canBuild field to clarify intent for future readers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/client/graphics/layers/StructureIconsLayer.test.ts`:
- Around line 98-106: The test correctly uses 0 as BuildableUnit["canBuild"] to
ensure strict inequality against false; update the test surrounding
shouldClearNukeGhostWhenOutOfGold (the case using nukeBuildable and
UnitType.AtomBomb) to include a short inline comment explaining that 0 is a
valid TileRef and not the boolean false, referencing the canBuild field to
clarify intent for future readers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ee7354b7-aa90-4360-a894-85010fc87f6e
📒 Files selected for processing (2)
src/client/graphics/layers/StructureIconsLayer.tstests/client/graphics/layers/StructureIconsLayer.test.ts
evanpelle
left a comment
evanpelle
left a comment
There was a problem hiding this comment.
I noticed a problem, if i don't have enough for an atom bomb, then pressing "8" just flashes and disaappears.
I'm thinking we should only check after click? I'm imagining it like:
- click 8
- launch a bunch of atom bombs
- run out of money
- nuke preview exits
…d after placing - Do not clear nuke preview in render path when user has not placed a nuke yet (press 8 with no gold shows red outline, no flash) - Clear nuke ghost only when user has placed at least one nuke and then runs out of gold (preview exits after last affordable nuke) - Add hasPlacedNukeWithCurrentGhost flag, shouldClearNukeGhostInRenderWhenOutOfGold - Simplify createStructure ghost-keep logic with keepGhostForMultiplePlacement - Add tests for both no-flash and clear-when-out-of-gold cases
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/graphics/layers/StructureIconsLayer.ts`:
- Around line 140-141: The hasPlacedNukeWithCurrentGhost flag can become stale
because uiState.ghostStructure is changed elsewhere; modify the shared
ghost-selection/change path to clear this flag whenever the ghost is changed
(i.e., whenever uiState.ghostStructure is set/cleared), not only in
clearGhostStructure(); locate the central function or handler that updates
uiState.ghostStructure (also update the other similar spot referenced around
600-602) and add a reset of hasPlacedNukeWithCurrentGhost = false there so a
newly-selected ghost is never treated as already-placed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4110464b-6b2b-400d-a477-38d52aa70510
📒 Files selected for processing (2)
src/client/graphics/layers/StructureIconsLayer.tstests/client/graphics/layers/StructureIconsLayer.test.ts
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
Covered in |
| if (ghostType !== UnitType.AtomBomb && ghostType !== UnitType.HydrogenBomb) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
I was playing the beta tests today, I actually think it should dissappear when you can't build a nuke, regardless of wether your silo is reloading or you run out of money.
There was a problem hiding this comment.
I mean I quite like to have the ghost structure (as do many others) to know how to plan the bomb,
also if I am like 20k gold off, I wanna get my bomb ready and send it the second I get enough
There was a problem hiding this comment.
I think you should be able to open the ghost structure, it should just disappear after you shoot the atom bomb and can't shoot another one
3 participants
Description:
Summary
When the atom or hydrogen bomb ghost is active and the player no longer has enough gold to place one, the ghost is now cleared instead of staying on screen with a red "can't build" outline.
Problem
If the user selected atom/hydrogen bomb placement and then ran out of gold (e.g. gold spent elsewhere or lost), the ghost stayed visible with a red outline. That was confusing because the user couldn't place a bomb but the UI still suggested the action was available.
Solution
buildables()returnscanBuild === falsefor the current ghost, we check whether the ghost is a nuke (atom/hydrogen bomb) and the player's gold is below the build cost.removeGhostStructure()so the ghost is cleared and the nuke "placement mode" is exited.Changes
src/client/graphics/layers/StructureIconsLayer.tsshouldClearNukeGhostWhenOutOfGold(ghostType, unit, player)and use it inrenderGhost()whenunit.canBuild === falseto clear the ghost when the player can't afford the nuke.tests/client/graphics/layers/StructureIconsLayer.test.tsshouldClearNukeGhostWhenOutOfGold(nuke + insufficient gold → clear; can afford / non-nuke / can place / null player / undefined cost → no clear).Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
.wozniakpl