attack panel

[evanpelle]

Relates #2260

Description:

Move outgoing & incoming boat & land attacks to a new "AttacksDisplay" layer that sits on top of the ControlPanel. The idea is to break up EventsDisplay so it's easier to find information. It's also more mobile friendly.

It still needs more styling, but this just a first pass.

Please complete the following:

• I have added screenshots for all UI updates
• I process any text displayed to the user through translateText() and I've added it to the en.json file
• I have added relevant tests to the test directory
• I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

evan

[coderabbitai]

Walkthrough

Adds a new AttacksDisplay UI layer mounted in the bottom‑right panel, moves attack/boat UI and logic out of EventsDisplay into that new layer, wires the layer in GameRenderer, and simplifies ControlPanel/bottom‑right container responsive classes.

Changes

Cohort / File(s)	Summary
Markup index.html	Inserted <attacks-display> into the bottom‑right panel container and added lg:max-w-[400px] to that container's classes.
Renderer integration src/client/graphics/GameRenderer.ts	Queries/imports attacks-display, validates it, attaches eventBus, game, and uiState, logs if missing, and registers it in the UI layers render order.
New attacks UI layer src/client/graphics/layers/AttacksDisplay.ts	Added AttacksDisplay LitElement implementing Layer: tick-driven state updates, visibility gating, collects/filters incoming/outgoing attacks and boats, resolves unit views, caches boat sprites, emits intents (cancel/send/go-to), and renders interactive attack/boat sections.
EventsDisplay simplification src/client/graphics/layers/EventsDisplay.ts	Removed attack/boat imports, state fields, per-tick attack processing, renderers, and related emitters/handlers; retained non-attack event logic and player/unit navigation emits.
Control panel styles src/client/graphics/layers/ControlPanel.ts	Removed landscape-specific Tailwind classes from the open-state class string; retained visibility logic and applied concise responsive sizing (lg:max-w-[400px]).
Localization resources/lang/en.json	Removed events_display.retaliate and events_display.boat keys.

Sequence Diagram(s)

sequenceDiagram
    participant Renderer as GameRenderer
    participant UI as AttacksDisplay
    participant EB as EventBus
    participant Game as GameState
    participant UIState as UIState

    Renderer->>UI: locate element & attach eventBus, game, uiState
    EB-->>UI: publish ticks / updates
    UI->>Game: query units/players/positions
    UI->>UIState: read local player & spawn phase
    UI->>EB: emit intents (CancelAttack, CancelBoat, SendAttackIntent, GoToPlayer/Unit)
    EB->>Game: forward intents for processing
    Game-->>Renderer: state changes trigger re-render
    Renderer->>UI: continue tick/render cycle

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A new corner shows the fight,
Boats and warnings in plain sight.
Renderer links the threads anew,
Panels moved and styles made true. ⚓✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'attack panel' is directly related to the main change: creating a new AttacksDisplay UI layer for managing attack and boat information.
Description check	✅ Passed	The description clearly explains the purpose of moving attack-related functionality from EventsDisplay to a new AttacksDisplay layer, mentions the related issue, and includes screenshots demonstrating the UI changes.

_{✏️ 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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 adding getTickIntervalMs() 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) inside layer.tick() methods. Instead, layers should expose a getTickIntervalMs() method and let the renderer schedule ticks based on wall-clock time.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/client/graphics/layers/AttacksDisplay.ts`:
- Around line 64-77: The loop in AttacksDisplay.ts is iterating
updates[GameUpdateType.UnitIncoming] which may be undefined and will throw;
update the code that reads updates (from this.game.updatesSinceLastTick()) to
guard before iterating GameUpdateType.UnitIncoming (e.g. check if updates &&
updates[GameUpdateType.UnitIncoming] exists or use optional chaining) so you
only for...of over a defined array of UnitIncomingUpdate, then keep the same
checks for event.playerID === myPlayer.smallID() and event.messageType ===
MessageType.NAVAL_INVASION_INBOUND before adding event.unitID to
this.incomingBoatIDs.

🧹 Nitpick comments (3)

src/client/graphics/layers/AttacksDisplay.ts (3)

96-108: Redundant requestUpdate() call at end of tick().

All the properties updated in tick() (incomingAttacks, outgoingAttacks, etc.) are decorated with @state(), so Lit already schedules an update when they change. The manual this.requestUpdate() on line 108 forces a double render cycle every tick.

Consider removing line 108 or, if you prefer explicit control, removing the @state() decorators and relying solely on requestUpdate().

---

117-148: renderButton is duplicated between AttacksDisplay and EventsDisplay.

Both classes have an identical renderButton helper with the same signature and logic. Consider extracting it into a shared utility to keep things DRY.

Also applies to: 102-133

---

176-189: Sprite cache key only uses owner ID, not unit appearance state.

getBoatSpriteDataURL caches by boat-${owner.id()}, but getColoredSprite (from SpriteLoader.ts) uses a composite key that includes territory color, border color, and spawn highlight. If a player's colors change during the game, the local cache here will serve a stale sprite. This is probably fine for boats in practice, but worth knowing.

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@src/client/graphics/layers/AttacksDisplay.ts`:
- Around line 162-166: The guard after casting in emitGoToPlayerEvent is
incorrect because playerBySmallID returns Player | TerraNullius (TerraNullius is
truthy); replace the truthy check with an explicit type guard using
attacker.isPlayer() and only call new GoToPlayerEvent(attacker) when isPlayer()
is true; apply the same change in handleRetaliate so both functions use
playerBySmallID(...) and then if (attacker.isPlayer()) { /* use attacker as
Player */ } to avoid treating TerraNullius as a Player.
- Around line 91-94: The filter currently unsafely casts
game.playerBySmallID(...) to PlayerView and calls .type(), which fails if it
returns TerraNullius (ID 0); change the predicate to first get const p =
this.game.playerBySmallID(a.attackerID) and only call p.type() when p.isPlayer()
is true, otherwise treat non-player (TerraNullius) as not a bot (or skip) —
update the incomingAttacks assignment (and any similar uses) to use this safe
check instead of the direct cast to PlayerView.
- Around line 47-51: AttacksDisplay.tick() is doing expensive per-frame work;
implement the optional getTickIntervalMs() on the AttacksDisplay class to
throttle updates (similar to ControlPanel) by returning a sensible interval
(e.g., 100–1000 ms) so the layer manager will call tick less frequently; locate
the AttacksDisplay class and add a getTickIntervalMs() method (matching the
Layer interface) that returns the chosen millisecond value to prevent
unnecessary continuous filtering/processing and requestUpdate() spamming.

🧹 Nitpick comments (2)

src/client/graphics/layers/AttacksDisplay.ts (2)

117-148: renderButton helper is duplicated from EventsDisplay.ts.

This helper (lines 117–148) is nearly identical to the one in EventsDisplay.ts (lines 102–133). Consider extracting it into a shared utility or mixin to keep things DRY.

---

183-203: attackWarningOnClick is async but does not await consistently.

The method is marked async and uses await playerView.attackAveragePosition(...). The click handler in renderOutgoingAttacks (line 289) also uses async () => this.attackWarningOnClick(attack), which correctly awaits the promise. The click handler in renderIncomingAttacks (line 244) uses a plain () => arrow — the returned promise is silently discarded. This is fine functionally but inconsistent. Consider making both handlers consistent.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/client/graphics/layers/ControlPanel.ts (1)

353-362: ⚠️ Potential issue | 🟡 Minor

Hardcoded "Attack Ratio" string not passed through translateText().

The PR checklist states all user-facing text goes through translateText() and en.json. This label on Line 361 is still a raw English string.

Proposed fix

-              ><img
+              >
+              <img
                 src=${swordIcon}
                 alt=""
                 aria-hidden="true"
                 width="14"
                 height="14"
                 style="filter: brightness(0) saturate(100%) invert(36%) sepia(95%) saturate(5500%) hue-rotate(350deg) brightness(95%) contrast(95%);"
-              />Attack Ratio</span
+              />${translateText("control_panel.attack_ratio")}</span

src/client/graphics/layers/EventsDisplay.ts (1)

778-787: ⚠️ Potential issue | 🟡 Minor

"Events" label not passed through translateText().

Line 780 has a hardcoded Events string. Same concern as with "Attack Ratio" in ControlPanel — the PR checklist requires user-facing text to go through translateText().

🧹 Nitpick comments (1)

src/client/graphics/layers/EventsDisplay.ts (1)

941-960: Empty-state logic uses a repeated inline IIFE for the traitor check.

The same (() => { ... myPlayer.isTraitor() ... })() pattern appears twice — once for the betrayal debuff timer row (Lines 924-930) and again here (Lines 943-950). Pull it into a local variable at the top of the render() method to keep things readable and DRY.

Proposed refactor

At the top of render(), after filteredEvents:

+ const isTraitorActive = (() => {
+   const myPlayer = this.game.myPlayer();
+   return myPlayer && myPlayer.isTraitor() && myPlayer.getTraitorRemainingTicks() > 0;
+ })();

Then replace both inline IIFEs with isTraitorActive.