Skip to content

Conversation

@Sgt-lewis
Copy link
Contributor

@Sgt-lewis Sgt-lewis commented Nov 4, 2025

Description:

Adds a win modal for OFM and the main discord. Adds needed translation keys and an OFM picture back to the resources file

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:

Iamlewis

@Sgt-lewis Sgt-lewis added this to the v27 milestone Nov 4, 2025
@Sgt-lewis Sgt-lewis self-assigned this Nov 4, 2025
@Sgt-lewis Sgt-lewis requested a review from a team as a code owner November 4, 2025 17:21
@CLAassistant
Copy link

CLAassistant commented Nov 4, 2025

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

Walkthrough

Adds six new localization keys for an OpenFront Masters Winter/Discord prompt and extends the WinModal component to pick between four display branches: steamWishlist, ofmDisplay, discordDisplay, or renderPatternButton, with iframe forcing steamWishlist.

Changes

Cohort / File(s) Change Summary
Win Modal Localization
resources/lang/en.json
Added six win_modal keys: ofm_winter, ofm_winter_description, join_tournament, join_discord, discord_description, join_server. Minor punctuation adjustment around existing wishlist entry.
Win Modal Component Logic
src/client/graphics/layers/WinModal.ts
Reworked innerHtml branching: if in iframe → steamWishlist(); else random branch selecting steamWishlist() for rand∈[0,0.25), ofmDisplay() for [0.25,0.5), discordDisplay() for [0.5,0.75), otherwise renderPatternButton(). Added public methods ofmDisplay(): TemplateResult and discordDisplay(): TemplateResult. Imported OfmWintersLogo asset and wired it into ofmDisplay.

Sequence Diagram(s)

sequenceDiagram
  participant WinModal
  participant Env as Environment (iframe?/rand)
  participant UI as UI Render

  rect rgba(50,115,220,0.06)
  Note over WinModal,Env: build innerHtml
  WinModal->>Env: check iframe?
  alt in iframe
    WinModal->>UI: steamWishlist()
    Note right of UI: Steam wishlist block
  else not iframe
    WinModal->>Env: generate rand (0..1)
    alt rand < 0.25
      WinModal->>UI: steamWishlist()
      Note right of UI: Steam wishlist (25%)
    else rand < 0.5
      WinModal->>UI: ofmDisplay()
      Note right of UI: OFM block (title, image, description, CTA)
    else rand < 0.75
      WinModal->>UI: discordDisplay()
      Note right of UI: Discord block (title, image, description, CTA)
    else
      WinModal->>UI: renderPatternButton()
      Note right of UI: Pattern/skin prompt (fallback)
    end
  end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Check localization keys and wording in resources/lang/en.json.
  • Verify ofmDisplay() and discordDisplay() templates for accessibility, escaping, and correct links/assets.
  • Confirm random thresholds and iframe gating behavior align with intended probabilities.

Possibly related PRs

Suggested labels

UI/UX

Poem

A tiny modal, options spun,
Wishlist, OFM, or chat in fun.
New keys added, a logo gleams,
Random nudge to join the teams. 🎉

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main changes: adding Discord and OFM advertisements to the win modal.
Description check ✅ Passed The description directly relates to the changeset, explaining what was added (win modal for OFM and Discord, translation keys, and OFM picture) and confirming completion of required steps.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fc9cd75 and eba9341.

⛔ Files ignored due to path filters (1)
  • resources/images/OfmWintersLogo.png is excluded by !**/*.png
📒 Files selected for processing (1)
  • src/client/graphics/layers/WinModal.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/client/graphics/layers/WinModal.ts

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

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/client/graphics/layers/WinModal.ts (1)

1-1: Run Prettier to fix formatting.

The pipeline indicates code style issues. Run Prettier to ensure consistent formatting.

#!/bin/bash
# Fix formatting issues
npx prettier --write src/client/graphics/layers/WinModal.ts
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c371112 and 4174db4.

⛔ Files ignored due to path filters (1)
  • resources/images/Ofm-Winters-Logo.png is excluded by !**/*.png
📒 Files selected for processing (2)
  • resources/lang/en.json (1 hunks)
  • src/client/graphics/layers/WinModal.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
📚 Learning: 2025-06-02T14:27:37.609Z
Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.

Applied to files:

  • resources/lang/en.json
📚 Learning: 2025-08-16T10:52:08.292Z
Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.

Applied to files:

  • resources/lang/en.json
📚 Learning: 2025-07-12T08:41:35.101Z
Learnt from: Aotumuri
Repo: openfrontio/OpenFrontIO PR: 1357
File: resources/lang/de.json:523-540
Timestamp: 2025-07-12T08:41:35.101Z
Learning: In OpenFrontIO project localization files, always check the en.json source file before flagging potential spelling errors in other language files, as some keys may intentionally use non-standard spellings that need to be consistent across all translations.

Applied to files:

  • resources/lang/en.json
📚 Learning: 2025-08-21T21:42:45.277Z
Learnt from: plazmaezio
Repo: openfrontio/OpenFrontIO PR: 1883
File: src/client/HostLobbyModal.ts:227-245
Timestamp: 2025-08-21T21:42:45.277Z
Learning: In openfrontio/OpenFrontIO, the en.json file contains difficulty translation keys using the descriptive names ("Relaxed", "Balanced", "Intense", "Impossible") rather than the enum values ("Easy", "Medium", "Hard", "Impossible"). The DifficultyDescription mapping in src/client/components/Difficulties.ts correctly maps enum values to these descriptive translation keys.

Applied to files:

  • resources/lang/en.json
📚 Learning: 2025-05-30T03:53:52.231Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 949
File: resources/lang/en.json:8-10
Timestamp: 2025-05-30T03:53:52.231Z
Learning: For the OpenFrontIO project, do not suggest updating translation files in resources/lang/*.json except for en.json. The project has a dedicated translation team that handles all other locale files.

Applied to files:

  • resources/lang/en.json
📚 Learning: 2025-08-27T08:12:19.610Z
Learnt from: mokizzz
Repo: openfrontio/OpenFrontIO PR: 1940
File: resources/lang/en.json:763-766
Timestamp: 2025-08-27T08:12:19.610Z
Learning: In OpenFrontIO, some country entries in src/client/data/countries.json may have similar names but different codes (e.g., "Empire of Japan" vs "Empire of Japan1"). Each unique code requires its own translation key in resources/lang/en.json after normalization. Always verify against countries.json before suggesting removal of translation keys.

Applied to files:

  • resources/lang/en.json
📚 Learning: 2025-08-19T11:00:55.422Z
Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 1864
File: resources/maps/arabianpeninsula/manifest.json:13-170
Timestamp: 2025-08-19T11:00:55.422Z
Learning: In OpenFrontIO, nation names in map manifests are displayed directly in the UI without translation. They do not need to be added to resources/lang/en.json or processed through translateText(). This is the established pattern across all existing maps including Europe, World, Asia, Africa, and others.

Applied to files:

  • resources/lang/en.json
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.

Applied to files:

  • src/client/graphics/layers/WinModal.ts
🧬 Code graph analysis (1)
src/client/graphics/layers/WinModal.ts (1)
src/client/Utils.ts (2)
  • isInIframe (239-247)
  • translateText (92-147)
🪛 Biome (2.1.2)
resources/lang/en.json

[error] 505-505: expected , but instead found {

Remove {

(parse)

🪛 GitHub Actions: 🧪 CI
resources/lang/en.json

[error] 503-503: Prettier check failed: SyntaxError: Unexpected token, expected "," (503:1).

src/client/graphics/layers/WinModal.ts

[warning] 1-1: Code style issues found in the file. Run Prettier with --write to fix.

🪛 GitHub Actions: 🚀 Deploy
resources/lang/en.json

[error] 503-503: npm run build-prod failed. Webpack error: Cannot parse JSON in resources/lang/en.json at position 24810 (line 503, column 1).

🔇 Additional comments (2)
src/client/graphics/layers/WinModal.ts (2)

102-114: Clean three-way display split.

The updated logic cleanly splits promotional content: Steam wishlist for iframes, and equal probability for OFM tournament, Discord community, or territory patterns otherwise. Using this.rand from construction ensures consistent display during the modal's lifetime.


228-247: Discord promotion display looks good.

The method properly uses translateText() for all user-facing strings and follows the same structure as ofmDisplay(). The indigo color scheme nicely differentiates it from the OFM promotion.

coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 4, 2025
return this.steamWishlist();
}

// 33% chance for each option
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we leave the steam page at 25% chance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/client/graphics/layers/WinModal.ts (1)

39-39: Consider randomizing on each modal display.

The rand value is set once when the WinModal instance is created (line 39), which means the same promotional display will appear every time this modal is shown during a game session.

If the intention is to randomize the display each time the modal appears (e.g., on death and then on win), consider moving the randomization to the show() method or innerHtml() method:

-  private rand = Math.random();
+  private getRand() {
+    return Math.random();
+  }

Then update innerHtml():

   innerHtml() {
     if (isInIframe()) {
       return this.steamWishlist();
     }
     
     // 25% chance for each display option
-    if (this.rand < 0.25) {
+    const rand = this.getRand();
+    if (rand < 0.25) {
       return this.steamWishlist();
-    } else if (this.rand < 0.5) {
+    } else if (rand < 0.5) {
       return this.ofmDisplay();
-    } else if (this.rand < 0.75) {
+    } else if (rand < 0.75) {
       return this.discordDisplay();
     }
     return this.renderPatternButton();
   }

If the current behavior (consistent display per session) is intentional, you can safely ignore this suggestion.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f725cc4 and fdd16d7.

📒 Files selected for processing (1)
  • src/client/graphics/layers/WinModal.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.

Applied to files:

  • src/client/graphics/layers/WinModal.ts
🧬 Code graph analysis (1)
src/client/graphics/layers/WinModal.ts (2)
src/client/Utils.ts (2)
  • isInIframe (239-247)
  • translateText (92-147)
src/client/LangSelector.ts (1)
  • translateText (254-274)
🪛 GitHub Actions: 🧪 CI
src/client/graphics/layers/WinModal.ts

[warning] 1-1: Prettier formatting check found issues in this file. Run 'prettier --write' to fix code style issues.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Deploy to openfront.dev
🔇 Additional comments (2)
src/client/graphics/layers/WinModal.ts (2)

202-228: Verify Discord link and translation keys.

The implementation looks clean and properly uses translateText() for localization. Please confirm:

  1. The Discord invite link https://discord.gg/wXXJshB8Jt is the correct OFM-specific server (differs from the main Discord link in discordDisplay()).
  2. The translation keys (win_modal.ofm_winter, win_modal.ofm_winter_description, win_modal.join_tournament) were added to all language files, not just en.json.

Based on learnings: The project typically separates technical changes from translation content updates, waiting for community translators to update values for non-English languages.


230-249: Verify translation keys were added.

The implementation is clean and properly uses translateText() for localization. Please confirm that the translation keys (win_modal.join_discord, win_modal.discord_description, win_modal.join_server) were added to all language files.

Based on learnings: The project typically separates technical changes from translation content updates, waiting for community translators to update values for non-English languages.

coderabbitai[bot]
coderabbitai bot previously approved these changes Nov 4, 2025
@evanpelle evanpelle merged commit bd4bbde into main Nov 4, 2025
12 of 13 checks passed
@evanpelle evanpelle deleted the discord-add branch November 4, 2025 23:56
evanpelle added a commit that referenced this pull request Nov 4, 2025
## Description:

Adds a win modal for OFM and the main discord. Adds needed translation
keys and an OFM picture back to the resources file

## Please complete the following:

- [x] I have added screenshots for all UI updates
- [x] I process any text displayed to the user through translateText()
and I've added it to the en.json file
- [x] I have added relevant tests to the test directory
- [x] 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:

Iamlewis

---------

Co-authored-by: evanpelle <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants