Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions src/client/graphics/layers/PlayerPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,12 +491,17 @@ export class PlayerPanel extends LitElement implements Layer {
`;
}

private renderIdentityRow(other: PlayerView, my: PlayerView) {
const flagCode = other.cosmetics.flag;
const country =
typeof flagCode === "string"
? Countries.find((c) => c.code === flagCode)
: undefined;
private renderIdentityRow(
other: PlayerView,
my: PlayerView,
) {
const activeFlagPath = other.cosmetics.flag
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Missing semicolon on activeFlagPath declaration is still breaking Prettier CI.

Line 498 has no semicolon:

-    const activeFlagPath = other.cosmetics.flag
+    const activeFlagPath = other.cosmetics.flag;

Run npx prettier --write src/client/graphics/layers/PlayerPanel.ts to confirm all remaining formatting issues are cleared before merging.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const activeFlagPath = other.cosmetics.flag
const activeFlagPath = other.cosmetics.flag;
🤖 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/graphics/layers/PlayerPanel.ts` at line 498, The declaration for
activeFlagPath in PlayerPanel.ts is missing a trailing semicolon which breaks
Prettier CI; add the semicolon to the const declaration (the identifier
activeFlagPath where it is set to other.cosmetics.flag) and then run npx
prettier --write src/client/graphics/layers/PlayerPanel.ts (or your repo-wide
prettier) to reformat and ensure no other formatting issues remain before
merging.

const flagCode = activeFlagPath?.match(/\/([^.]+)\./)?.[1];

// Find the country based on that code
const country = flagCode
? Countries.find((c) => c.code === flagCode.split("/")[1])
Comment on lines +499 to +503
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find where cosmetics.flag is assigned or read to understand the path format
rg -n "cosmetics[.\[]" --type ts -A 2 -B 2 | grep -i flag | head -40

# Also check cosmetics.json for flag entry shapes
fd "cosmetics.json" --exec cat {}

Repository: openfrontio/OpenFrontIO

Length of output: 2640


🏁 Script executed:

# Check how the flag is used after extraction in PlayerPanel.ts
rg -n "flagCode" src/client/graphics/layers/PlayerPanel.ts -A 5 -B 2

# Check if there's validation for custom URLs elsewhere
rg -n "cosmetics.flag" src/client/graphics/layers/ -A 3 -B 1 | head -50

Repository: openfrontio/OpenFrontIO

Length of output: 2042


🏁 Script executed:

# Find Countries definition and what codes it contains
rg -n "Countries\s*=" src/client --type ts -A 5 | head -30

# See how 'country' variable is used after line 506 in PlayerPanel.ts
rg -n "const country = " src/client/graphics/layers/PlayerPanel.ts -A 10

Repository: openfrontio/OpenFrontIO

Length of output: 418


🏁 Script executed:

# Find the Countries import in PlayerPanel.ts
rg -n "^import|^from|Countries" src/client/graphics/layers/PlayerPanel.ts | head -20

# Find Countries definition in the entire codebase
rg -n "const Countries|export.*Countries" src/client --type ts -B 2 -A 3

Repository: openfrontio/OpenFrontIO

Length of output: 939


🏁 Script executed:

# Read the countries.json file to see code structure
cat resources/countries.json | head -50

Repository: openfrontio/OpenFrontIO

Length of output: 913


🏁 Script executed:

# Check GameView.ts flagAssignment and asset generation
rg -n "flag:" src/core/game/GameView.ts -B 3 -A 3

# Check if assetUrl or asset hashing changes the path before reaching PlayerPanel
rg -n "assetUrl|flagAssignments" src/client/graphics/layers/PlayerPanel.ts -B 2 -A 2

# Check core/AssetUrls to see how flag paths are transformed
cat src/core/AssetUrls.ts | head -40

Repository: openfrontio/OpenFrontIO

Length of output: 2500


Flag code extraction breaks for hashed asset paths—improve regex to handle hash suffixes.

The current regex /\/([^.]+)\./ incorrectly handles bundled assets. For /_assets/flags/us-abc123.svg (which appears after build hashing), it captures _assets/flags/us-abc123, then split("/")[1] yields flags instead of the country code us. The Countries.find() silently fails because no entry has code "flags".

The proposed regex /([^/]+)\.[^.]+$/ extracts just the filename but still includes the hash suffix: for us-abc123.svg it captures us-abc123, which also fails the country lookup. Strip the hash portion using a pattern like /([a-z]{2})(?:-[a-f0-9]+)?\.svg$/ to extract the 2-letter code before any hash:

Suggested refactor
-    const flagCode = activeFlagPath?.match(/\/([^.]+)\./)?.[1];
-
-    // Find the country based on that code
-    const country = flagCode
-      ? Countries.find((c) => c.code === flagCode.split("/")[1])
-      : flagCode;
+    // Extract the 2-letter code from paths like "/flags/us.svg" or "/_assets/flags/us-hash.svg"
+    const flagCode = activeFlagPath?.match(/([a-z]{2})(?:-[a-f0-9]+)?\.svg$/)?.[1];
+    const country = flagCode
+      ? Countries.find((c) => c.code === flagCode)
+      : undefined;
🤖 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/graphics/layers/PlayerPanel.ts` around lines 502 - 506, The flag
extraction logic around activeFlagPath and flagCode in PlayerPanel.ts fails for
hashed build asset names; update the matching to extract the 2-letter country
code before any hash instead of splitting on "/"—for example run a
case-insensitive regex match like /([a-z]{2})(?:-[a-f0-9]+)?\.svg$/i against
activeFlagPath and use the captured group as the flagCode, then pass that
flagCode directly into Countries.find((c) => c.code === flagCode); keep the safe
fallback when no match is found so country can be undefined.

Comment on lines +502 to +503
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to do this lookup? Can we do what NameLayer.ts does?

: flagCode;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const chip =
other.type() === PlayerType.Human
Expand All @@ -505,10 +510,11 @@ export class PlayerPanel extends LitElement implements Layer {

return html`
<div class="flex items-center gap-2.5 flex-wrap">
${country && typeof flagCode === "string"
${activeFlagPath
? html`<img
src=${assetUrl(`flags/${encodeURIComponent(flagCode)}.svg`)}
alt=${country?.name ?? "Flag"}
src=${activeFlagPath}
title=${typeof country !== "string" ? country?.name : country}
alt=${typeof country !== "string" ? country?.name : country}
class="h-10 w-10 rounded-full object-cover"
@error=${(e: Event) => {
(e.target as HTMLImageElement).style.display = "none";
Expand Down Expand Up @@ -946,7 +952,9 @@ export class PlayerPanel extends LitElement implements Layer {
class="p-6 flex flex-col gap-2 font-sans antialiased text-[14.5px] leading-relaxed"
>
<!-- Identity (flag, name, type, traitor, relation) -->
<div class="mb-1">${this.renderIdentityRow(other, my)}</div>
<div class="mb-1">
${this.renderIdentityRow(other, my)}
</div>

${this.sendTarget
? html`
Expand Down
Loading