feat: self-guided onboarding Learning Path (Beginner to Advanced)#3890
Open
bhaumik wants to merge 5 commits intoopenfrontio:mainfrom
Open
feat: self-guided onboarding Learning Path (Beginner to Advanced)#3890bhaumik wants to merge 5 commits intoopenfrontio:mainfrom
bhaumik wants to merge 5 commits intoopenfrontio:mainfrom
Conversation
Adds a 13-step onboarding experience organized into 3 skill tiers
(Beginner → Intermediate → Advanced) to take new players from zero to
competitive.
Changes:
- OnboardingModal.ts: new Lit component with sidebar step list, progress
tracking (localStorage), per-step tutorial video embeds from YouTube,
and a 3-tier learning path (5 Beginner + 5 Intermediate + 3 Advanced steps)
- index.html: registers <onboarding-modal> as an inline page-content element
- Main.ts: imports modal, auto-navigates first-time visitors to the
onboarding page on load (after 600ms grace period)
- DesktopNavBar.ts: adds '🎓 Guide' nav item (emerald hover)
- MobileNavBar.ts: adds '🎓 Learning Path' nav item (emerald hover)
Learning Path covers:
Beginner: Goal/Controls, Spawn Selection, Attack Ratio, Economy/Cities,
Bots vs Nations
Intermediate: 42% Growth Rule, Alliances & Traitor, Buildings Deep Dive,
Border Warfare, Naval Basics
Advanced: Nuclear Strategy, Multi-Front Coordination, Endgame Closing
Each step includes 4-5 actionable tips and an optional inline YouTube tutorial
video clip (timestamped to the relevant section where available).
Resolves: new player onboarding gap
|
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a multi-step onboarding modal (13 lessons) with localStorage progress, auto-open for first-time users, nav buttons, tour overlay with in-game event wiring, API/WS env overrides, a .env.production gitignore exception, and a Vercel build config. ChangesOnboarding Learning Path Feature
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Main as App.bootstrap
participant Check as hasSeenOnboarding()
participant Modal as onboarding-modal
participant Storage as localStorage
Browser->>Main: load
Main->>Check: hasSeenOnboarding()
alt not seen
Main->>Modal: schedule open (600ms)
Modal->>Storage: markOnboardingShown()
Modal->>Storage: loadProgress()
end
Browser->>Main: click data-action=open-onboarding
Main->>Modal: navigate to page-onboarding
Modal->>Storage: saveProgress() (on completes)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/client/components/DesktopNavBar.ts`:
- Around line 131-134: Replace the hardcoded "🎓 Guide" label in DesktopNavBar
(the <button> with data-page="page-onboarding") to use the translateText() call
and set its data-i18n attribute like the other nav items; add the key
"main.guide": "🎓 Guide" to resources/lang/en.json so
translateText("main.guide") will resolve correctly. Ensure the button uses the
same class and data-i18n pattern as sibling items and that translateText is
imported/used where other nav labels are handled.
In `@src/client/components/MobileNavBar.ts`:
- Around line 145-148: The button in MobileNavBar.ts currently hardcodes "🎓
Learning Path" (data-page="page-onboarding"); replace the literal with a call to
translateText('main.learning_path') (or use the same data-i18n pattern used by
other nav items) so the UI string goes through the i18n pipeline, and add the
key "main.learning_path": "🎓 Learning Path" to resources/lang/en.json to
provide the English translation.
In `@src/client/Main.ts`:
- Line 37: The import currently pulls in OnboardingModal but never uses it;
change the import to a side-effect-only import so the `@customElement`
registration in OnboardingModal still runs. Replace the named import line that
references OnboardingModal (while keeping hasSeenOnboarding if still needed)
with a side-effect import for "./OnboardingModal" and only import
hasSeenOnboarding as a named import when used; this ensures the onboarding-modal
custom element is registered without unused symbols.
In `@src/client/OnboardingModal.ts`:
- Around line 252-269: Update TRACK_META to replace the hardcoded label string
with an i18n key (e.g., change the label property to labelKey) so TRACK_META:
Record<Track, { labelKey: string; color: string; bg: string }>, update the three
entries (beginner/intermediate/advanced) to use keys like
"onboarding.track.beginner", then update the component render code that reads
TRACK_META (where TRACK_META is referenced) to call translateText(meta.labelKey)
instead of using meta.label; finally add the corresponding keys to
resources/lang/en.json to satisfy the translation requirement.
- Around line 26-250: The STEPS array in OnboardingModal.ts contains hardcoded
user-visible strings; replace each title/subtitle/videoLabel/tip with i18n keys
(e.g., titleKey, subtitleKey, tipsKeys[], videoLabelKey) inside each step object
in the STEPS const, add the corresponding text entries to resources/lang/en.json
(using keys like onboarding.step_{id}.title, .subtitle, .tip_{n}, .video_label),
and update all render sites to call translateText(step.titleKey),
translateText(step.subtitleKey), translateText(key) for each tipsKeys entry, and
translateText(step.videoLabelKey); ensure videoUrl remains unchanged and keep
icon/track/id as-is.
- Around line 402-660: Replace all hardcoded user-facing strings in the render
methods with calls to translateText(...) and use interpolation for dynamic
values: update renderStepContent (replace "Hide Video", "Watch:", "✓ Mark
Complete", "Next lesson →", "All X lessons complete!", "You've gone from recruit
to commander.", "Reset progress", "Key Tips", "Step ${this.currentStep + 1} of
${STEPS.length}", "Done", video title default) to translateText with appropriate
i18n keys and placeholders; update renderProgressBar (replace "% complete"
display, track pill labels like `${meta.label}` and percent string) and render()
sidebar/header (replace "Learning Path", "complete", "Close" aria-label, "Prev",
"Next") likewise; ensure dynamic numbers (step index, STEPS.length,
trackDone/trackSteps.length, pct) are passed as interpolation params to
translateText; verify keys exist in resources/lang/en.json and fallback to
existing step.videoLabel when present; touch methods referenced here
(renderStepContent, renderProgressBar, render) and UI handlers (markComplete,
resetProgress, close, toggleVideo) only to pass translated labels.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8a9d64b-3df2-4f12-aa55-071424954b9c
📒 Files selected for processing (5)
index.htmlsrc/client/Main.tssrc/client/OnboardingModal.tssrc/client/components/DesktopNavBar.tssrc/client/components/MobileNavBar.ts
Comment on lines
+131
to
+134
| <button | ||
| class="nav-menu-item text-white/70 hover:text-emerald-400 font-medium tracking-wider uppercase cursor-pointer transition-colors [&.active]:text-emerald-400" | ||
| data-page="page-onboarding" | ||
| >🎓 Guide</button> |
Contributor
There was a problem hiding this comment.
Add i18n support for "Guide" text.
The hardcoded text "🎓 Guide" violates the coding guideline requiring all user-visible text to go through translateText(). Other navigation items use data-i18n attributes.
🌐 Proposed fix
<button
class="nav-menu-item text-white/70 hover:text-emerald-400 font-medium tracking-wider uppercase cursor-pointer transition-colors [&.active]:text-emerald-400"
data-page="page-onboarding"
->🎓 Guide</button>
+ data-i18n="main.guide"
+></button>Add to resources/lang/en.json:
"main.guide": "🎓 Guide"As per coding guidelines: All user-visible text in src/client/**/*.{ts,tsx} must go through translateText() function with corresponding entries in resources/lang/en.json.
📝 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
| <button | |
| class="nav-menu-item text-white/70 hover:text-emerald-400 font-medium tracking-wider uppercase cursor-pointer transition-colors [&.active]:text-emerald-400" | |
| data-page="page-onboarding" | |
| >🎓 Guide</button> | |
| <button | |
| class="nav-menu-item text-white/70 hover:text-emerald-400 font-medium tracking-wider uppercase cursor-pointer transition-colors [&.active]:text-emerald-400" | |
| data-page="page-onboarding" | |
| data-i18n="main.guide" | |
| ></button> |
🤖 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/components/DesktopNavBar.ts` around lines 131 - 134, Replace the
hardcoded "🎓 Guide" label in DesktopNavBar (the <button> with
data-page="page-onboarding") to use the translateText() call and set its
data-i18n attribute like the other nav items; add the key "main.guide": "🎓
Guide" to resources/lang/en.json so translateText("main.guide") will resolve
correctly. Ensure the button uses the same class and data-i18n pattern as
sibling items and that translateText is imported/used where other nav labels are
handled.
Comment on lines
+145
to
+148
| <button | ||
| class="nav-menu-item block w-full text-left font-bold uppercase tracking-[0.05em] text-white/70 transition-all duration-200 cursor-pointer hover:text-emerald-400 hover:translate-x-2.5 hover:drop-shadow-[0_0_20px_rgba(52,211,153,0.5)] [&.active]:text-emerald-400 [&.active]:translate-x-2.5 text-[clamp(18px,2.8vh,32px)] py-[clamp(0.2rem,0.8vh,0.75rem)]" | ||
| data-page="page-onboarding" | ||
| >🎓 Learning Path</button> |
Contributor
There was a problem hiding this comment.
Add i18n support for "Learning Path" text.
The hardcoded text "🎓 Learning Path" violates the coding guideline requiring all user-visible text to go through translateText(). Other navigation items use data-i18n attributes.
🌐 Proposed fix
<button
class="nav-menu-item block w-full text-left font-bold uppercase tracking-[0.05em] text-white/70 transition-all duration-200 cursor-pointer hover:text-emerald-400 hover:translate-x-2.5 hover:drop-shadow-[0_0_20px_rgba(52,211,153,0.5)] [&.active]:text-emerald-400 [&.active]:translate-x-2.5 text-[clamp(18px,2.8vh,32px)] py-[clamp(0.2rem,0.8vh,0.75rem)]"
data-page="page-onboarding"
->🎓 Learning Path</button>
+ data-i18n="main.learning_path"
+></button>Add to resources/lang/en.json:
"main.learning_path": "🎓 Learning Path"As per coding guidelines: All user-visible text in src/client/**/*.{ts,tsx} must go through translateText() function with corresponding entries in resources/lang/en.json.
📝 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
| <button | |
| class="nav-menu-item block w-full text-left font-bold uppercase tracking-[0.05em] text-white/70 transition-all duration-200 cursor-pointer hover:text-emerald-400 hover:translate-x-2.5 hover:drop-shadow-[0_0_20px_rgba(52,211,153,0.5)] [&.active]:text-emerald-400 [&.active]:translate-x-2.5 text-[clamp(18px,2.8vh,32px)] py-[clamp(0.2rem,0.8vh,0.75rem)]" | |
| data-page="page-onboarding" | |
| >🎓 Learning Path</button> | |
| <button | |
| class="nav-menu-item block w-full text-left font-bold uppercase tracking-[0.05em] text-white/70 transition-all duration-200 cursor-pointer hover:text-emerald-400 hover:translate-x-2.5 hover:drop-shadow-[0_0_20px_rgba(52,211,153,0.5)] [&.active]:text-emerald-400 [&.active]:translate-x-2.5 text-[clamp(18px,2.8vh,32px)] py-[clamp(0.2rem,0.8vh,0.75rem)]" | |
| data-page="page-onboarding" | |
| data-i18n="main.learning_path" | |
| ></button> |
🤖 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/components/MobileNavBar.ts` around lines 145 - 148, The button in
MobileNavBar.ts currently hardcodes "🎓 Learning Path"
(data-page="page-onboarding"); replace the literal with a call to
translateText('main.learning_path') (or use the same data-i18n pattern used by
other nav items) so the UI string goes through the i18n pipeline, and add the
key "main.learning_path": "🎓 Learning Path" to resources/lang/en.json to
provide the English translation.
| import { GameStartingModal } from "./GameStartingModal"; | ||
| import "./GoogleAdElement"; | ||
| import { HelpModal } from "./HelpModal"; | ||
| import { hasSeenOnboarding, OnboardingModal } from "./OnboardingModal"; |
Contributor
There was a problem hiding this comment.
Use side-effect-only import for custom element registration.
The OnboardingModal class is imported but never used. The import is only needed to register the onboarding-modal custom element via the @customElement decorator.
♻️ Proposed fix
-import { hasSeenOnboarding, OnboardingModal } from "./OnboardingModal";
+import { hasSeenOnboarding } from "./OnboardingModal";
+import "./OnboardingModal";📝 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
| import { hasSeenOnboarding, OnboardingModal } from "./OnboardingModal"; | |
| import { hasSeenOnboarding } from "./OnboardingModal"; | |
| import "./OnboardingModal"; |
🧰 Tools
🪛 ESLint
[error] 37-37: 'OnboardingModal' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Check: 🔍 ESLint
[failure] 37-37:
'OnboardingModal' is defined but never used. (@typescript-eslint/no-unused-vars)
🤖 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/Main.ts` at line 37, The import currently pulls in OnboardingModal
but never uses it; change the import to a side-effect-only import so the
`@customElement` registration in OnboardingModal still runs. Replace the named
import line that references OnboardingModal (while keeping hasSeenOnboarding if
still needed) with a side-effect import for "./OnboardingModal" and only import
hasSeenOnboarding as a named import when used; this ensures the onboarding-modal
custom element is registered without unused symbols.
Comment on lines
+26
to
+250
| const STEPS: OnboardingStep[] = [ | ||
| // ── BEGINNER ──────────────────────────────────────────────────────────────── | ||
| { | ||
| id: 0, | ||
| track: "beginner", | ||
| icon: "🌍", | ||
| title: "The Goal: World Domination", | ||
| subtitle: | ||
| "OpenFront is a real-time browser strategy game. Your mission: conquer 80% of the map before anyone else does.", | ||
| tips: [ | ||
| "Reach 80% of the map's territory to win. Lose all your territory and you're eliminated.", | ||
| "The game runs in real time — there are no turns. Keep your eye on the action.", | ||
| "You start as a tiny nation and expand by clicking adjacent enemy or neutral tiles.", | ||
| "Gold and troops are your two key resources — everything you build costs one or both.", | ||
| ], | ||
| videoUrl: "https://www.youtube.com/embed/8bxcAsJJXJg", | ||
| videoLabel: "Ultimate Beginner's Guide", | ||
| }, | ||
| { | ||
| id: 1, | ||
| track: "beginner", | ||
| icon: "📍", | ||
| title: "Picking Your Spawn", | ||
| subtitle: | ||
| "Where you start matters enormously. The right spawn gives you space to breathe while everyone else fights.", | ||
| tips: [ | ||
| "Prefer plains / grasslands (green terrain) — cheapest and fastest to capture.", | ||
| "Avoid the extreme edge of the map: you'll only be able to expand in 1–2 directions.", | ||
| "Center spawns allow 360° expansion but attract more neighbors — good for diplomacy.", | ||
| "Highlands and mountains are slow and expensive early; save them for mid-game fortification.", | ||
| "Look for a spawn with a coast nearby — ports will be crucial later.", | ||
| ], | ||
| videoUrl: "https://www.youtube.com/embed/EdcdsayA_ac?start=375", | ||
| videoLabel: "Picking a Spot (Ultimus_Rex Guide)", | ||
| }, | ||
| { | ||
| id: 2, | ||
| track: "beginner", | ||
| icon: "⚔️", | ||
| title: "Troops & Attack Ratio", | ||
| subtitle: | ||
| "The attack ratio slider controls how many troops you commit to each offensive. Mastering it is the difference between a steady conquest and a catastrophic overextension.", | ||
| tips: [ | ||
| "Start attacks at ~30% ratio. Going all-in early leaves you exposed to neighbors.", | ||
| "Your population recovers fastest when it sits at 40–50% of your cap — don't drain it.", | ||
| "NEVER send 100% of your troops at once. You will be wiped out by a third party.", | ||
| "The blue bar (bottom-left) shows troops vs. workers — more workers = more gold income.", | ||
| "Raise the ratio for a finishing blow; lower it when you need fast recovery.", | ||
| ], | ||
| videoUrl: "https://www.youtube.com/embed/fKCWgr5_nwo", | ||
| videoLabel: "How to Play (Enzo Plays Beginner Tutorial)", | ||
| }, | ||
| { | ||
| id: 3, | ||
| track: "beginner", | ||
| icon: "🏙️", | ||
| title: "Economy: Cities & Workers", | ||
| subtitle: | ||
| "Cities are your power ceiling. Every city you build or capture raises your max population — which directly raises how many troops you can field.", | ||
| tips: [ | ||
| "Each city adds 25,000 to your population cap. More cities = larger army ceiling.", | ||
| "Workers generate gold passively. Keeping some workers alive is crucial for sustained growth.", | ||
| "Build your first city in a safe interior tile — not on a contested border.", | ||
| "Gold unlocks everything: buildings, nukes, warships. A broke nation can't compete.", | ||
| "Capturing an enemy's territory inherits all their gold. Rich enemies are worth targeting.", | ||
| ], | ||
| videoUrl: "https://www.youtube.com/embed/fKCWgr5_nwo?start=300", | ||
| videoLabel: "City Importance (Enzo Plays)", | ||
| }, | ||
| { | ||
| id: 4, | ||
| track: "beginner", | ||
| icon: "🤖", | ||
| title: "Bots vs. Nations — Easy Prey First", | ||
| subtitle: | ||
| "Not every opponent is equal. Bots are easy targets for early expansion. Learn to tell them apart and prioritize accordingly.", | ||
| tips: [ | ||
| "Bots don't counter-attack aggressively — ideal for your first land grabs.", | ||
| "Encircling a bot (surrounding it on all sides) makes it surrender without a fight.", | ||
| "Nations (AI with flags) are harder — only attack them when you massively outnumber them.", | ||
| "Human players are the most dangerous. Ally with them early when possible.", | ||
| "Clearing nearby bots fast creates a secure base before the mid-game fights start.", | ||
| ], | ||
| videoUrl: "https://www.youtube.com/embed/EdcdsayA_ac?start=510", | ||
| videoLabel: "Bot Taking Strategy (Ultimus_Rex)", | ||
| }, | ||
|
|
||
| // ── INTERMEDIATE ───────────────────────────────────────────────────────────── | ||
| { | ||
| id: 5, | ||
| track: "intermediate", | ||
| icon: "📈", | ||
| title: "The 42% Rule: Population Science", | ||
| subtitle: | ||
| "Population growth follows a bell curve. It peaks at roughly 42% of your max cap — understanding this is what separates casual players from consistent winners.", | ||
| tips: [ | ||
| "Your pop grows fastest at ~42% of your max cap. Don't sit at 1% or 99%.", | ||
| "Conquer territory AND build cities to keep raising your cap — that raises your actual troop ceiling.", | ||
| "Watch the growth color: green = accelerating, yellow = slowing. Adjust your aggression accordingly.", | ||
| "After a big attack that drains you, wait for recovery before launching the next one.", | ||
| "City count is the best proxy for long-term power. Count your cities, count theirs.", | ||
| ], | ||
| videoUrl: "https://www.youtube.com/embed/9LOx9lFJn6I", | ||
| videoLabel: "Population Mechanics Deep-Dive (Enzo Plays)", | ||
| }, | ||
| { | ||
| id: 6, | ||
| track: "intermediate", | ||
| icon: "🤝", | ||
| title: "Alliances & the Traitor Mechanic", | ||
| subtitle: | ||
| "Diplomacy can win games. But alliances can also be broken — and the game punishes betrayal with a combat debuff.", | ||
| tips: [ | ||
| "Right-click an ally's territory to request/accept an alliance. Allied players can't attack each other.", | ||
| "Breaking an alliance gives you the Traitor debuff — your attack efficiency drops significantly.", | ||
| "Best practice: ally non-threatening neighbors, then break only when you can guarantee a fast kill.", | ||
| "Trade ships from allied ports generate passive income — a financial incentive to stay friendly.", | ||
| "A strong alliance network acts as a deterrent. Others are less likely to attack a well-connected player.", | ||
| ], | ||
| videoUrl: "https://www.youtube.com/embed/1fpszw34sQg", | ||
| videoLabel: "Diplomacy Guide (Risk4Ever)", | ||
| }, | ||
| { | ||
| id: 7, | ||
| track: "intermediate", | ||
| icon: "🏗️", | ||
| title: "Buildings Deep Dive", | ||
| subtitle: | ||
| "Four core structures define your mid-game. Know when to build each one — and in what order.", | ||
| tips: [ | ||
| "🏙️ Cities (cheapest): Raise max pop cap. Build in safe interior tiles first.", | ||
| "⚓ Ports (250k gold): Generate trade-ship income. Build once you have a coastline.", | ||
| "🛡️ Defense Posts: +5× defensive multiplier on that tile. Place on vulnerable border tiles.", | ||
| "🚀 SAM Launchers: Intercept incoming nukes. Essential once anyone on the map has silos.", | ||
| "Recommended build order: Cities → Ports → Defense Posts → SAM Launchers.", | ||
| ], | ||
| videoUrl: "https://www.youtube.com/embed/jvHEvbko3uw", | ||
| videoLabel: "Buildings Overview", | ||
| }, | ||
| { | ||
| id: 8, | ||
| track: "intermediate", | ||
| icon: "🗺️", | ||
| title: "Border Warfare & Chokepoints", | ||
| subtitle: | ||
| "Your attack speed is directly proportional to your shared border with the enemy. Longer borders = faster conquest — but also more exposure.", | ||
| tips: [ | ||
| "Larger shared borders mean you send more troops per tick into the fight.", | ||
| "Chokepoints (narrow land bridges, mountain passes) are natural defense lines. Hold them.", | ||
| "Once one border is secure, redirect ALL surplus forces to the active fight.", | ||
| "Multi-front attacks force the enemy to split their defense — they can't hold everywhere.", | ||
| "Never fight on two human-player fronts simultaneously early — consolidate first.", | ||
| ], | ||
| videoUrl: "https://www.youtube.com/embed/EdcdsayA_ac?start=660", | ||
| videoLabel: "Mid-game Strategy (Ultimus_Rex)", | ||
| }, | ||
| { | ||
| id: 9, | ||
| track: "intermediate", | ||
| icon: "🌊", | ||
| title: "Naval Warfare Basics", | ||
| subtitle: | ||
| "Ports unlock an entirely separate dimension of the game. Trade ships earn you money; warships let you bypass land defenses entirely.", | ||
| tips: [ | ||
| "Build a port as soon as you control a coastline. Trade ships generate passive gold.", | ||
| "Up to 150 trade ships can be active at once — longer routes between allied ports = more gold.", | ||
| "Warships bombard adjacent land tiles. Use them to soften coastal cities before a land push.", | ||
| "Destroyers hunt enemy trade ships — economic warfare can cripple a rich opponent.", | ||
| "Battleships are the strongest offensive naval unit. Protect them; they're expensive.", | ||
| ], | ||
| videoUrl: "https://www.youtube.com/embed/jvHEvbko3uw?start=120", | ||
| videoLabel: "Ports & Naval Units", | ||
| }, | ||
|
|
||
| // ── ADVANCED ──────────────────────────────────────────────────────────────── | ||
| { | ||
| id: 10, | ||
| track: "advanced", | ||
| icon: "💣", | ||
| title: "Nuclear Strategy", | ||
| subtitle: | ||
| "Nukes change the game the moment the first silo is built. Whether you're launching or defending, you need a nuclear doctrine before you need it.", | ||
| tips: [ | ||
| "Atom Bomb: best vs. dense city clusters. Targeted, relatively cheap.", | ||
| "Hydrogen Bomb: massive area devastation. Reserve for eliminating a dominant top player.", | ||
| "MIRV: fires multiple warheads — SAMs can't intercept all of them. The finisher.", | ||
| "SAM Launchers auto-target the nearest incoming nuke. Spread your nukes to overwhelm SAM coverage.", | ||
| "Don't launch nukes reactively. Plan each strike: target high-density cities, not empty wilderness.", | ||
| ], | ||
| videoUrl: "https://www.youtube.com/embed/EdcdsayA_ac?start=35", | ||
| videoLabel: "Nuclear Weapons (Ultimus_Rex)", | ||
| }, | ||
| { | ||
| id: 11, | ||
| track: "advanced", | ||
| icon: "🎯", | ||
| title: "Multi-Front Coordination", | ||
| subtitle: | ||
| "The difference between a good player and a great one: great players never fight a fair fight. They always bring overwhelming force to a single front while keeping everyone else quiet.", | ||
| tips: [ | ||
| "Attack 2–3 enemy borders at the same time — they can't reinforce all of them.", | ||
| "Coordinate timing with allies: if you hit the north while they hit the south, the enemy collapses.", | ||
| "Keep a token defense on your safe borders — just enough to deter, everything else goes to offense.", | ||
| "After eliminating a player, pause briefly to absorb their gold before the next offensive.", | ||
| "Target the weakest player first, not the strongest — snowball momentum matters.", | ||
| ], | ||
| }, | ||
| { | ||
| id: 12, | ||
| track: "advanced", | ||
| icon: "👑", | ||
| title: "Endgame: Closing Out the Win", | ||
| subtitle: | ||
| "Past 60% map control, you're the target. Everyone else will unite against you. The endgame is as much about politics as firepower.", | ||
| tips: [ | ||
| "Keep at least one strong ally alive — they divide enemy attention and absorb attacks meant for you.", | ||
| "Use MIRVs to eliminate the top 2 threats, then conventional forces to clean up the rest.", | ||
| "The 80% win threshold means you don't need to eliminate everyone — just dominate the land.", | ||
| "Avoid overextension: holding a huge front line is expensive and risky. Consolidate first.", | ||
| "Watch for the coalition forming — break it by offering the weakest member a better deal.", | ||
| ], | ||
| videoUrl: "https://www.youtube.com/embed/EdcdsayA_ac?start=775", | ||
| videoLabel: "Post-nuclear Endgame (Ultimus_Rex)", | ||
| }, | ||
| ]; |
Contributor
There was a problem hiding this comment.
All step content requires i18n support.
The entire STEPS array contains extensive hardcoded English text (titles, subtitles, tips, video labels) that violates the coding guideline requiring all user-visible text to go through translateText().
The refactor should:
- Replace hardcoded strings with i18n keys
- Add all content to
resources/lang/en.json - Use
translateText()when rendering each field
Example pattern for one step:
{
id: 0,
track: "beginner",
icon: "🌍",
titleKey: "onboarding.step_0.title",
subtitleKey: "onboarding.step_0.subtitle",
tipsKeys: [
"onboarding.step_0.tip_0",
"onboarding.step_0.tip_1",
// ...
],
videoUrl: "https://www.youtube.com/embed/8bxcAsJJXJg",
videoLabelKey: "onboarding.step_0.video_label",
}Then in render methods, use:
translateText(step.titleKey)As per coding guidelines: All user-visible text in src/client/**/*.{ts,tsx} must go through translateText() function with corresponding entries in resources/lang/en.json.
🤖 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/OnboardingModal.ts` around lines 26 - 250, The STEPS array in
OnboardingModal.ts contains hardcoded user-visible strings; replace each
title/subtitle/videoLabel/tip with i18n keys (e.g., titleKey, subtitleKey,
tipsKeys[], videoLabelKey) inside each step object in the STEPS const, add the
corresponding text entries to resources/lang/en.json (using keys like
onboarding.step_{id}.title, .subtitle, .tip_{n}, .video_label), and update all
render sites to call translateText(step.titleKey),
translateText(step.subtitleKey), translateText(key) for each tipsKeys entry, and
translateText(step.videoLabelKey); ensure videoUrl remains unchanged and keep
icon/track/id as-is.
Comment on lines
+252
to
+269
| const TRACK_META: Record<Track, { label: string; color: string; bg: string }> = | ||
| { | ||
| beginner: { | ||
| label: "Beginner", | ||
| color: "text-emerald-400", | ||
| bg: "bg-emerald-500/20 border-emerald-500/30", | ||
| }, | ||
| intermediate: { | ||
| label: "Intermediate", | ||
| color: "text-blue-400", | ||
| bg: "bg-blue-500/20 border-blue-500/30", | ||
| }, | ||
| advanced: { | ||
| label: "Advanced", | ||
| color: "text-amber-400", | ||
| bg: "bg-amber-500/20 border-amber-500/30", | ||
| }, | ||
| }; |
Contributor
There was a problem hiding this comment.
Track labels require i18n support.
The track labels ("Beginner", "Intermediate", "Advanced") are hardcoded strings that should use i18n keys.
🌐 Proposed fix pattern
const TRACK_META: Record<Track, { labelKey: string; color: string; bg: string }> = {
beginner: {
labelKey: "onboarding.track.beginner",
color: "text-emerald-400",
bg: "bg-emerald-500/20 border-emerald-500/30",
},
// ...
};
// In render:
translateText(meta.labelKey)As per coding guidelines: All user-visible text must go through translateText() with entries in resources/lang/en.json.
🤖 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/OnboardingModal.ts` around lines 252 - 269, Update TRACK_META to
replace the hardcoded label string with an i18n key (e.g., change the label
property to labelKey) so TRACK_META: Record<Track, { labelKey: string; color:
string; bg: string }>, update the three entries (beginner/intermediate/advanced)
to use keys like "onboarding.track.beginner", then update the component render
code that reads TRACK_META (where TRACK_META is referenced) to call
translateText(meta.labelKey) instead of using meta.label; finally add the
corresponding keys to resources/lang/en.json to satisfy the translation
requirement.
Comment on lines
+402
to
+660
| private renderStepContent(step: OnboardingStep) { | ||
| const isDone = this.completed.has(step.id); | ||
| const isFirst = this.currentStep === 0; | ||
| const isLast = this.currentStep === STEPS.length - 1; | ||
| const meta = TRACK_META[step.track]; | ||
| const totalDone = this.completed.size; | ||
| const allDone = totalDone === STEPS.length; | ||
|
|
||
| return html` | ||
| <div class="flex flex-col h-full overflow-hidden"> | ||
|
|
||
| <!-- Header --> | ||
| <div class="shrink-0 px-5 pt-5 pb-4 border-b border-white/10"> | ||
| <div class="flex items-start justify-between gap-4"> | ||
| <div class="flex items-center gap-3"> | ||
| <span class="text-3xl">${step.icon}</span> | ||
| <div> | ||
| <div class="flex items-center gap-2 mb-1"> | ||
| ${this.renderTrackBadge(step.track)} | ||
| <span class="text-white/30 text-xs">Step ${this.currentStep + 1} of ${STEPS.length}</span> | ||
| </div> | ||
| <h2 class="text-lg font-bold text-white leading-tight">${step.title}</h2> | ||
| </div> | ||
| </div> | ||
| ${isDone | ||
| ? html`<span class="shrink-0 flex items-center gap-1 text-emerald-400 text-xs font-bold"> | ||
| <svg class="w-4 h-4" fill="none" viewBox="0 0 24 24" stroke="currentColor"> | ||
| <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2.5" d="M5 13l4 4L19 7"/> | ||
| </svg> | ||
| Done | ||
| </span>` | ||
| : nothing} | ||
| </div> | ||
| </div> | ||
|
|
||
| <!-- Body --> | ||
| <div class="flex-1 min-h-0 overflow-y-auto px-5 py-4 space-y-5 | ||
| scrollbar-thin scrollbar-thumb-white/20 scrollbar-track-transparent"> | ||
|
|
||
| <!-- Subtitle --> | ||
| <p class="text-white/70 text-sm leading-relaxed">${step.subtitle}</p> | ||
|
|
||
| <!-- Tips --> | ||
| <div class="space-y-2"> | ||
| <h3 class="text-xs font-bold uppercase tracking-widest ${meta.color}">Key Tips</h3> | ||
| <ul class="space-y-2"> | ||
| ${step.tips.map( | ||
| (tip) => html` | ||
| <li class="flex items-start gap-2 text-sm text-white/80 leading-relaxed"> | ||
| <span class="shrink-0 mt-1 w-4 h-4 rounded-full ${meta.bg} border flex items-center justify-center"> | ||
| <svg class="w-2.5 h-2.5 ${meta.color}" fill="currentColor" viewBox="0 0 8 8"> | ||
| <circle cx="4" cy="4" r="3"/> | ||
| </svg> | ||
| </span> | ||
| ${tip} | ||
| </li> | ||
| `, | ||
| )} | ||
| </ul> | ||
| </div> | ||
|
|
||
| <!-- Video section --> | ||
| ${step.videoUrl | ||
| ? html` | ||
| <div> | ||
| <button | ||
| @click=${this.toggleVideo} | ||
| class="flex items-center gap-2 text-xs font-bold text-blue-400 hover:text-blue-300 transition-colors" | ||
| > | ||
| <svg class="w-4 h-4" fill="none" viewBox="0 0 24 24" stroke="currentColor"> | ||
| <polygon points="5 3 19 12 5 21 5 3" fill="currentColor"/> | ||
| </svg> | ||
| ${this.videoOpen ? "Hide Video" : `Watch: ${step.videoLabel ?? "Tutorial Video"}`} | ||
| </button> | ||
| ${this.videoOpen | ||
| ? html` | ||
| <div class="mt-3 rounded-xl overflow-hidden border border-white/10 aspect-video"> | ||
| <iframe | ||
| class="w-full h-full" | ||
| src="${step.videoUrl}" | ||
| title="Tutorial Video" | ||
| frameborder="0" | ||
| allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture" | ||
| allowfullscreen | ||
| ></iframe> | ||
| </div> | ||
| ` | ||
| : nothing} | ||
| </div> | ||
| ` | ||
| : nothing} | ||
|
|
||
| <!-- All-done celebration --> | ||
| ${allDone | ||
| ? html` | ||
| <div class="rounded-xl p-4 bg-emerald-500/10 border border-emerald-500/20 text-center"> | ||
| <div class="text-2xl mb-1">🎖️</div> | ||
| <p class="text-emerald-300 font-bold text-sm">All ${STEPS.length} lessons complete!</p> | ||
| <p class="text-white/50 text-xs mt-1">You've gone from recruit to commander. Now go win some games.</p> | ||
| <button | ||
| @click=${this.resetProgress} | ||
| class="mt-3 text-xs text-white/30 hover:text-white/60 transition-colors underline" | ||
| >Reset progress</button> | ||
| </div> | ||
| ` | ||
| : nothing} | ||
| </div> | ||
|
|
||
| <!-- Footer nav --> | ||
| <div class="shrink-0 px-5 py-4 border-t border-white/10 flex items-center justify-between gap-3"> | ||
| <button | ||
| ?disabled=${isFirst} | ||
| @click=${() => this.goTo(this.currentStep - 1)} | ||
| class="flex items-center gap-1.5 px-3 py-2 rounded-lg text-xs font-bold text-white/60 hover:text-white | ||
| hover:bg-white/5 transition-all disabled:opacity-30 disabled:cursor-not-allowed" | ||
| > | ||
| <svg class="w-4 h-4" fill="none" viewBox="0 0 24 24" stroke="currentColor"> | ||
| <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M15 19l-7-7 7-7"/> | ||
| </svg> | ||
| Prev | ||
| </button> | ||
|
|
||
| ${isDone | ||
| ? html` | ||
| <button | ||
| @click=${() => this.goTo(this.currentStep + 1)} | ||
| ?disabled=${isLast} | ||
| class="flex-1 px-4 py-2 rounded-lg text-sm font-bold bg-white/5 hover:bg-white/10 | ||
| text-white/60 transition-all disabled:opacity-30 disabled:cursor-not-allowed" | ||
| > | ||
| ${isLast ? "All done 🎖️" : "Next lesson →"} | ||
| </button> | ||
| ` | ||
| : html` | ||
| <button | ||
| @click=${() => this.markComplete(step.id)} | ||
| class="flex-1 px-4 py-2 rounded-lg text-sm font-bold | ||
| bg-emerald-500/20 hover:bg-emerald-500/30 text-emerald-300 | ||
| border border-emerald-500/30 transition-all" | ||
| > | ||
| ✓ Mark Complete | ||
| </button> | ||
| `} | ||
|
|
||
| <button | ||
| ?disabled=${isLast} | ||
| @click=${() => this.goTo(this.currentStep + 1)} | ||
| class="flex items-center gap-1.5 px-3 py-2 rounded-lg text-xs font-bold text-white/60 hover:text-white | ||
| hover:bg-white/5 transition-all disabled:opacity-30 disabled:cursor-not-allowed" | ||
| > | ||
| Next | ||
| <svg class="w-4 h-4" fill="none" viewBox="0 0 24 24" stroke="currentColor"> | ||
| <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M9 5l7 7-7 7"/> | ||
| </svg> | ||
| </button> | ||
| </div> | ||
| </div> | ||
| `; | ||
| } | ||
|
|
||
| private renderProgressBar() { | ||
| const totalDone = this.completed.size; | ||
| const pct = Math.round((totalDone / STEPS.length) * 100); | ||
|
|
||
| const tracks: Track[] = ["beginner", "intermediate", "advanced"]; | ||
|
|
||
| return html` | ||
| <div class="shrink-0 px-5 py-3 border-b border-white/10"> | ||
| <!-- Track pills --> | ||
| <div class="flex items-center gap-2 mb-2"> | ||
| ${tracks.map((t) => { | ||
| const meta = TRACK_META[t]; | ||
| const trackSteps = STEPS.filter((s) => s.track === t); | ||
| const trackDone = trackSteps.filter((s) => this.completed.has(s.id)).length; | ||
| const allTrackDone = trackDone === trackSteps.length; | ||
| return html` | ||
| <span class="flex items-center gap-1 px-2 py-0.5 rounded-full text-xs font-bold border | ||
| ${allTrackDone ? "bg-emerald-500/20 border-emerald-500/30 text-emerald-400" : `${meta.bg} ${meta.color}`}"> | ||
| ${allTrackDone ? "✓ " : ""}${meta.label} ${trackDone}/${trackSteps.length} | ||
| </span> | ||
| `; | ||
| })} | ||
| <span class="ml-auto text-xs text-white/30 font-mono">${pct}%</span> | ||
| </div> | ||
| <!-- Bar --> | ||
| <div class="h-1.5 bg-white/10 rounded-full overflow-hidden"> | ||
| <div | ||
| class="h-full rounded-full transition-all duration-500 bg-gradient-to-r from-emerald-500 via-blue-500 to-amber-500" | ||
| style="width: ${pct}%" | ||
| ></div> | ||
| </div> | ||
| </div> | ||
| `; | ||
| } | ||
|
|
||
| render() { | ||
| const step = STEPS[this.currentStep]; | ||
|
|
||
| const content = html` | ||
| <div class="${this.modalContainerClass} flex flex-row"> | ||
|
|
||
| <!-- Sidebar --> | ||
| <div class="hidden md:flex flex-col w-56 shrink-0 border-r border-white/10 overflow-hidden"> | ||
| <!-- Header --> | ||
| <div class="px-4 pt-4 pb-3 border-b border-white/10 flex items-center gap-2 shrink-0"> | ||
| <span class="text-lg">🎓</span> | ||
| <div> | ||
| <p class="text-xs font-bold text-white/90 uppercase tracking-widest">Learning Path</p> | ||
| <p class="text-[10px] text-white/40">${this.completed.size}/${STEPS.length} complete</p> | ||
| </div> | ||
| <button | ||
| @click=${() => this.close()} | ||
| class="ml-auto text-white/30 hover:text-white/70 transition-colors text-lg leading-none" | ||
| aria-label="Close" | ||
| >✕</button> | ||
| </div> | ||
|
|
||
| <!-- Step list --> | ||
| <div class="flex-1 overflow-y-auto px-2 py-2 space-y-0.5 | ||
| scrollbar-thin scrollbar-thumb-white/20 scrollbar-track-transparent"> | ||
| ${STEPS.map((s, i) => this.renderSidebarStep(s, i))} | ||
| </div> | ||
| </div> | ||
|
|
||
| <!-- Main content --> | ||
| <div class="flex flex-col flex-1 min-w-0 overflow-hidden"> | ||
| <!-- Mobile close --> | ||
| <div class="md:hidden flex items-center justify-between px-4 py-3 border-b border-white/10 shrink-0"> | ||
| <div class="flex items-center gap-2"> | ||
| <span class="text-lg">🎓</span> | ||
| <p class="text-xs font-bold text-white/90 uppercase tracking-widest">Learning Path</p> | ||
| </div> | ||
| <button | ||
| @click=${() => this.close()} | ||
| class="text-white/30 hover:text-white/70 transition-colors text-lg leading-none" | ||
| aria-label="Close" | ||
| >✕</button> | ||
| </div> | ||
|
|
||
| ${this.renderProgressBar()} | ||
| ${this.renderStepContent(step)} | ||
| </div> | ||
| </div> | ||
| `; | ||
|
|
||
| if (this.inline) { | ||
| return content; | ||
| } | ||
|
|
||
| return html` | ||
| <o-modal | ||
| id="onboardingModal" | ||
| title="Learning Path" | ||
| ?hideHeader=${true} | ||
| ?hideCloseButton=${true} | ||
| > | ||
| ${content} | ||
| </o-modal> | ||
| `; |
Contributor
There was a problem hiding this comment.
All UI labels and messages require i18n support.
The render methods contain numerous hardcoded UI strings that violate the i18n guideline:
- Button labels: "Hide Video", "Watch:", "Mark Complete", "Next", "Prev", "Reset progress"
- Messages: "All 13 lessons complete!", "You've gone from recruit to commander..."
- Aria labels: "Close"
- Headers: "Learning Path", "Key Tips"
- Status text: "complete", "Done", "Step X of Y"
All these strings should use translateText() with i18n keys from resources/lang/en.json.
As per coding guidelines: All user-visible text in src/client/**/*.{ts,tsx} must go through translateText() function with corresponding entries in resources/lang/en.json.
🤖 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/OnboardingModal.ts` around lines 402 - 660, Replace all hardcoded
user-facing strings in the render methods with calls to translateText(...) and
use interpolation for dynamic values: update renderStepContent (replace "Hide
Video", "Watch:", "✓ Mark Complete", "Next lesson →", "All X lessons complete!",
"You've gone from recruit to commander.", "Reset progress", "Key Tips", "Step
${this.currentStep + 1} of ${STEPS.length}", "Done", video title default) to
translateText with appropriate i18n keys and placeholders; update
renderProgressBar (replace "% complete" display, track pill labels like
`${meta.label}` and percent string) and render() sidebar/header (replace
"Learning Path", "complete", "Close" aria-label, "Prev", "Next") likewise;
ensure dynamic numbers (step index, STEPS.length, trackDone/trackSteps.length,
pct) are passed as interpolation params to translateText; verify keys exist in
resources/lang/en.json and fallback to existing step.videoLabel when present;
touch methods referenced here (renderStepContent, renderProgressBar, render) and
UI handlers (markComplete, resetProgress, close, toggleVideo) only to pass
translated labels.
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
- vercel.json: build with 'vite build', output dist/, SPA fallback rewrite - .env.production: VITE_API_DOMAIN=api.openfront.io, VITE_WS_HOST=openfront.io - Api.ts: getApiBase() checks VITE_API_DOMAIN first - LobbySocket.ts: wsUrl uses VITE_WS_HOST when set - Transport.ts: connectRemote() uses VITE_WS_HOST when set
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/client/Api.ts`:
- Around line 173-178: The code that builds the API base URL using viteApiDomain
(from (import.meta as unknown as { env?: Record<string,string>
}).env?.VITE_API_DOMAIN) always prefixes with "https://" which breaks local dev;
change the logic in Api.ts so that if VITE_API_DOMAIN already includes a
protocol (starts with "http://" or "https://") you return it as-is, otherwise if
the domain looks like localhost (e.g., starts with "localhost" or contains
"localhost:") default to "http://" + viteApiDomain, and for all other hosts
default to "https://" + viteApiDomain; ensure you update the return branch that
currently does return `https://${viteApiDomain}` accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cb8e5d5c-f6cc-4dfa-b9e7-29243736cccf
📒 Files selected for processing (6)
.env.production.gitignoresrc/client/Api.tssrc/client/LobbySocket.tssrc/client/Transport.tsvercel.json
✅ Files skipped from review due to trivial changes (3)
- .gitignore
- .env.production
- vercel.json
Comment on lines
+173
to
+178
| const viteApiDomain = ( | ||
| import.meta as unknown as { env?: Record<string, string> } | ||
| ).env?.VITE_API_DOMAIN; | ||
| if (viteApiDomain) { | ||
| return `https://${viteApiDomain}`; | ||
| } |
Contributor
There was a problem hiding this comment.
Hardcoded https:// may break local development.
When VITE_API_DOMAIN is set to a localhost address (e.g., localhost:8787), the code returns https://localhost:8787. Local dev servers typically run on http://, so this would fail to connect. The existing localStorage fallback at line 187 can include the protocol in the stored value (http://localhost:8787), but this new override cannot.
Consider allowing the protocol to be specified or defaulting to http:// when the domain starts with localhost:
🔧 Suggested fix to support http for localhost
export function getApiBase() {
const viteApiDomain = (
import.meta as unknown as { env?: Record<string, string> }
).env?.VITE_API_DOMAIN;
if (viteApiDomain) {
- return `https://${viteApiDomain}`;
+ const protocol = viteApiDomain.startsWith('localhost') ? 'http' : 'https';
+ return `${protocol}://${viteApiDomain}`;
}📝 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 viteApiDomain = ( | |
| import.meta as unknown as { env?: Record<string, string> } | |
| ).env?.VITE_API_DOMAIN; | |
| if (viteApiDomain) { | |
| return `https://${viteApiDomain}`; | |
| } | |
| const viteApiDomain = ( | |
| import.meta as unknown as { env?: Record<string, string> } | |
| ).env?.VITE_API_DOMAIN; | |
| if (viteApiDomain) { | |
| const protocol = viteApiDomain.startsWith('localhost') ? 'http' : 'https'; | |
| return `${protocol}://${viteApiDomain}`; | |
| } |
🤖 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/Api.ts` around lines 173 - 178, The code that builds the API base
URL using viteApiDomain (from (import.meta as unknown as { env?:
Record<string,string> }).env?.VITE_API_DOMAIN) always prefixes with "https://"
which breaks local dev; change the logic in Api.ts so that if VITE_API_DOMAIN
already includes a protocol (starts with "http://" or "https://") you return it
as-is, otherwise if the domain looks like localhost (e.g., starts with
"localhost" or contains "localhost:") default to "http://" + viteApiDomain, and
for all other hosts default to "https://" + viteApiDomain; ensure you update the
return branch that currently does return `https://${viteApiDomain}` accordingly.
TourOverlay.ts (new): - 7-step spotlight tour: Click Play → Join Game → Spawn → Conquer 10 Tiles → Build a City → Request Alliance → Nuclear Endgame - SVG cutout spotlight with animated violet ring around target element - RAF loop keeps spotlight synced to live element bounds - pointer-events:none backdrop lets clicks pass through to spotlit elements - Window event listeners for game actions (tour:spawn-clicked, tour:city-built, tour:alliance-sent, join-lobby) - Polling every 600ms for territory milestone (numTilesOwned >= 10) - Progress dots, Back button, Skip step, completion celebration card ClientGameRunner.ts: - Exposes window.__tourGameView after each game tick for tour polling - Dispatches tour:spawn-clicked after SendSpawnIntentEvent - Dispatches tour:alliance-sent after SendAllianceRequestIntentEvent BuildMenu.ts: - Dispatches tour:city-built when UnitType.City is built OnboardingModal.ts: - 'Start Interactive Tour' violet gradient CTA in progress bar - Calls getTourOverlay().activate() and closes self index.html: adds <tour-overlay> element
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/client/Main.ts (1)
37-39:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused imports.
Both
OnboardingModalandgetTourOverlayare imported but never used. ForOnboardingModal, you only need the side-effect registration; forgetTourOverlay, remove it entirely unless you plan to use it.♻️ Proposed fix
-import { hasSeenOnboarding, OnboardingModal } from "./OnboardingModal"; -import "./TourOverlay"; -import { getTourOverlay } from "./TourOverlay"; +import { hasSeenOnboarding } from "./OnboardingModal"; +import "./OnboardingModal"; +import "./TourOverlay";🤖 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/Main.ts` around lines 37 - 39, The file imports unused symbols: remove the unused named import OnboardingModal from "./OnboardingModal" (keep hasSeenOnboarding or keep the side-effect import only if you rely on its module registration) and remove the unused getTourOverlay import from "./TourOverlay" while retaining the side-effect import "./TourOverlay" if that module needs to run on load; update the import line(s) so only hasSeenOnboarding (or a plain side-effect import) is imported from "./OnboardingModal" and delete the getTourOverlay specifier from the "./TourOverlay" import.
🧹 Nitpick comments (3)
src/client/TourOverlay.ts (3)
201-211: ⚡ Quick winEvent listener cleanup may fail silently.
The
detachAllListenersmethod assumes the play button still exists in the DOM and attempts to remove listeners that may not have been attached. Ifstep.id !== "click-play", you never attached abtnlistener, so line 208 does nothing. Similarly, if there's nocompletionEvent, line 205 won't match the actual listener.Consider tracking which listeners were actually attached, or rely on
{ once: true }(already used on lines 188, 196) to auto-cleanup and skip manual removal entirely.♻️ Simpler approach
Since you already use
{ once: true }on event listeners (lines 188, 196), those listeners auto-remove after firing. You only need to clean up if the step changes before the event fires. You could track theAbortControllerpattern or just clear the Map without callingremoveEventListeneron elements that may not exist:private detachAllListeners() { - for (const [, handler] of this.boundHandlers) { - const step = STEPS[this.stepIndex]; - if (step?.completionEvent) { - window.removeEventListener(step.completionEvent, handler); - } - const btn = document.querySelector("[data-page='page-matchmaking']"); - btn?.removeEventListener("click", handler); - } + // Listeners added with { once: true } auto-cleanup on fire. + // Only remove if step transitions before they fire. + const step = STEPS[this.stepIndex]; + if (step?.completionEvent) { + const handler = this.boundHandlers.get("completion-event"); + if (handler) window.removeEventListener(step.completionEvent, handler); + } + if (step?.id === "click-play") { + const handler = this.boundHandlers.get("btn-click"); + const btn = document.querySelector("[data-page='page-matchmaking']"); + if (handler && btn) btn.removeEventListener("click", handler); + } this.boundHandlers.clear(); }🤖 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/TourOverlay.ts` around lines 201 - 211, The detachAllListeners method currently assumes listeners exist and removes them blindly; instead record the actual targets and event types when you call addEventListener so detachAllListeners can remove only what was attached. Update the logic that populates boundHandlers (and references to STEPS and stepIndex) to store an object per key like { target, eventType, handler } for the click-play step and for completionEvent listeners, then in detachAllListeners iterate those records and call removeEventListener on the recorded target/eventType/handler (skip if target is null) or alternatively use an AbortController per step and call controller.abort() to cancel pending listeners when changing steps. This ensures you only remove listeners that were actually attached and avoids silent failures.
301-303: 💤 Low valueExtract magic tooltip dimensions as constants.
The tooltip width (
TW = 320), height (TH = 200), and gap (GAP = 16) are used only in this method, but extracting them as class-level or file-level constants would improve readability and make future adjustments easier.♻️ Proposed change
At the top of the class or file:
const TOOLTIP_WIDTH = 320; const TOOLTIP_HEIGHT = 200; const TOOLTIP_GAP = 16;Then use
TOOLTIP_WIDTH,TOOLTIP_HEIGHT,TOOLTIP_GAPincomputeTooltipPos.🤖 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/TourOverlay.ts` around lines 301 - 303, The magic numbers for tooltip sizing inside computeTooltipPos should be pulled out as named constants to improve readability and reuse: define TOOLTIP_WIDTH = 320, TOOLTIP_HEIGHT = 200, and TOOLTIP_GAP = 16 at the file or class level (above the TourOverlay class or as static members) and replace TW, TH, and GAP usages in computeTooltipPos with these new constants (update any references inside TourOverlay.computeTooltipPos accordingly).
112-114: 💤 Low valueRemove unused
savedStepfunction.The
savedStep()helper is defined but never called. Remove it to reduce dead code.♻️ Proposed fix
-function savedStep(): number { - return parseInt(localStorage.getItem(STORAGE_KEY) ?? "0", 10) || 0; -} function saveStep(n: number) { localStorage.setItem(STORAGE_KEY, String(n)); }🤖 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/TourOverlay.ts` around lines 112 - 114, Remove the unused helper function savedStep() from src/client/TourOverlay.ts by deleting the entire function declaration; after removal, run a quick scan for the STORAGE_KEY symbol and any other variables only used by savedStep (and remove those if they are now unused) to keep the module free of dead code and unused imports.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/client/TourOverlay.ts`:
- Around line 35-106: STEPS currently contains hard-coded user-visible strings
(title, body, manualCta) which must be passed through translateText(); replace
each literal in the STEPS array with
translateText("tour.step.<stepId>.(title|body|manualCta)") calls (e.g., for id
"click-play" use keys like "tour.step.click_play.title" and
"tour.step.click_play.body"), and add the corresponding entries to
resources/lang/en.json for every step (include missing manualCta keys only when
present). Update target entries for STEP ids with underscore-normalized keys
(click-play -> click_play) to match the JSON keys and ensure translateText is
imported/available in src/client/TourOverlay.ts.
---
Duplicate comments:
In `@src/client/Main.ts`:
- Around line 37-39: The file imports unused symbols: remove the unused named
import OnboardingModal from "./OnboardingModal" (keep hasSeenOnboarding or keep
the side-effect import only if you rely on its module registration) and remove
the unused getTourOverlay import from "./TourOverlay" while retaining the
side-effect import "./TourOverlay" if that module needs to run on load; update
the import line(s) so only hasSeenOnboarding (or a plain side-effect import) is
imported from "./OnboardingModal" and delete the getTourOverlay specifier from
the "./TourOverlay" import.
---
Nitpick comments:
In `@src/client/TourOverlay.ts`:
- Around line 201-211: The detachAllListeners method currently assumes listeners
exist and removes them blindly; instead record the actual targets and event
types when you call addEventListener so detachAllListeners can remove only what
was attached. Update the logic that populates boundHandlers (and references to
STEPS and stepIndex) to store an object per key like { target, eventType,
handler } for the click-play step and for completionEvent listeners, then in
detachAllListeners iterate those records and call removeEventListener on the
recorded target/eventType/handler (skip if target is null) or alternatively use
an AbortController per step and call controller.abort() to cancel pending
listeners when changing steps. This ensures you only remove listeners that were
actually attached and avoids silent failures.
- Around line 301-303: The magic numbers for tooltip sizing inside
computeTooltipPos should be pulled out as named constants to improve readability
and reuse: define TOOLTIP_WIDTH = 320, TOOLTIP_HEIGHT = 200, and TOOLTIP_GAP =
16 at the file or class level (above the TourOverlay class or as static members)
and replace TW, TH, and GAP usages in computeTooltipPos with these new constants
(update any references inside TourOverlay.computeTooltipPos accordingly).
- Around line 112-114: Remove the unused helper function savedStep() from
src/client/TourOverlay.ts by deleting the entire function declaration; after
removal, run a quick scan for the STORAGE_KEY symbol and any other variables
only used by savedStep (and remove those if they are now unused) to keep the
module free of dead code and unused imports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c0b36cb-1204-4fa7-a4ef-61b291095b59
📒 Files selected for processing (6)
index.htmlsrc/client/ClientGameRunner.tssrc/client/Main.tssrc/client/OnboardingModal.tssrc/client/TourOverlay.tssrc/client/graphics/layers/BuildMenu.ts
✅ Files skipped from review due to trivial changes (2)
- index.html
- src/client/ClientGameRunner.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/client/OnboardingModal.ts
Comment on lines
+35
to
+106
| const STEPS: StepDef[] = [ | ||
| { | ||
| id: "click-play", | ||
| emoji: "🎮", | ||
| title: "Click PLAY", | ||
| body: "Tap the PLAY button in the left sidebar to open the game lobby. That's your gateway to live matches.", | ||
| targetSelector: "[data-page='page-matchmaking']", | ||
| targetPadding: 10, | ||
| tooltipSide: "right", | ||
| completionEvent: "tour:play-clicked", | ||
| }, | ||
| { | ||
| id: "join-game", | ||
| emoji: "🌍", | ||
| title: "Join a Game", | ||
| body: "Pick any Free For All game from the list — the World map is great for beginners. Click it to join.", | ||
| targetSelector: "game-mode-selector", | ||
| targetPadding: 16, | ||
| tooltipSide: "bottom", | ||
| completionEvent: "join-lobby", | ||
| }, | ||
| { | ||
| id: "spawn", | ||
| emoji: "📍", | ||
| title: "Choose Your Spawn", | ||
| body: "Click any green (plains) land tile to place your nation. Aim for the center of the map — it gives you room to expand in every direction.", | ||
| targetSelector: "canvas", | ||
| targetPadding: 0, | ||
| tooltipSide: "bottom", | ||
| completionEvent: "tour:spawn-clicked", | ||
| }, | ||
| { | ||
| id: "expand", | ||
| emoji: "⚔️", | ||
| title: "Conquer 10 Tiles", | ||
| body: "Click neighboring territory to attack it. Keep the attack slider (bottom left) around 20–30%. Reach 10 tiles to continue.", | ||
| targetSelector: "control-panel", | ||
| targetPadding: 12, | ||
| tooltipSide: "top", | ||
| completionPoll: "tiles-10", | ||
| }, | ||
| { | ||
| id: "build-city", | ||
| emoji: "🏙️", | ||
| title: "Build Your First City", | ||
| body: "Right-click anywhere on your territory — the build menu opens. Select City to increase your population cap. More pop = more troops every tick.", | ||
| targetSelector: "build-menu", | ||
| targetPadding: 12, | ||
| tooltipSide: "left", | ||
| completionEvent: "tour:city-built", | ||
| manualCta: "I built a city →", | ||
| }, | ||
| { | ||
| id: "alliance", | ||
| emoji: "🤝", | ||
| title: "Request an Alliance", | ||
| body: "Right-click a neighboring player's territory and choose Request Alliance. Allied nations can't attack each other — use it to secure your flanks while you grow.", | ||
| targetSelector: "canvas", | ||
| targetPadding: 0, | ||
| tooltipSide: "bottom", | ||
| completionEvent: "tour:alliance-sent", | ||
| manualCta: "Alliance sent →", | ||
| }, | ||
| { | ||
| id: "nukes", | ||
| emoji: "☢️", | ||
| title: "The Nuclear Endgame", | ||
| body: "When you control 40%+: right-click your territory → build a Missile Silo (100k gold) → launch at the biggest threat. Target players with no SAM launchers. MIRVs fire multiple warheads and overwhelm any SAM defense. Time your strike when opponents are busy fighting each other.", | ||
| tooltipSide: "center", | ||
| manualCta: "Got it — I'm ready to dominate →", | ||
| }, | ||
| ]; |
Contributor
There was a problem hiding this comment.
All user-visible text must use translateText().
The STEPS array contains hard-coded English strings for title, body, and manualCta. As per coding guidelines, all user-visible text in src/client/**/*.{ts,tsx} must go through the translateText() function with corresponding entries in resources/lang/en.json.
Example refactor for one step
In resources/lang/en.json, add:
{
"tour.step.click_play.title": "Click PLAY",
"tour.step.click_play.body": "Tap the PLAY button in the left sidebar to open the game lobby. That's your gateway to live matches."
}Then in the code:
{
id: "click-play",
emoji: "🎮",
- title: "Click PLAY",
- body: "Tap the PLAY button in the left sidebar to open the game lobby. That's your gateway to live matches.",
+ title: translateText("tour.step.click_play.title"),
+ body: translateText("tour.step.click_play.body"),
targetSelector: "[data-page='page-matchmaking']",Apply this pattern to all steps.
As per coding guidelines: src/client/**/*.{ts,tsx}: All user-visible text must go through translateText() function with corresponding entries in resources/lang/en.json.
🤖 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/TourOverlay.ts` around lines 35 - 106, STEPS currently contains
hard-coded user-visible strings (title, body, manualCta) which must be passed
through translateText(); replace each literal in the STEPS array with
translateText("tour.step.<stepId>.(title|body|manualCta)") calls (e.g., for id
"click-play" use keys like "tour.step.click_play.title" and
"tour.step.click_play.body"), and add the corresponding entries to
resources/lang/en.json for every step (include missing manualCta keys only when
present). Update target entries for STEP ids with underscore-normalized keys
(click-play -> click_play) to match the JSON keys and ensure translateText is
imported/available in src/client/TourOverlay.ts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a 13-step, self-guided Learning Path that takes new players from zero to competitive — covering Beginner fundamentals, Intermediate strategy, and Advanced mastery tips.
Why this matters
OpenFront has a steep learning curve for new players. The existing Help modal contains a tutorial video, but there's no structured progression. Players either figure it out through trial and error or watch long YouTube videos. This feature provides a lightweight, in-game path that bridges both.
What's new
OnboardingModal.ts(new file)A Lit Web Component with:
openfront-onboarding-v1) — progress survives page reloadsinline(page nav) and overlay (modal) rendering modesLearning Path content
Other changes
index.html: registers<onboarding-modal id="page-onboarding">as inline page-contentMain.ts: auto-navigates first-time visitors after 600ms;hasSeenOnboarding()prevents repeat auto-openDesktopNavBar.ts: adds "Guide" nav button (emerald color)MobileNavBar.ts: adds "Learning Path" nav item (emerald color)Testing
Notes
openfront-onboarding-shownkey gates the auto-open;openfront-onboarding-v1stores step progress