attack panel #3114
Draft
attack panel #3114
+490
−293
Conversation
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
Contributor
WalkthroughThe changes extract and separate attack and boat display functionality from EventsDisplay into a new dedicated AttacksDisplay component, integrated into the game's rendering pipeline via GameRenderer. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ 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: 2
🤖 Fix all issues with AI agents
In `@src/client/graphics/GameRenderer.ts`:
- Around line 127-136: The code logs when the "attacks-display" element is
missing but then dereferences attacksDisplay anyway; update the block around
attacksDisplay in GameRenderer.ts to guard before use: after querying const
attacksDisplay = ... check if (!(attacksDisplay instanceof AttacksDisplay)) then
handle the missing element (e.g., log error and return or skip assigning) so the
subsequent lines attacksDisplay.eventBus = eventBus, attacksDisplay.game = game,
and attacksDisplay.uiState = uiState only run when attacksDisplay is a valid
AttacksDisplay instance.
In `@src/client/graphics/layers/AttacksDisplay.ts`:
- Around line 229-409: Replace the Tailwind utility backdrop-blur-sm with
backdrop-blur-xs in all attack/boat display renderers in this file: update the
blur class in the incoming attacks block (inside the method that renders
incoming attacks), renderOutgoingAttacks, renderOutgoingLandAttacks, and
renderBoats so each div using "bg-gray-800/70 backdrop-blur-sm ..." becomes
"bg-gray-800/70 backdrop-blur-xs ..."; search for backdrop-blur-sm to find each
occurrence and ensure className strings passed to renderButton (and the boat
div) are updated consistently.
🧹 Nitpick comments (1)
src/client/graphics/layers/AttacksDisplay.ts (1)
49-109: Add a tick interval to avoid per-frame UI work.This layer updates every tick and always calls
requestUpdate(). Consider addinggetTickIntervalMs()so the renderer can throttle it on wall-clock time.🔧 Suggested change
tick() { this.active = true; ... this.requestUpdate(); } + getTickIntervalMs(): number { + return 200; + }Based on learnings: In this TypeScript game codebase, UI/layer updates should be wall-clock throttled (independent of simulation speed). Do not use tick-modulus gating like
if (game.ticks()%N===0)insidelayer.tick()methods. Instead, layers should expose agetTickIntervalMs()method and let the renderer schedule ticks based on wall-clock time.
Comment on lines
+127
to
+136
| const attacksDisplay = document.querySelector( | ||
| "attacks-display", | ||
| ) as AttacksDisplay; | ||
| if (!(attacksDisplay instanceof AttacksDisplay)) { | ||
| console.error("attacks display not found"); | ||
| } | ||
| attacksDisplay.eventBus = eventBus; | ||
| attacksDisplay.game = game; | ||
| attacksDisplay.uiState = uiState; | ||
|
|
Contributor
There was a problem hiding this comment.
Guard against missing attacks-display before use.
Right now it logs an error but still dereferences the element. That can throw if the DOM tag is missing.
🛡️ Suggested fix
- const attacksDisplay = document.querySelector(
- "attacks-display",
- ) as AttacksDisplay;
- if (!(attacksDisplay instanceof AttacksDisplay)) {
- console.error("attacks display not found");
- }
- attacksDisplay.eventBus = eventBus;
- attacksDisplay.game = game;
- attacksDisplay.uiState = uiState;
+ const attacksDisplay = document.querySelector(
+ "attacks-display",
+ ) as AttacksDisplay | null;
+ if (!(attacksDisplay instanceof AttacksDisplay)) {
+ console.error("attacks display not found");
+ } else {
+ attacksDisplay.eventBus = eventBus;
+ attacksDisplay.game = game;
+ attacksDisplay.uiState = uiState;
+ }🤖 Prompt for AI Agents
In `@src/client/graphics/GameRenderer.ts` around lines 127 - 136, The code logs
when the "attacks-display" element is missing but then dereferences
attacksDisplay anyway; update the block around attacksDisplay in GameRenderer.ts
to guard before use: after querying const attacksDisplay = ... check if
(!(attacksDisplay instanceof AttacksDisplay)) then handle the missing element
(e.g., log error and return or skip assigning) so the subsequent lines
attacksDisplay.eventBus = eventBus, attacksDisplay.game = game, and
attacksDisplay.uiState = uiState only run when attacksDisplay is a valid
AttacksDisplay instance.
Comment on lines
+229
to
+409
| <div class="flex flex-col gap-1"> | ||
| ${this.incomingAttacks.map( | ||
| (attack) => html` | ||
| <div | ||
| class="flex items-center gap-1 w-full bg-gray-800/70 backdrop-blur-sm rounded-lg px-2 py-1" | ||
| > | ||
| ${this.renderButton({ | ||
| content: html`<img | ||
| src="${swordIcon}" | ||
| class="h-4 w-4 inline-block" | ||
| style="filter: invert(1)" | ||
| /> | ||
| <span class="inline-block min-w-[3rem] text-right" | ||
| >${renderTroops(attack.troops)}</span | ||
| > | ||
| ${( | ||
| this.game.playerBySmallID(attack.attackerID) as PlayerView | ||
| )?.name()} | ||
| ${attack.retreating | ||
| ? `(${translateText("events_display.retreating")}...)` | ||
| : ""} `, | ||
| onClick: () => this.attackWarningOnClick(attack), | ||
| className: | ||
| "text-left text-red-400 inline-flex items-center gap-1", | ||
| translate: false, | ||
| })} | ||
| ${!attack.retreating | ||
| ? this.renderButton({ | ||
| content: html`<img | ||
| src="${swordIcon}" | ||
| class="h-4 w-4" | ||
| style="filter: brightness(0) saturate(100%) invert(27%) sepia(91%) saturate(4551%) hue-rotate(348deg) brightness(89%) contrast(97%)" | ||
| />`, | ||
| onClick: () => this.handleRetaliate(attack), | ||
| className: | ||
| "ml-auto inline-flex items-center justify-center cursor-pointer bg-red-900/50 hover:bg-red-800/70 rounded px-1.5 py-1 border border-red-700/50", | ||
| translate: false, | ||
| }) | ||
| : ""} | ||
| </div> | ||
| `, | ||
| )} | ||
| </div> | ||
| `; | ||
| } | ||
|
|
||
| private renderOutgoingAttacks() { | ||
| if (this.outgoingAttacks.length === 0) return html``; | ||
|
|
||
| return html` | ||
| <div class="flex flex-col gap-1"> | ||
| ${this.outgoingAttacks.map( | ||
| (attack) => html` | ||
| <div | ||
| class="flex items-center gap-1 w-full bg-gray-800/70 backdrop-blur-sm rounded-lg px-2 py-1" | ||
| > | ||
| ${this.renderButton({ | ||
| content: html`<img | ||
| src="${swordIcon}" | ||
| class="h-4 w-4 inline-block" | ||
| style="filter: invert(1)" | ||
| /> | ||
| <span class="inline-block min-w-[3rem] text-right" | ||
| >${renderTroops(attack.troops)}</span | ||
| > | ||
| ${( | ||
| this.game.playerBySmallID(attack.targetID) as PlayerView | ||
| )?.name()} `, | ||
| onClick: async () => this.attackWarningOnClick(attack), | ||
| className: | ||
| "text-left text-blue-400 inline-flex items-center gap-1", | ||
| translate: false, | ||
| })} | ||
| ${!attack.retreating | ||
| ? this.renderButton({ | ||
| content: "❌", | ||
| onClick: () => this.emitCancelAttackIntent(attack.id), | ||
| className: "ml-auto text-left shrink-0", | ||
| disabled: attack.retreating, | ||
| }) | ||
| : html`<span class="ml-auto shrink-0 text-blue-400" | ||
| >(${translateText("events_display.retreating")}...)</span | ||
| >`} | ||
| </div> | ||
| `, | ||
| )} | ||
| </div> | ||
| `; | ||
| } | ||
|
|
||
| private renderOutgoingLandAttacks() { | ||
| if (this.outgoingLandAttacks.length === 0) return html``; | ||
|
|
||
| return html` | ||
| <div class="flex flex-col gap-1"> | ||
| ${this.outgoingLandAttacks.map( | ||
| (landAttack) => html` | ||
| <div | ||
| class="flex items-center gap-1 w-full bg-gray-800/70 backdrop-blur-sm rounded-lg px-2 py-1" | ||
| > | ||
| ${this.renderButton({ | ||
| content: html`<img | ||
| src="${swordIcon}" | ||
| class="h-4 w-4 inline-block" | ||
| style="filter: invert(1)" | ||
| /> | ||
| <span class="inline-block min-w-[3rem] text-right" | ||
| >${renderTroops(landAttack.troops)}</span | ||
| > | ||
| ${translateText("help_modal.ui_wilderness")}`, | ||
| className: | ||
| "text-left text-gray-400 inline-flex items-center gap-1", | ||
| translate: false, | ||
| })} | ||
| ${!landAttack.retreating | ||
| ? this.renderButton({ | ||
| content: "❌", | ||
| onClick: () => this.emitCancelAttackIntent(landAttack.id), | ||
| className: "ml-auto text-left shrink-0", | ||
| disabled: landAttack.retreating, | ||
| }) | ||
| : html`<span class="ml-auto shrink-0 text-blue-400" | ||
| >(${translateText("events_display.retreating")}...)</span | ||
| >`} | ||
| </div> | ||
| `, | ||
| )} | ||
| </div> | ||
| `; | ||
| } | ||
|
|
||
| private getBoatTargetName(boat: UnitView): string { | ||
| const target = boat.targetTile(); | ||
| if (target === undefined) return ""; | ||
| const ownerID = this.game.ownerID(target); | ||
| if (ownerID === 0) return ""; | ||
| const player = this.game.playerBySmallID(ownerID) as PlayerView; | ||
| return player?.name() ?? ""; | ||
| } | ||
|
|
||
| private renderBoatIcon(boat: UnitView) { | ||
| const dataURL = this.getBoatSpriteDataURL(boat); | ||
| if (!dataURL) return html``; | ||
| return html`<img | ||
| src="${dataURL}" | ||
| class="h-5 w-5 inline-block" | ||
| style="image-rendering: pixelated" | ||
| />`; | ||
| } | ||
|
|
||
| private renderBoats() { | ||
| if (this.outgoingBoats.length === 0) return html``; | ||
|
|
||
| return html` | ||
| <div class="flex flex-col gap-1"> | ||
| ${this.outgoingBoats.map( | ||
| (boat) => html` | ||
| <div | ||
| class="flex items-center gap-1 w-full bg-gray-800/70 backdrop-blur-sm rounded-lg px-2 py-1" | ||
| > | ||
| ${this.renderButton({ | ||
| content: html`${this.renderBoatIcon(boat)} | ||
| <span class="inline-block min-w-[3rem] text-right" | ||
| >${renderTroops(boat.troops())}</span | ||
| > | ||
| ${this.getBoatTargetName(boat)}`, | ||
| onClick: () => this.emitGoToUnitEvent(boat), | ||
| className: | ||
| "text-left text-blue-400 inline-flex items-center gap-1", | ||
| translate: false, | ||
| })} | ||
| ${!boat.retreating() | ||
| ? this.renderButton({ | ||
| content: "❌", | ||
| onClick: () => this.emitBoatCancelIntent(boat.id()), | ||
| className: "ml-auto text-left shrink-0", | ||
| disabled: boat.retreating(), | ||
| }) | ||
| : html`<span class="ml-auto shrink-0 text-blue-400" | ||
| >(${translateText("events_display.retreating")}...)</span | ||
| >`} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and get its line count
if [ -f "src/client/graphics/layers/AttacksDisplay.ts" ]; then
wc -l "src/client/graphics/layers/AttacksDisplay.ts"
else
echo "File not found"
fiRepository: openfrontio/OpenFrontIO
Length of output: 115
🏁 Script executed:
# Find all occurrences of backdrop-blur-sm in the file
rg "backdrop-blur-sm" src/client/graphics/layers/AttacksDisplay.ts -nRepository: openfrontio/OpenFrontIO
Length of output: 616
🏁 Script executed:
# Check Tailwind CSS configuration
fd -e "tailwind" -e "tailwindrc" -e "tailwind.config" . --type f | head -20Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Look for package.json to check Tailwind version
fd "package.json" . --type f | head -5 | xargs -I {} bash -c 'echo "=== {} ===" && cat {}'Repository: openfrontio/OpenFrontIO
Length of output: 4540
🏁 Script executed:
# Check for any other blur utilities that might need updating
rg "(blur-sm|backdrop-blur[^-]|\sblur\s)" src/client/graphics/layers/AttacksDisplay.ts -nRepository: openfrontio/OpenFrontIO
Length of output: 616
🏁 Script executed:
# Verify no other blur utilities besides backdrop-blur-sm in the file
rg "blur" src/client/graphics/layers/AttacksDisplay.ts -n | grep -v "backdrop-blur-sm"Repository: openfrontio/OpenFrontIO
Length of output: 49
Update Tailwind blur utility names for v4.
The code uses backdrop-blur-sm in five places (lines 233, 283, 327, 387, 425). In Tailwind v4, this maps to backdrop-blur-xs. Update all occurrences.
Suggested replacements
- class="flex items-center gap-1 w-full bg-gray-800/70 backdrop-blur-sm rounded-lg px-2 py-1"
+ class="flex items-center gap-1 w-full bg-gray-800/70 backdrop-blur-xs rounded-lg px-2 py-1"🤖 Prompt for AI Agents
In `@src/client/graphics/layers/AttacksDisplay.ts` around lines 229 - 409, Replace
the Tailwind utility backdrop-blur-sm with backdrop-blur-xs in all attack/boat
display renderers in this file: update the blur class in the incoming attacks
block (inside the method that renders incoming attacks), renderOutgoingAttacks,
renderOutgoingLandAttacks, and renderBoats so each div using "bg-gray-800/70
backdrop-blur-sm ..." becomes "bg-gray-800/70 backdrop-blur-xs ..."; search for
backdrop-blur-sm to find each occurrence and ensure className strings passed to
renderButton (and the boat div) are updated consistently.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #(issue number)
Description:
Describe the PR.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
DISCORD_USERNAME