Add dev feature flags and settings UI improvements #2685
Conversation
- Add Config.ts with feature flag system for analytics, publicLobbies, cloudflare, ads - Remove external scripts from index.html (Cloudflare Turnstile, ads, analytics) - Add feature flag checks in Main.ts and PublicLobby.ts - Fix lit import path in PlayerInfoOverlay.ts (lit-html -> lit) - Add skipLibCheck and jest types to tsconfig.json - Switch from npm to pnpm in package.json scripts - Add config.example.json template and .gitignore entries 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Shift+drag no longer pans the map, reserving it for future unit selection - Change Toggle View default keybind from Space to Backslash - Remove spacePlusDrag setting (no longer needed) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add enablePublicGames() to ServerConfig interface - Default true, set ENABLE_PUBLIC_GAMES=false to disable - Skip public game scheduling loop in Master.ts when disabled - Create .env with ENABLE_PUBLIC_GAMES=false for local dev - Add .env to .gitignore Reduces server log noise during local development. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds a config-based feature flag system to disable external services (analytics, Cloudflare/Turnstile, ads, public lobbies) for local dev. - Add DevConfig.ts with feature flag loading from config.json - Add cloudflare feature check in getTurnstileToken() - Add publicLobbies feature check in PublicLobby component - Add enablePublicGames() server config to disable lobby scheduling - Add config.example.json template 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds OPENFRONT_CLIENT_PORT and OPENFRONT_SERVER_PORT env vars to allow customizing ports for local development. Defaults remain unchanged (client: 9000, server: 3000). - Master.ts reads OPENFRONT_SERVER_PORT - webpack.config.js reads both vars via dotenv and updates all proxy targets 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds a config-based feature flag system to disable external services (analytics, Cloudflare/Turnstile, ads, public lobbies) for local dev. Client-side: - Add DevConfig.ts with feature flag loading from config.json - Add cloudflare feature check in getTurnstileToken() - Add publicLobbies feature check in PublicLobby component - Add config.example.json template Server-side: - Add enablePublicGames() to ServerConfig interface - Add ENABLE_PUBLIC_GAMES env var support (defaults to true) - Skip public game scheduling when disabled 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Load config.json synchronously in HTML before external scripts - Conditionally load Turnstile, ads, and analytics based on config - Fix PublicLobby to wait for config before starting lobby polling - Auto-load DevConfig at module init to prevent race conditions - Remove duplicate enablePublicGames() in TestServerConfig 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Local-only changes not intended for upstream: - stop:dev script to kill dev processes on configured ports - .project/ folder with ports config and Claude instructions - .claude/ folder with task tracking and PR notes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add new SettingGroup component with collapsible panels and localStorage state - Reorganize UserSettingModal into logical groups: Display, Graphics, Controls, Privacy - Keybinds tab now uses collapsible groups: View, Build, Attack, Camera - Widen settings modal to 720px to accommodate 2-column layout - Add settings schema to config.example.json for dev defaults - Update DevConfig.ts to apply config.json settings to localStorage 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add new Display tab to settings modal as first tab - Add theme mode selector with light/dark/system options - Add themeMode to UserSettings with system preference detection - Add placeholder sections for Territory Skins and Colors with cosmetics note - Rename Basic Settings tab to Settings, rename Display group to Interface - Add SettingThemeMode component with visual theme selection - Update config.example.json to use themeMode instead of darkMode 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move Display tab to last position (Settings | Keybinds | Display) - Default to Settings tab on open - Set modal header background to rgba(0,0,0,0.9) - Set modal content background to rgba(35,35,35,0.7) - Fix modal width to 720px for consistent sizing across all tabs - Add responsive breakpoint for smaller screens 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Switch from CSS Grid to CSS columns layout for 2-column mode, creating a true masonry effect where items flow vertically first. This eliminates vertical gaps between cards of different heights. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Ensure consistent 12px vertical gap between all cards by applying margin-bottom and inline-block display to slotted items in columns mode. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace legacy darkMode check with applyTheme() call to properly handle light/dark/system theme modes on page load. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Create SettingColorPalette component with: - HSL color picker with saturation/brightness gradient - Hue slider for color selection - Hex color input field - Grid of named color swatches for theme elements (Primary, Secondary, Territory, Border, Enemy, Ally, Neutral, Background) - Each swatch has name and description - Disabled state with overlay badge for cosmetics module 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reduce component sizes for better fit: - Smaller color picker (160px width, 100px height preview) - Smaller hue slider (12px height) - Compact swatch items (24px color squares, smaller text) - Reduced padding and gaps throughout - Added min-width: 0 for proper flex overflow handling 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add .settings-tabs class with sticky positioning so tabs stay visible when scrolling through settings content. Uses semi-transparent background with backdrop blur for visual consistency. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix vertical gap between modal header and sticky tabs by adjusting top offset to -1.4rem - Reduce padding and margin for tighter layout - Organize config.example.json settings into logical groups with comments: Display, Interface, Graphics, Controls, Privacy, Audio - Update themeMode from legacy darkMode 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update DevConfig.ts with grouped interfaces (display, interface, graphics, controls, privacy, audio) - Maintain backwards compatibility for flat legacy settings - Update config.example.json with nested sub-objects - Fix sticky tabs to span full width with proper box-sizing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create SettingTerritorySkins component with preview and skin grid - Left column shows large preview with Upload/Save buttons - Right column shows scrollable grid of available skins - Premium skins marked with star badge - Disabled state with "Cosmetics Module Required" overlay - Replace placeholder in UserSettingModal with new component 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add light theme CSS variables in Modal.ts for child components - Update SettingGroup with theme-aware background variables - Fix dark mode button to use isDarkModeActive() for correct state - Sync theme changes between settings modal and homescreen button - Add event listeners for cross-component theme synchronization - Remove emoji icons from setting labels in all language files - Update tab styling with vertical dividers and theme-aware colors - Reorder Interface and Graphics settings for better layout 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove internal dev files from tracking (.claude/, .project/) - Remove duplicate/unused src/client/Config.ts - Remove package-lock.json (use pnpm-lock.yaml) - Remove debug console.log statements - Restore original script names (dev, dev:staging, dev:prod) - Update .gitignore for local dev files 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
WalkthroughThis pull request introduces a development configuration system with runtime feature flags, refactors the settings user interface with a new tabbed layout and theme mode selector, adds web components for managing colors and territory skins, and enables dynamic port configuration via environment variables. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (3 passed)
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.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resources/lang/fi.json (1)
310-314: Finnish translation file edited outside translation workflowThis is a cosmetic label tweak, but
fi.jsonis a non‑English locale. Normal practice is to update onlyen.jsonin feature PRs and leave other locales to the translation pipeline. Please verify with maintainers whether this should stay in this PR or be reverted and handled via an"mls"translation PR.Based on learnings, non‑English
resources/lang/*.jsonare owned by the translation team.
♻️ Duplicate comments (4)
resources/lang/nl.json (1)
330-333: Same locale policy concern as other files.Similar to the Hindi locale file, these structural emoji removals in the Dutch translations should be verified against the project's localization workflow policy.
Based on learnings, translation files should generally only be updated in dedicated "mls" PRs.
resources/lang/he.json (1)
268-272: Duplicate locale policy concern.Same verification needed as in Hindi and Dutch locale files.
resources/lang/sk.json (1)
296-299: Duplicate locale policy concern.Same verification needed as in other locale files regarding the policy for structural changes.
resources/lang/ru.json (1)
330-333: Duplicate locale policy concern.Same verification needed as in other locale files regarding the policy for structural changes.
🧹 Nitpick comments (5)
src/client/components/baseComponents/setting/SettingTerritorySkins.ts (1)
325-344: Consider adding file size validation for uploads.Storing large images as base64 data URLs in localStorage can quickly exhaust the ~5MB storage limit. A size check would prevent unexpected failures.
Optional: Add file size check
private handleUploadClick() { if (this.disabled) return; const input = document.createElement("input"); input.type = "file"; input.accept = "image/*"; input.onchange = (e) => { const file = (e.target as HTMLInputElement).files?.[0]; - if (file) { + if (file) { + const MAX_SIZE = 512 * 1024; // 512KB limit + if (file.size > MAX_SIZE) { + console.warn("Image too large. Please use an image under 512KB."); + return; + } const reader = new FileReader();src/client/components/baseComponents/setting/SettingColorPalette.ts (1)
402-424: Consider adding drag support for smoother color picking.Currently only click events are handled. Adding
mousemovewhile dragging would make the color picker feel more natural. This is optional since the current click-based approach works.src/client/index.html (1)
134-147: Consider async alternatives to synchronous XHR.While synchronous XHR ensures config loads before external scripts, it blocks the main thread and is deprecated. Consider using a module script that loads config before bootstrapping, or use the Fetch API with
awaitin a top-level async context.Alternative approach using async module script
Replace the synchronous XHR with:
- <!-- Dev config check - blocks external scripts in dev mode --> - <script> - (function () { - var xhr = new XMLHttpRequest(); - xhr.open("GET", "/config.json", false); // Synchronous request - try { - xhr.send(); - if (xhr.status === 200) { - window.__devConfig = JSON.parse(xhr.responseText); - } - } catch (e) { - // No config.json, use production defaults - } - })(); - </script> + <!-- Dev config check - loads before other scripts --> + <script type="module"> + try { + const response = await fetch("/config.json"); + if (response.ok) { + window.__devConfig = await response.json(); + } + } catch (e) { + // No config.json, use production defaults + } + // Dispatch event when config is ready + window.dispatchEvent(new Event('devConfigReady')); + </script>Then update conditional scripts to wait for the event if needed.
src/client/components/baseComponents/setting/SettingGroup.ts (1)
68-80: Consider avoiding!importantfor maintainability.The
!importantdeclarations in.collapsedand.columnsclasses can make future style overrides harder. Since this is a shadow DOM component, specificity conflicts are less likely.That said, this works correctly for the current use case.
src/client/UserSettingModal.ts (1)
723-730: Layout fix using requestAnimationFrame.The
requestAnimationFramecall afteropen()forces a re-render to fix initial layout issues with the columns grid. This is a common workaround for Lit timing issues.Consider adding a brief comment explaining why this is needed for future maintainers.
Suggested comment
public open() { this.requestUpdate(); this.modalEl?.open(); - // Force reflow after modal opens to fix initial render issues with columns layout + // Force re-render after modal opens - Lit may not have computed + // the column layout correctly on first render inside a hidden modal requestAnimationFrame(() => { this.requestUpdate(); }); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (57)
.gitignoreconfig.example.jsonpackage.jsonresources/lang/ar.jsonresources/lang/bg.jsonresources/lang/bn.jsonresources/lang/cs.jsonresources/lang/da.jsonresources/lang/de.jsonresources/lang/el.jsonresources/lang/en.jsonresources/lang/eo.jsonresources/lang/es.jsonresources/lang/fa.jsonresources/lang/fi.jsonresources/lang/fr.jsonresources/lang/gl.jsonresources/lang/he.jsonresources/lang/hi.jsonresources/lang/hu.jsonresources/lang/it.jsonresources/lang/ja.jsonresources/lang/ko.jsonresources/lang/nl.jsonresources/lang/pl.jsonresources/lang/pt-PT.jsonresources/lang/ru.jsonresources/lang/sk.jsonresources/lang/sl.jsonresources/lang/sv-SE.jsonresources/lang/tp.jsonresources/lang/tr.jsonresources/lang/uk.jsonresources/lang/zh-CN.jsonscripts/dev-stop.jssrc/client/DarkModeButton.tssrc/client/DevConfig.tssrc/client/InputHandler.tssrc/client/Main.tssrc/client/PublicLobby.tssrc/client/UserSettingModal.tssrc/client/components/baseComponents/Modal.tssrc/client/components/baseComponents/setting/SettingColorPalette.tssrc/client/components/baseComponents/setting/SettingGroup.tssrc/client/components/baseComponents/setting/SettingTerritorySkins.tssrc/client/components/baseComponents/setting/SettingThemeMode.tssrc/client/graphics/layers/PlayerInfoOverlay.tssrc/client/index.htmlsrc/client/styles/components/modal.csssrc/client/styles/components/setting.csssrc/core/configuration/Config.tssrc/core/configuration/DefaultConfig.tssrc/core/game/UserSettings.tssrc/server/Master.tstests/util/TestServerConfig.tstsconfig.jsonwebpack.config.js
🧰 Additional context used
🧠 Learnings (14)
📚 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/tp.jsonresources/lang/cs.jsonresources/lang/ko.jsonresources/lang/es.jsonresources/lang/ru.jsonresources/lang/sv-SE.jsonresources/lang/uk.jsonresources/lang/nl.jsonresources/lang/sl.jsonresources/lang/da.jsonresources/lang/gl.jsonresources/lang/ja.jsonresources/lang/fa.jsonresources/lang/pl.jsonresources/lang/fr.jsonresources/lang/el.jsonresources/lang/pt-PT.jsonresources/lang/sk.jsonresources/lang/zh-CN.jsonresources/lang/tr.jsonresources/lang/hu.jsonresources/lang/eo.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/tp.jsonresources/lang/ru.jsonresources/lang/sv-SE.jsonresources/lang/sl.jsonresources/lang/gl.jsonresources/lang/ja.jsonresources/lang/pl.jsonresources/lang/fr.jsonresources/lang/el.jsonresources/lang/pt-PT.jsonresources/lang/tr.jsonresources/lang/hu.jsonresources/lang/eo.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/tp.jsonresources/lang/sv-SE.jsonresources/lang/sl.jsonresources/lang/da.jsonresources/lang/gl.jsonresources/lang/ja.jsonresources/lang/pl.jsonresources/lang/fr.jsonresources/lang/el.jsonresources/lang/zh-CN.jsonresources/lang/tr.jsonresources/lang/hu.jsonresources/lang/eo.json
📚 Learning: 2025-07-26T05:35:03.686Z
Learnt from: aaa4xu
Repo: openfrontio/OpenFrontIO PR: 1581
File: webpack.config.js:139-141
Timestamp: 2025-07-26T05:35:03.686Z
Learning: In OpenFrontIO's webpack configuration, binary-loader processes files from resources/maps at build-time and inlines the binary payload directly into the compiled JS bundle. This means the original .bin assets are no longer needed at runtime and can be safely excluded from the final Docker image using webpack's globOptions.ignore pattern to reduce image size.
Applied to files:
webpack.config.js
📚 Learning: 2025-07-12T08:42:02.109Z
Learnt from: Aotumuri
Repo: openfrontio/OpenFrontIO PR: 1357
File: resources/lang/zh_cn.json:527-539
Timestamp: 2025-07-12T08:42:02.109Z
Learning: In OpenFrontIO project, the territory pattern key "embelem" is the correct spelling used in all language files, not "emblem". This is the technical identifier that appears in en.json and must be consistent across all localization files.
Applied to files:
resources/lang/ko.jsonresources/lang/ru.jsonresources/lang/sv-SE.jsonresources/lang/de.jsonresources/lang/uk.jsonresources/lang/nl.jsonresources/lang/sl.jsonresources/lang/da.jsonresources/lang/gl.jsonresources/lang/he.jsonresources/lang/ja.jsonresources/lang/fa.jsonresources/lang/pl.jsonresources/lang/fr.jsonresources/lang/bg.jsonresources/lang/el.jsonresources/lang/pt-PT.jsonresources/lang/sk.jsonresources/lang/zh-CN.jsonresources/lang/tr.jsonresources/lang/hu.jsonresources/lang/eo.json
📚 Learning: 2025-10-20T20:15:28.858Z
Learnt from: sambokai
Repo: openfrontio/OpenFrontIO PR: 2225
File: src/core/execution/FakeHumanExecution.ts:51-51
Timestamp: 2025-10-20T20:15:28.858Z
Learning: In src/core/execution/FakeHumanExecution.ts, game balance constants like MIRV_COOLDOWN_TICKS, MIRV_HESITATION_ODDS, VICTORY_DENIAL_TEAM_THRESHOLD, VICTORY_DENIAL_INDIVIDUAL_THRESHOLD, and STEAMROLL_CITY_GAP_MULTIPLIER are experimental tuning parameters subject to frequent change during balance testing. Do not flag changes to these values as issues or compare them against previous values.
Applied to files:
src/core/configuration/Config.ts
📚 Learning: 2025-08-12T00:31:50.144Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.
Applied to files:
src/server/Master.tssrc/core/configuration/DefaultConfig.ts
📚 Learning: 2025-06-07T19:47:34.514Z
Learnt from: its-sii
Repo: openfrontio/OpenFrontIO PR: 1089
File: src/client/styles/components/setting.css:9-22
Timestamp: 2025-06-07T19:47:34.514Z
Learning: In src/client/styles/components/setting.css, the padding difference between .setting-item and .setting-item-group is intentional - .setting-item-group doesn't need padding for the specific grouped setting styling being implemented.
Applied to files:
src/client/styles/components/setting.csssrc/client/components/baseComponents/setting/SettingGroup.ts
📚 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/sl.jsonresources/lang/pl.json
📚 Learning: 2025-11-12T23:11:34.445Z
Learnt from: MaxHT0x
Repo: openfrontio/OpenFrontIO PR: 2262
File: src/core/configuration/DefaultConfig.ts:806-832
Timestamp: 2025-11-12T23:11:34.445Z
Learning: In src/core/configuration/DefaultConfig.ts, JSDoc documentation for configuration methods should not be added inline, as it was requested that documentation be placed elsewhere in the codebase.
Applied to files:
src/core/configuration/DefaultConfig.tssrc/client/DevConfig.ts
📚 Learning: 2025-06-20T20:11:00.965Z
Learnt from: devalnor
Repo: openfrontio/OpenFrontIO PR: 1195
File: src/client/graphics/layers/AlertFrame.ts:18-18
Timestamp: 2025-06-20T20:11:00.965Z
Learning: In the OpenFrontIO codebase, UserSettings instances are created directly with `new UserSettings()` in each component that needs them. This pattern is used consistently across at least 12+ files including OptionsMenu, EventsDisplay, DarkModeButton, Main, UserSettingModal, UsernameInput, NameLayer, AlertFrame, UILayer, InputHandler, ClientGameRunner, and GameView. This is the established architectural pattern and should be followed for consistency.
Applied to files:
src/core/game/UserSettings.tssrc/client/UserSettingModal.ts
📚 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/PublicLobby.tssrc/client/Main.tssrc/client/UserSettingModal.ts
📚 Learning: 2025-08-28T15:14:40.664Z
Learnt from: mokizzz
Repo: openfrontio/OpenFrontIO PR: 1956
File: src/client/InputHandler.ts:430-449
Timestamp: 2025-08-28T15:14:40.664Z
Learning: On trackpads and touchscreens, users don't expect purely vertical two-finger swipes to trigger zoom. Pan detection using deltaX !== 0 is appropriate since any horizontal movement indicates pan intent, and zoom should be handled via pinch gestures instead.
Applied to files:
src/client/InputHandler.ts
📚 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/eo.json
🧬 Code graph analysis (7)
webpack.config.js (2)
src/core/configuration/DefaultConfig.ts (1)
gitCommit(153-155)src/core/configuration/DevConfig.ts (1)
gitCommit(45-47)
src/client/components/baseComponents/setting/SettingTerritorySkins.ts (2)
src/client/UserSettingModal.ts (1)
customElement(15-735)src/client/components/baseComponents/Modal.ts (1)
customElement(5-214)
src/server/Master.ts (1)
src/client/PublicLobby.ts (1)
fetchLobbies(90-111)
src/client/PublicLobby.ts (1)
src/client/DevConfig.ts (2)
waitForDevConfig(191-193)isDevFeatureEnabled(205-209)
src/client/components/baseComponents/setting/SettingColorPalette.ts (6)
src/client/UserSettingModal.ts (1)
customElement(15-735)src/client/DarkModeButton.ts (1)
customElement(5-45)src/client/components/baseComponents/Modal.ts (1)
customElement(5-214)src/client/components/baseComponents/setting/SettingGroup.ts (1)
customElement(6-153)src/client/components/baseComponents/setting/SettingTerritorySkins.ts (1)
customElement(87-441)src/client/components/baseComponents/setting/SettingThemeMode.ts (1)
customElement(5-167)
src/client/Main.ts (1)
src/client/DevConfig.ts (2)
loadDevConfig(214-214)isDevFeatureEnabled(205-209)
src/client/UserSettingModal.ts (4)
src/client/components/baseComponents/setting/SettingColorPalette.ts (1)
customElement(62-522)src/client/components/baseComponents/setting/SettingTerritorySkins.ts (1)
customElement(87-441)src/client/components/baseComponents/setting/SettingThemeMode.ts (1)
customElement(5-167)src/core/game/UserSettings.ts (2)
UserSettings(8-260)ThemeMode(6-6)
🔇 Additional comments (71)
src/client/graphics/layers/PlayerInfoOverlay.ts (1)
3-3: Import path correctly modernized for Lit 3.xThe change from
lit-html/directives/ref.jstolit/directives/ref.jsaligns with Lit 3.x conventions and improves consistency with other imports in the file. No old import patterns remain in the codebase.src/client/InputHandler.ts (2)
555-560: Shift+drag suppression is consistent with keyboard panning.The Shift-held check correctly prevents pointer drag from emitting movement events, matching the pattern at lines 242-248 that returns early for keyboard panning when Shift is held. Both use identical key detection logic and achieve the same goal: suppress movement when Shift is pressed. The comment reserves Shift+drag for bounding box selection—the key combination is blocked from other uses, though the selection feature itself is not yet implemented in the codebase.
191-191: Update UserSettingModal to match InputHandler default keybind and document the change.The InputHandler now uses
"Backslash"as the default fortoggleView(line 191), but UserSettingModal.ts still declaresdefaultKey="Space"(line 505). This mismatch breaks the "Reset to Default" button—users pressing it would reset to"Space"instead of the actual default"Backslash".Change line 505 in UserSettingModal.ts from
defaultKey="Space"todefaultKey="Backslash"to stay in sync with InputHandler. Also, add this keybind change to the changelog so users are aware of the migration.resources/lang/ko.json (1)
283-283: LGTM! Consistent emoji removal for cleaner UI labels.The removal of emoji prefixes from
attack_ratio_labelandterritory_patterns_labelaligns with the broader UI cleanup across all language files in this PR, improving visual consistency in the settings interface.Also applies to: 287-287
resources/lang/cs.json (1)
235-235: LGTM! Emoji prefix removed for UI consistency.The change mirrors the emoji removal pattern applied across all locale files in this PR.
resources/lang/tp.json (1)
219-219: LGTM! Consistent with the locale-wide emoji cleanup.The emoji removal maintains consistency with the UI improvements across all language files.
resources/lang/es.json (1)
235-235: LGTM! Emoji removed for cleaner settings labels.Consistent with the UI polish applied across all localization files in this PR.
resources/lang/it.json (1)
235-235: LGTM! Emoji prefix removed.The change aligns with the consistent emoji cleanup across all language files for improved UI clarity.
resources/lang/ar.json (1)
219-219: LGTM! Emoji removed for consistency.The change is part of the systematic emoji removal across all locale files, contributing to a cleaner settings UI.
tsconfig.json (1)
24-26: LGTM! TypeScript configuration updated for Jest support.The additions are appropriate:
skipLibCheck: trueimproves compilation performance by skipping type checks in declaration files fromnode_modulestypes: ["jest"]provides Jest type definitions for the test infrastructureBoth changes align with the development and testing setup mentioned in the PR.
resources/lang/zh-CN.json (1)
330-330: LGTM! Emoji prefixes removed for UI consistency.The removal of emoji decorations from
attack_ratio_labelandterritory_patterns_labelmatches the systematic cleanup applied across all localization files, resulting in cleaner, more professional-looking settings labels.Also applies to: 333-333
.gitignore (1)
13-24: LGTM - Clean ignore patterns for dev config.The new ignore entries are appropriate for the dev configuration system. Excluding
config.jsonfiles prevents local dev configs from being committed, and thepackage-lock.jsonexclusion correctly enforces pnpm usage.resources/lang/hi.json (1)
192-192: Verify locale update policy for structural changes.Based on project learnings, non-English locale files are typically only updated in dedicated "mls" translation PRs by Aotumori. While this change removes an emoji prefix (structural) rather than modifying translation content, please confirm this aligns with the project's localization workflow.
Based on learnings, translation files in resources/lang/*.json (except en.json) should generally not be updated in regular PRs.
package.json (1)
12-12: LGTM - Script wired correctly.The new
dev:stopscript correctly references the dev-stop.js file. Note that the cross-platform concern from scripts/dev-stop.js applies to this script as well.resources/lang/da.json (1)
296-300: Emoji removed from labels; keys and wording are stableBoth
attack_ratio_labelandterritory_patterns_labeljust drop the emoji prefix; the keys and Danish phrases stay the same, so this is a safe, purely visual tweak and matches the new no‑emoji settings UI.One small process note: past guidance for this repo was that non‑English locale files are usually updated in dedicated “mls” translation PRs, with regular feature PRs only touching
en.json. It might be worth confirming with maintainers that making these small value changes here is acceptable, or whether they prefer to move them into a future mls PR. Based on learnings, ...resources/lang/fa.json (1)
330-334: Emoji prefixes removed; translation unchanged in meaning
attack_ratio_labelandterritory_patterns_labelnow start without emojis, but the Persian text is unchanged otherwise. Keys and surrounding schema are intact, so this is a safe UI‑only change that keeps behaviour the same.resources/lang/fr.json (1)
313-317: Plain‑text labels, no structural changesFor
attack_ratio_labelandterritory_patterns_labelyou only removed the emoji, keeping the same French wording and the same keys. This keeps the JSON shape stable while aligning with the new icon‑free settings labels.resources/lang/sv-SE.json (1)
285-290: Emoji cleanup looks good; considertroop_ratio_labelfor consistencyRemoving the emoji from
attack_ratio_labelandterritory_patterns_labelkeeps the Swedish text the same and only affects appearance, which is fine and consistent with the updated settings UI.Right above,
troop_ratio_labelstill starts with emojis (🪖🛠️). If the goal is “no emojis in setting labels”, you may want to remove those as well here (and in other locales) for a fully consistent look.resources/lang/el.json (1)
330-334: Greek labels simplified by removing emoji
attack_ratio_labelandterritory_patterns_labelnow omit the emoji prefix but keep the same Greek phrases. Keys and descriptions remain the same, so this is a harmless presentation tweak aligned with the rest of the PR.resources/lang/ja.json (1)
330-334: Attack/territory labels now emoji‑free, behaviour unchangedThe Japanese
attack_ratio_labelandterritory_patterns_labelhave only had their leading emoji removed. Keys, phrasing, and related descriptions stay the same, so this is a safe UI polish change.resources/lang/hu.json (1)
313-317: Hungarian setting labels cleaned up, no schema impactFor
attack_ratio_labelandterritory_patterns_labelyou dropped the emoji but kept the same Hungarian text and keys. This keeps the JSON contract stable while matching the new visual style for settings.resources/lang/gl.json (1)
296-300: Galician labels updated to plain text only
attack_ratio_labelandterritory_patterns_labelnow use plain Galician without emoji prefixes. Keys and descriptions are untouched, so this is a straightforward cosmetic change consistent with the rest of the settings UI refactor.resources/lang/bg.json (1)
330-333: LGTM - Clean emoji removal from labels.This aligns with the PR-wide cleanup removing emoji prefixes from setting labels. Consistent with other locale files.
config.example.json (1)
1-36: LGTM - Good example config structure.Clean, well-organized config example. Disabling external services by default makes local development easier. The nested structure under
settingsis clear and easy to extend.src/client/components/baseComponents/setting/SettingTerritorySkins.ts (1)
384-440: LGTM - Clean render structure.Good use of template literals for dynamic styling. The disabled overlay pattern is consistent with
SettingColorPalette. Swatch grid is well organized.src/core/game/UserSettings.ts (3)
6-6: Good use of typed union for theme modes.Clean TypeScript pattern - typed union instead of class hierarchy. Easy to read and extend.
70-111: LGTM - Well-designed theme API with legacy migration.Good approach:
- Migration from legacy
darkModesetting (lines 76-78) ensures smooth upgrade for existing usersisDarkModeActive()correctly resolves system preference- Clean separation between storage, application, and state query
176-188: LGTM - Toggle logic and event dispatch.Skipping "system" mode on button toggle is sensible UX. The
dark-mode-changedevent keeps other components in sync.src/client/components/baseComponents/setting/SettingColorPalette.ts (3)
4-60: LGTM - Clean interface and default swatches.Good typed interface for
ColorSwatch. The default swatches provide a comprehensive set of customization options for game UI elements.
328-396: LGTM - Correct color conversion functions.The HSL↔HEX conversions follow standard formulas. Clean implementation without external dependencies.
449-521: LGTM - Clean render with consistent disabled pattern.Template structure is clear. The disabled overlay pattern matches
SettingTerritorySkinsfor consistency.src/core/configuration/Config.ts (1)
68-68: LGTM - Clean interface extension.Follows the existing pattern (
enableMatchmaking). Good placement in the interface.src/client/DarkModeButton.ts (2)
8-8: LGTM - Correct API migration.Using
isDarkModeActive()instead ofdarkMode()properly resolves the effective theme including system preference.
29-32: LGTM - Consistent state update after toggle.Matches the initialization pattern. Clean sync with the new theme mode API.
src/client/styles/components/modal.css (1)
17-17: LGTM - More readable alpha format.Using
rgba(35, 35, 35, 0.7)is clearer than the hex-with-alpha format.resources/lang/uk.json (1)
330-330: LGTM! Emoji removal aligns with PR pattern.The removal of emoji prefixes (⚔️ and 🏳️) from the Ukrainian translation labels is consistent with the broader PR changes across all language files. The technical keys remain unchanged while only the display text is updated.
Also applies to: 333-333
tests/util/TestServerConfig.ts (1)
16-18: LGTM! Test helper properly reflects new API.The new
enablePublicGames()method correctly mirrors the ServerConfig interface change. The "Method not implemented." pattern is consistent with other test helper methods.src/server/Master.ts (1)
96-114: LGTM! Public games gating is clear and well-logged.The conditional logic properly gates public game scheduling based on the feature flag. When disabled, it logs clearly and skips the interval setup. The preserved behavior when enabled maintains existing functionality.
src/client/styles/components/setting.css (4)
9-12: LGTM! New layout classes support tabbed settings UI.The
.settings-list--groupedand.user-setting-modal--wideclasses properly support the new settings layout with responsive behavior at 768px breakpoint.Also applies to: 14-27
33-40: LGTM! CSS variables enable theme switching.The migration from hard-coded colors to CSS variables (e.g.,
var(--modal-card-bg),var(--modal-text)) throughout the file properly supports the new theme mode system introduced in this PR.Also applies to: 149-150, 170-171, 245-246, 250-251, 313-314, 329-330
355-397: LGTM! Cosmetics placeholder styling is well-structured.The placeholder component styling is clear and provides good visual hierarchy with icon, title, description, and note elements.
399-442: LGTM! Sticky tabs implementation is clean.The sticky tab bar with backdrop-filter blur and vertical dividers between tabs creates a polished tabbed interface. The positioning and z-index values are appropriate.
webpack.config.js (3)
3-3: LGTM! dotenv integration is straightforward.The dotenv import and
.config()call enable environment-driven port configuration. This aligns with the PR's goal of customizable dev ports.Also applies to: 10-10
18-19: LGTM! Port configuration from env vars with safe defaults.The
parseInt()calls with defaults (9000 for client, 3000 for server) provide sensible fallbacks. The devServer port and proxy targets correctly use these values.Also applies to: 188-188
193-222: LGTM! Proxy targets correctly offset by serverPort.WebSocket and HTTP proxy targets for workers (/w0, /w1, /w2) correctly use
serverPort + 1,serverPort + 2,serverPort + 3. The API proxy uses baseserverPort. This matches the expected worker port allocation.Also applies to: 226-246, 262-262
src/client/PublicLobby.ts (2)
36-46: LGTM! Config-driven initialization with proper async handling.The component correctly waits for dev config to load via
waitForDevConfig()before checking thepublicLobbiesfeature flag. The early return when disabled prevents unnecessary polling and API calls.
122-124: LGTM! Feature-gated rendering prevents UI clutter.Returning empty HTML when
publicLobbiesis disabled properly hides the lobby UI in development when the feature is turned off.src/client/index.html (1)
150-161: LGTM! Feature flag gating prevents external scripts in dev.The conditional script injection pattern correctly checks
window.__devConfig.featuresfor each service (cloudflare, ads, analytics) and falls back to production defaults when config is missing. This properly supports the dev feature flag system.Also applies to: 164-176, 179-204, 524-535
src/core/configuration/DefaultConfig.ts (1)
234-236: LGTM! Public games toggle with sensible default.The
enablePublicGames()method correctly returnstrueby default and only disables whenENABLE_PUBLIC_GAMESis explicitly set to"false". This default-enabled approach is appropriate for production while allowing dev environments to opt out.src/client/components/baseComponents/setting/SettingGroup.ts (3)
1-12: Clean component setup with typed properties.The component declaration is well-organized with clear property definitions. The
columnsproperty using{ type: Boolean }is the correct Lit pattern for boolean attributes.
91-107: Solid localStorage handling with proper guards.The
loadCollapsedStatemethod correctly:
- Guards against missing
groupId- Wraps JSON parsing in try-catch
- Uses nullish coalescing for default value
Good defensive coding.
128-152: Clean render method with proper class composition.The
filter(Boolean).join(" ")pattern for building class strings is idiomatic and readable. The template structure is clear.src/client/Main.ts (3)
112-121: Correct type widening for nullable token promise.The type change from
Promise<{ token: string; createdAt: number; }>toPromise<{ token: string; createdAt: number; } | null>properly reflects thatgetTurnstileTokencan now returnnullwhen the cloudflare feature is disabled.The early
loadDevConfig()call ensures feature flags are ready before token prefetch.
348-349: Good refactor to centralized theme application.Moving from manual DOM class toggling to
this.userSettings.applyTheme()centralizes theme logic, making it easier to maintain the light/dark/system modes.
713-720: Feature flag gate for Turnstile is correct.Short-circuiting to
nullwhencloudflarefeature is disabled prevents unnecessary script loading and API calls during local development.src/client/components/baseComponents/Modal.ts (3)
8-33: Proper theme observation pattern.The MutationObserver watching
document.documentElementfor class changes is the right approach for detecting theme switches without tight coupling. The cleanup indisconnectedCallbackprevents memory leaks.
118-169: Comprehensive CSS variable system for theming.The dual CSS variable definitions (light theme in
.lightclass, dark theme as default) provide a clean theming API for child components. The variable naming is consistent and descriptive.
182-189: Clean dynamic class composition.The
filter(Boolean).join(" ")pattern matches the one used inSettingGroup.ts, maintaining consistency across components.src/client/DevConfig.ts (4)
4-47: Well-structured settings interface with backwards compatibility.The
DevSettingsConfiginterface supports both nested (grouped) and flat (legacy) formats. This allows gradual migration without breaking existing config files.
74-98: Config loading with sensible defaults.The
loadConfigfunction:
- Fetches
/config.jsonsafely- Merges with defaults using spread (features override correctly)
- Falls back to defaults on any error
One consideration: the empty
catchblock silently swallows all errors. For debugging, aconsole.debugmight help during development.
105-183: Settings application respects user preferences.The
applyConfigSettingsfunction only sets localStorage values when they are not already present (localStorage.getItem(key) === null). This prevents overwriting user-modified settings.Good use of helper functions (
applyBool,applyFloat,applyString) to reduce repetition.
185-214: Module-level auto-loading is a good pattern.The
configPromisestarts loading immediately on import, so by the timewaitForDevConfig()is called, the config is likely already loaded. ThegetDevConfig()sync fallback todefaultConfighandles race conditions gracefully.The
@deprecatedtag onloadDevConfigis helpful for guiding future usage.src/client/components/baseComponents/setting/SettingThemeMode.ts (3)
91-100: Proper lifecycle management with event cleanup.The component correctly adds the event listener in
connectedCallbackand removes it indisconnectedCallback. The arrow function syntax forhandleExternalChangeensures properthisbinding.
102-111: Good legacy migration for theme mode.The
loadThemeModemethod checks for the newsettings.themeModefirst, then falls back to the legacysettings.darkModeboolean. This ensures smooth migration for existing users.
128-166: Clean three-option UI with clear visual states.The render method produces a simple, accessible UI with clear visual feedback for the selected state. The emoji icons (☀️, 🌙, 💻) provide quick visual recognition.
src/client/UserSettingModal.ts (4)
4-12: Good import organization for new display components.The new imports are grouped logically with the existing setting component imports. The
ThemeModetype import enables proper typing for the theme change handler.
19-19: Type union extended cleanly.The
settingsModetype is extended from"basic" | "keybinds"to include"display". Using a string literal union is cleaner than a class hierarchy for this simple state.
280-298: Theme mode change handler with proper event dispatching.The
handleThemeModeChangemethod:
- Updates
UserSettingswith the new mode- Dispatches a custom
theme-mode-changedevent- Dispatches
dark-mode-changedforDarkModeButtonsyncThis event-based communication keeps components loosely coupled.
300-330: Display settings with disabled placeholders.The
renderDisplaySettingsmethod creates a clear structure for future cosmetics features. Thedisabledattribute onsetting-territory-skinsandsetting-color-palettesignals these are placeholders.resources/lang/en.json (2)
365-384: Well-organized new localization keys.The new keys follow a clear naming pattern:
tab_*for tab labelsgroup_*for setting group headerstheme_*for theme-related stringsThe descriptions are user-friendly and explain the purpose clearly.
404-406: Good emoji removal from translatable labels.Removing emojis from
attack_ratio_labelandterritory_patterns_labelimproves localization. Emojis should be rendered in the UI component, not embedded in translation strings.
| "attack_ratio_label": "আক্রমণের অনুপাত", | ||
| "attack_ratio_desc": "আক্রমণে আপনার সৈন্যদের কত শতাংশ পাঠাবেন (1-100%)", | ||
| "troop_ratio_label": "🪖🛠️ সৈন্য এবং শ্রমিক অনুপাত", | ||
| "troop_ratio_desc": "সৈন্য (যুদ্ধের জন্য) এবং শ্রমিক (সোনা উৎপাদনের জন্য) এর মধ্যে ভারসাম্য সমন্বয় করুন (1-100%)", |
There was a problem hiding this comment.
Respect localization workflow for non‑English JSON
This change only removes an emoji, but bn.json is a non‑English translation file. Usual OpenFrontIO process is to change only en.json in feature PRs and let the translation PRs update other locales. Please check with maintainers whether this change should be reverted here and handled in an "mls" translation PR instead.
Based on learnings, non‑English locale updates belong in dedicated translation PRs.
🤖 Prompt for AI Agents
In resources/lang/bn.json around lines 192 to 195, a non‑English translation
file was edited to remove an emoji; per the localization workflow you should not
modify locale files in feature PRs. Revert the change in bn.json back to its
original state, then make the emoji removal in en.json (or open a dedicated
"mls" translation PR) and coordinate with the localization/maintainers to
propagate the change through the proper translation process.
| "attack_ratio_label": "Angriffsverhältnis", | ||
| "attack_ratio_desc": "Prozentsatz deiner Truppen die in den Angriff geschickt werden (1–100 %)", | ||
| "troop_ratio_label": "🪖🛠️ Truppen-/Arbeiter-Verhältnis", | ||
| "troop_ratio_desc": "Stelle das Verhältnis zwischen Truppen (für den Kampf) und Arbeitern (für die Goldproduktion) ein (1–100 %)", | ||
| "territory_patterns_label": "🏳️ Gebietsmuster", | ||
| "territory_patterns_label": "Gebietsmuster", | ||
| "territory_patterns_desc": "Wähle, ob Gebietsmuster im Spiel angezeigt werden sollen", |
There was a problem hiding this comment.
German locale updates should go through the translation process
These two label changes are consistent with the emoji‑removal goal, but de.json is a non‑English locale and is usually not modified in regular feature PRs. Please confirm that maintainers are OK with touching this file here, or move these adjustments into a dedicated "mls" translation PR and revert them from this one.
Based on learnings, only en.json is changed in normal PRs; other locales are handled separately.
🤖 Prompt for AI Agents
In resources/lang/de.json around lines 268–273 the German locale was directly
edited to remove emojis; revert these lines to their previous state (undo the
label/desc changes in this PR) so non‑English locales remain untouched, then
move the emoji‑removal updates into a dedicated MLS/translation PR where
maintainers can approve and apply them to all locales; if you already have
explicit maintainer approval to modify de.json in this feature PR, instead add a
short PR comment stating that approval and leave changes in place.
| "attack_ratio_label": "Atakkvociento", | ||
| "attack_ratio_desc": "Kian procenton de viaj trupoj sendi en atako (1–100%)", | ||
| "troop_ratio_desc": "Alĝustigu la ekvilibron inter soldatoj (por batalo) kaj laboristoj (por orproduktado) (1–100%)", | ||
| "territory_patterns_label": "🏳️ Teritoriaj Ŝablonoj", | ||
| "territory_patterns_label": "Teritoriaj Ŝablonoj", | ||
| "territory_patterns_desc": "Elektu ĉu montri teritoriajn ŝablonajn dezajnojn en la ludo", |
There was a problem hiding this comment.
Esperanto locale change outside standard translation pipeline
The adjustments here match the UI cleanup, but eo.json is part of the non‑English translation set that is normally updated only by the localization maintainer. Please check whether these edits should instead be done in an "mls" translation PR and be reverted from this feature PR.
Based on learnings, non‑English locale JSON is not normally edited in feature PRs.
🤖 Prompt for AI Agents
In resources/lang/eo.json around lines 313 to 317, these Esperanto string edits
were made outside the standard localization pipeline; revert the changes from
this feature PR and leave eo.json unchanged here, or alternatively create a
separate "mls" localization PR containing these exact string updates and notify
the localization maintainer for review; ensure the feature branch no longer
modifies non‑English locale files unless accompanied by the proper MLS PR and
maintainer sign‑off.
| "attack_ratio_label": "Współczynnik ataku", | ||
| "attack_ratio_desc": "Jaki procent swoich żołnierzy chcesz wysłać do ataku (1–100%)", | ||
| "troop_ratio_desc": "Dostosuj balans między żołnierzami (do walki) a pracownikami (do produkcji złota) (1–100%)", | ||
| "territory_patterns_label": "🏳️ Wzory terytorium", | ||
| "territory_patterns_label": "Wzory terytorium", | ||
| "territory_patterns_desc": "Wybierz, czy wyświetlać wzory terytorium w grze", |
There was a problem hiding this comment.
Non‑English locale edited in a non‑translation PR
The value changes here look fine, but this file is a non‑English locale. Project workflow says only en.json should be changed in regular feature PRs; other locales are updated in dedicated "mls" translation PRs by the localization maintainer. Please confirm this PR is allowed to touch pl.json, or move/revert these edits into the proper translation workflow.
Based on learnings, non‑English resources/lang/*.json files should not be edited in regular PRs.
🤖 Prompt for AI Agents
In resources/lang/pl.json around lines 330 to 334, this PR modifies a
non‑English locale which violates the repo workflow (only en.json should be
changed in regular feature PRs); either revert these edits from pl.json and
leave translation updates to the dedicated "mls" translation PR, or if this
change is intentionally allowed, add a short PR note/approval comment from the
localization maintainer confirming the change and move the strings into the
proper translation process. Ensure the PR no longer contains direct edits to
non‑English locale files unless explicitly approved by the localization team.
| "attack_ratio_label": "Rácio de ataque", | ||
| "attack_ratio_desc": "Qual a percentagem de tropas para enviar num ataque (1–100%)", | ||
| "troop_ratio_desc": "Ajuste o equilíbrio entre tropas (para combate) e trabalhadores (para produção de ouro) (1–100%)", | ||
| "territory_patterns_label": "🏳️ Padrões do Território", | ||
| "territory_patterns_label": "Padrões do Território", | ||
| "territory_patterns_desc": "Escolha se deseja exibir os padrões de território no jogo", |
There was a problem hiding this comment.
Portuguese locale change may need to move to translation PR
The emoji removal here is harmless functionally, but pt-PT.json is maintained by the localization team, not general feature PRs. Please confirm this edit is acceptable in this PR, or plan to revert and let the next "mls" translation PR apply it.
Based on learnings, only en.json should be edited outside dedicated translation PRs.
🤖 Prompt for AI Agents
In resources/lang/pt-PT.json around lines 296 to 300, a localization change
(emoji removal) was made but this locale is maintained by the translations team;
either revert this edit in this PR and leave pt-PT.json unchanged, or remove the
pt-PT.json modification from the diff and instead include the change in the
dedicated "mls" translation PR so only en.json is edited in feature PRs.
| "attack_ratio_label": "Saldırı Oranı", | ||
| "attack_ratio_desc": "Bir saldırıda birliklerinin yüzde kaçını göndereceksin (%1-100)", | ||
| "troop_ratio_desc": "Askerler (savaş için) ve işçiler (altın üretimi için) arasındaki dengeyi ayarlayın (%1–100)", | ||
| "territory_patterns_label": "🏳️ Bölge Desenleri", | ||
| "territory_patterns_label": "Bölge Desenleri", | ||
| "territory_patterns_desc": "Oyunda bölge desenlerinin gösterilip gösterilmeyeceğini seç", |
There was a problem hiding this comment.
Turkish translation updates may conflict with locale workflow
These strings look fine, but tr.json is a non‑English translation file and is usually updated only in dedicated translation PRs, not in feature work. Please confirm this exception with maintainers, or plan to revert these lines here and let the translation PR handle them.
Based on learnings, non‑English resources/lang/*.json updates are reserved for "mls" translation PRs.
🤖 Prompt for AI Agents
In resources/lang/tr.json around lines 310 to 314, these Turkish string changes
are from a non‑English translation file which should not be modified in feature
PRs; either revert the edited lines back to the previous state in this branch so
the change is removed, or obtain explicit approval from the maintainers and move
these string edits into the dedicated "mls" translation PR (or create one) so
translations are handled through the proper workflow. Ensure the commit message
notes whether you reverted the translation or moved it to the translation PR and
link the maintainer approval if you chose to keep the change.
| import { execSync } from "child_process"; | ||
| import dotenv from "dotenv"; | ||
|
|
||
| dotenv.config(); | ||
|
|
||
| const clientPort = process.env.OPENFRONT_CLIENT_PORT || "9000"; | ||
| const serverPort = parseInt(process.env.OPENFRONT_SERVER_PORT || "3000", 10); | ||
|
|
||
| const ports = [ | ||
| clientPort, | ||
| serverPort, | ||
| serverPort + 1, | ||
| serverPort + 2, | ||
| serverPort + 3, | ||
| ]; | ||
|
|
||
| const portArgs = ports.map((p) => `-ti :${p}`).join(" "); | ||
|
|
||
| try { | ||
| execSync(`lsof ${portArgs} | xargs kill -9 2>/dev/null || true`, { | ||
| stdio: "inherit", | ||
| shell: true, | ||
| }); | ||
| console.log(`Stopped processes on ports: ${ports.join(", ")}`); | ||
| } catch { | ||
| // Ignore errors - processes may not be running | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check package.json for existing cross-platform alternatives and dev environment setup
cat package.json | head -100Repository: openfrontio/OpenFrontIO
Length of output: 3762
🏁 Script executed:
# Check if there are any other platform-specific scripts or platform detection patterns
rg -n "process.platform|win32|cross-platform" --max-count=10Repository: openfrontio/OpenFrontIO
Length of output: 519
🏁 Script executed:
# Check if there's any documentation about Windows support
fd -e md -e txt | head -20 | xargs grep -l "Windows\|windows\|win32" 2>/dev/null || echo "No Windows docs found"Repository: openfrontio/OpenFrontIO
Length of output: 88
Add platform detection to handle Windows in dev-stop script.
The script uses lsof to kill processes, which only works on Unix/macOS and will fail on Windows. The project targets cross-platform development (evidenced by cross-env usage and Windows builds), so Windows developers will encounter errors when running npm run dev:stop. Add a simple check using process.platform to execute the appropriate command for Windows (such as taskkill) or document that the script is Unix/macOS only.
🤖 Prompt for AI Agents
In scripts/dev-stop.js lines 1-27: add platform detection using process.platform
so the script runs different commands on Windows vs Unix; if process.platform
=== "win32", find PIDs for each port with netstat (or tasklist/findstr) and call
taskkill /PID <pid> /F (or build a combined command per port) to force-kill
processes, otherwise keep the existing lsof | xargs kill -9 approach; ensure you
handle string/number port formats consistently and run commands via execSync
with shell:true, preserving the try/catch behavior and logging which ports were
stopped.
There was a problem hiding this comment.
Not sure what to do with this one. Didn't know a clean way to keep it around just for me, without having to constantly cherry pick. I generally like to have pnpm start:dev and pnpm stop:dev. I could resubmit with this gone if preferred.
There was a problem hiding this comment.
@nodeGarden, there are a few good options here depending on whether this script would be useful for other contributors:
Option 1: Make it cross-platform (recommended if others might use it)
Add platform detection as suggested in the review. It's fairly straightforward—check process.platform === "win32" and use netstat/taskkill on Windows, or keep the existing lsof approach on Unix/macOS. This way the script works for everyone and you don't need to maintain a personal copy.
Option 2: Document as Unix/macOS only
If you think other Unix/macOS developers would find it useful but Windows support isn't critical, keep it as-is and add a comment in the script or README noting it requires Unix/macOS. Simple and transparent.
Option 3: Create a gitignored local scripts directory
Add scripts/local/ to .gitignore and move personal dev tooling there. Update your local package.json script to point to scripts/local/dev-stop.js. This keeps personal convenience scripts out of the repo entirely while avoiding cherry-picks.
Since you've already wired it into package.json and it solves a common dev pain point (cleaning up dangling processes), I'd lean toward Option 1—making it cross-platform means it's useful infrastructure for the whole team. But if it's truly just a personal preference and not something others would use, Option 3 keeps your workflow clean without adding repo clutter.
What would work best for your team's workflow?
| private saveSkin() { | ||
| localStorage.setItem("settings.territorySkin", this.selectedSkinId); | ||
| if (this.customSkinUrl) { | ||
| localStorage.setItem("settings.territorySkinCustom", this.customSkinUrl); | ||
| } | ||
| this.dispatchEvent( | ||
| new CustomEvent("change", { | ||
| detail: { skinId: this.selectedSkinId, customUrl: this.customSkinUrl }, | ||
| bubbles: true, | ||
| composed: true, | ||
| }), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Custom skin URL not cleared from localStorage when switching to preset.
When selectSkin is called (line 310-315), customSkinUrl is set to null, but saveSkin only writes to localStorage when customSkinUrl is truthy. The stale custom URL remains in storage and could cause confusion on reload.
Proposed fix
private saveSkin() {
localStorage.setItem("settings.territorySkin", this.selectedSkinId);
if (this.customSkinUrl) {
localStorage.setItem("settings.territorySkinCustom", this.customSkinUrl);
+ } else {
+ localStorage.removeItem("settings.territorySkinCustom");
}
this.dispatchEvent(📝 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.
| private saveSkin() { | |
| localStorage.setItem("settings.territorySkin", this.selectedSkinId); | |
| if (this.customSkinUrl) { | |
| localStorage.setItem("settings.territorySkinCustom", this.customSkinUrl); | |
| } | |
| this.dispatchEvent( | |
| new CustomEvent("change", { | |
| detail: { skinId: this.selectedSkinId, customUrl: this.customSkinUrl }, | |
| bubbles: true, | |
| composed: true, | |
| }), | |
| ); | |
| } | |
| private saveSkin() { | |
| localStorage.setItem("settings.territorySkin", this.selectedSkinId); | |
| if (this.customSkinUrl) { | |
| localStorage.setItem("settings.territorySkinCustom", this.customSkinUrl); | |
| } else { | |
| localStorage.removeItem("settings.territorySkinCustom"); | |
| } | |
| this.dispatchEvent( | |
| new CustomEvent("change", { | |
| detail: { skinId: this.selectedSkinId, customUrl: this.customSkinUrl }, | |
| bubbles: true, | |
| composed: true, | |
| }), | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/client/components/baseComponents/setting/SettingTerritorySkins.ts around
lines 296 to 308, saveSkin currently only writes the custom skin URL to
localStorage when this.customSkinUrl is truthy, leaving a stale
"settings.territorySkinCustom" value when switching back to a preset; update
saveSkin so that when this.customSkinUrl is falsy it removes the
"settings.territorySkinCustom" entry from localStorage (or sets it to
null/empty) instead of leaving the old value, keeping persisted state consistent
with the in-memory selected skin.
There was a problem hiding this comment.
This is only for mockup purposes, and not fully functional
There was a problem hiding this comment.
@nodeGarden, understood! Since this is mockup/placeholder code, the localStorage persistence logic can be completed when the cosmetics module is fully implemented. Thanks for the clarification! 👍
| border-top-right-radius: 4px; | ||
| font-size: 18px; | ||
| background: #000000a1; | ||
| background: rgb(2 118 161 / 95%); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if modal.css is imported anywhere
rg -l "modal\.css" --type=ts --type=html --type=css
# Check for c-modal class usage outside the component
rg "c-modal" --type=ts --type=html -C2 | head -100Repository: openfrontio/OpenFrontIO
Length of output: 5797
🏁 Script executed:
# Find and examine the Modal.ts component
fd "Modal\.ts" --type fRepository: openfrontio/OpenFrontIO
Length of output: 739
🏁 Script executed:
# Look for static styles in TypeScript files related to modal
rg "static\s+styles" -A 10 --type=ts | grep -i modal -A 10 -B 2Repository: openfrontio/OpenFrontIO
Length of output: 8074
🏁 Script executed:
# Read modal.css to see the context around line 28
cat -n src/client/styles/components/modal.css | head -40Repository: openfrontio/OpenFrontIO
Length of output: 1033
🏁 Script executed:
# Search for modal.css in all file types (including HTML)
rg "modal\.css" -g "*.html" -g "*.ts" -g "*.js"Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Check if modal.css is referenced in styles.css or any index file
cat src/client/styles.css | head -50Repository: openfrontio/OpenFrontIO
Length of output: 1267
🏁 Script executed:
# Check if c-modal classes are used outside Modal.ts component
rg "c-modal" src/client --type=ts --type=html -l | grep -v "Modal\.ts" | grep -v "node_modules"Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
# Check the full modal.css file to see all conflicting styles
cat src/client/styles/components/modal.cssRepository: openfrontio/OpenFrontIO
Length of output: 1060
Remove modal.css — it is dead code and conflicts with Modal.ts component styles.
The c-modal classes are used only within the Modal.ts Web Component, which has its own scoped styles via Shadow DOM. The global modal.css file defines identical selectors but different values (for example, header background is rgb(2 118 161 / 95%) in the CSS file but rgb(29 180 236 / 95%) in the component). Since Shadow DOM encapsulates the component styles, the global stylesheet does not apply to Modal instances and serves no purpose. Delete the file and remove the import from styles.css.
🤖 Prompt for AI Agents
In src/client/styles/components/modal.css around line 28, this global stylesheet
is dead and conflicts with the Modal.ts Web Component's scoped Shadow DOM
styles; delete the file src/client/styles/components/modal.css and remove any
import reference to it in src/client/styles/styles.css (search for the import
line and delete it) so only the component's internal styles are used.
| }); | ||
|
|
||
| const PORT = 3000; | ||
| const PORT = parseInt(process.env.OPENFRONT_SERVER_PORT ?? "3000", 10); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's check the exact code at the specified location
echo "=== Checking Master.ts around line 142 ==="
sed -n '135,150p' src/server/Master.ts
echo -e "\n=== Searching for OPENFRONT_SERVER_PORT and PORT usage ==="
rg -n "OPENFRONT_SERVER_PORT|const PORT|\.PORT" --type=ts -C 3
echo -e "\n=== Checking for validation of PORT after parsing ==="
rg -n "PORT" src/server/Master.ts -C 2 | head -50Repository: openfrontio/OpenFrontIO
Length of output: 1638
🏁 Script executed:
#!/bin/bash
# Check for any validation of OPENFRONT_SERVER_PORT before it's used
echo "=== Searching for OPENFRONT_SERVER_PORT definition/validation ==="
rg "OPENFRONT_SERVER_PORT" -C 5
echo -e "\n=== Checking if there's any validation before line 142 in Master.ts ==="
head -142 src/server/Master.ts | tail -20
echo -e "\n=== Looking for error handling around server.listen ==="
rg "server\.listen" --type=ts -B 5 -A 5Repository: openfrontio/OpenFrontIO
Length of output: 3509
Add validation for port parsing to prevent invalid values from reaching server.listen().
If OPENFRONT_SERVER_PORT contains non-numeric values like "abc", parseInt() returns NaN, which will cause server.listen() to fail unexpectedly. Add a check to ensure the parsed value is a valid number in the port range (0-65535), or wrap it in error handling.
Consider:
const portString = process.env.OPENFRONT_SERVER_PORT ?? "3000";
const PORT = parseInt(portString, 10);
if (isNaN(PORT) || PORT < 0 || PORT > 65535) {
throw new Error(`Invalid port: ${portString}`);
}
server.listen(PORT, () => {
log.info(`Master HTTP server listening on port ${PORT}`);
});🤖 Prompt for AI Agents
In src/server/Master.ts around line 142, the PORT value is derived using
parseInt without validation, which can yield NaN or out-of-range values if
OPENFRONT_SERVER_PORT is invalid; update the code to read the env var into a
string, parse it to an integer, and validate that it is a number within 0–65535,
throwing or falling back to a safe default (or returning an error) if validation
fails, before passing PORT to server.listen so invalid values never reach
server.listen.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
|
Please remove all i18n files expect en.json, we are using crowdin to manage translations (See README.md) A screenshot of the Settings UI Improvements would probably be nice |
|
please keep PR smaller (<300 lines) |
github-project-automation
bot
moved this from Final Review
to Complete
in OpenFront Release Management
5 participants
Summary
Adds developer tooling and settings UI improvements:
Dev Feature Flags - Config-based system to disable external services (analytics, Cloudflare/Turnstile, ads, public lobbies) for local development without modifying code.
Settings UI Overhaul - New Display tab with theme mode selector (Light/Dark/System), collapsible setting groups, and placeholder components for future premium features.
Sorry these aren't separated, but I went too far down the path, and separating became messy.
Changes
Dev Feature Flags
Client-side
config.jsonServer-side
enablePublicGames()to ServerConfig interfaceENABLE_PUBLIC_GAMESenv var (defaults to true)Dev convenience
dev:stopscriptSettings UI Improvements
New Components
Theme Support
themeMode(),setThemeMode(),isDarkModeActive()UI Improvements
Other
skipLibCheckand jest typesUsage
Client features (config.json)
Copy
config.example.jsontoconfig.jsonand customize:{ "features": { "analytics": false, "publicLobbies": false, "cloudflare": false, "ads": false }, "settings": { "display": { "themeMode": "dark" } } }Server features (.env)
Test plan
Dev Feature Flags
cloudflare: false- Turnstile skipped, no verification errorspublicLobbies: false- no lobby pollingENABLE_PUBLIC_GAMES=false- server skips public game schedulingSettings UI
🤖 Generated with Claude Code