fix(crazygames): guest username on logout, hide fullscreen, in-game pop-ups#4520
Closed
evanpelle wants to merge 2 commits into
Closed
fix(crazygames): guest username on logout, hide fullscreen, in-game pop-ups#4520evanpelle wants to merge 2 commits into
evanpelle wants to merge 2 commits into
Conversation
…op-ups Addresses four CrazyGames platform requests: - Username reverts to a fresh guest (AnonXXX) name when the player logs out of their CrazyGames account, instead of keeping the account name. - Hide our in-game fullscreen button on CrazyGames (the frame provides one). - Replace native browser confirm()/alert() dialogs shown during gameplay with an in-game pop-up (new InGameModal): exit-game confirmation, kick confirmation, host-left, and connection-refused. - Report gameplayStop to CrazyGames whenever the Settings menu or a pop-up is open (previously only when it also paused the sim), resuming on close. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
WalkthroughReplaces native alert and confirm dialogs with shared in-game dialog helpers, updates several caller flows to use them, and adjusts CrazyGames gameplay pause, fullscreen, and username logout handling. ChangesIn-game dialogs and CrazyGames flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/client/hud/layers/SettingsModal.ts (1)
110-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame duplicated logic as
GraphicsSettingsModal.pauseGame.See the extraction suggestion left on
src/client/hud/layers/GraphicsSettingsModal.ts(lines 169-183) — this block is identical and would benefit from sharing a single helper.🤖 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/hud/layers/SettingsModal.ts` around lines 110 - 124, The pauseGame logic in SettingsModal duplicates the same behavior already implemented in GraphicsSettingsModal.pauseGame, so extract the shared CrazyGames/eventBus pause handling into a common helper and have both modals call it. Keep the helper focused on the pause/resume flow driven by pause, wasPausedWhenOpened, shouldPause, crazyGamesSDK, and eventBus so the two classes share one source of truth.src/client/hud/layers/GraphicsSettingsModal.ts (1)
169-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCorrect fix, but duplicated verbatim in
SettingsModal.ts.The CrazyGames reporting logic (decoupling
gameplayStop/gameplayStartfromshouldPause) is correct and matches the PR intent. However this exact 15-line block is copy-pasted intoSettingsModal.pauseGame(lines 110-124). Any future tweak to this logic risks drifting between the two files. Consider extracting it into a small shared helper (e.g. inCrazyGamesSDK.tsor a shared util) that both modals call.♻️ Example extraction
// CrazyGamesSDK.ts (or a small shared helper module) export function reportCrazyGamesPause(pause: boolean, wasPausedWhenOpened: boolean) { if (pause) { crazyGamesSDK.gameplayStop(); } else if (!wasPausedWhenOpened) { crazyGamesSDK.gameplayStart(); } }private pauseGame(pause: boolean) { - // CrazyGames: report gameplay as stopped whenever this settings menu is open, - // and resumed when it closes — unless the game was already paused when opened. - if (pause) { - crazyGamesSDK.gameplayStop(); - } else if (!this.wasPausedWhenOpened) { - crazyGamesSDK.gameplayStart(); - } + reportCrazyGamesPause(pause, this.wasPausedWhenOpened); // Only pause the simulation itself when we own the pause (singleplayer or // lobby creator). if (this.shouldPause && !this.wasPausedWhenOpened) { this.eventBus.emit(new PauseGameIntentEvent(pause)); } }🤖 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/hud/layers/GraphicsSettingsModal.ts` around lines 169 - 183, The CrazyGames pause/reporting logic in GraphicsSettingsModal.pauseGame is correct, but it is duplicated verbatim in SettingsModal.pauseGame, so future changes may drift between the two. Extract the shared gameplayStop/gameplayStart behavior into a small helper (for example in CrazyGamesSDK or a shared util) and have both pauseGame methods call it, keeping only the modal-specific PauseGameIntentEvent logic in each class.
🤖 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/InGameModal.ts`:
- Around line 92-107: Add the missing translation entries used by InGameModal so
translateText("common.cancel"), translateText("common.confirm"), and
translateText("common.close") resolve correctly. Update the English resource
data in resources/lang/en.json to include these new common keys with appropriate
button labels, keeping them alongside the existing common translations so the
InGameModal button text remains localized.
In `@src/client/Transport.ts`:
- Line 391: The showInGameAlert call in Transport.ts is using a hardcoded
English message, so replace the inline string with a translateText() lookup and
add the matching connection_error.refused entry in resources/lang/en.json. Keep
the dynamic event.reason part in the translated message and update the Transport
connection error handling path to use the localized text consistently with the
other converted call sites.
---
Nitpick comments:
In `@src/client/hud/layers/GraphicsSettingsModal.ts`:
- Around line 169-183: The CrazyGames pause/reporting logic in
GraphicsSettingsModal.pauseGame is correct, but it is duplicated verbatim in
SettingsModal.pauseGame, so future changes may drift between the two. Extract
the shared gameplayStop/gameplayStart behavior into a small helper (for example
in CrazyGamesSDK or a shared util) and have both pauseGame methods call it,
keeping only the modal-specific PauseGameIntentEvent logic in each class.
In `@src/client/hud/layers/SettingsModal.ts`:
- Around line 110-124: The pauseGame logic in SettingsModal duplicates the same
behavior already implemented in GraphicsSettingsModal.pauseGame, so extract the
shared CrazyGames/eventBus pause handling into a common helper and have both
modals call it. Keep the helper focused on the pause/resume flow driven by
pause, wasPausedWhenOpened, shouldPause, crazyGamesSDK, and eventBus so the two
classes share one source of truth.
🪄 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: 27670e41-ce46-4f48-8362-8ad5a5260913
📒 Files selected for processing (9)
src/client/ClientGameRunner.tssrc/client/InGameModal.tssrc/client/Main.tssrc/client/Transport.tssrc/client/UsernameInput.tssrc/client/hud/layers/GameRightSidebar.tssrc/client/hud/layers/GraphicsSettingsModal.tssrc/client/hud/layers/PlayerModerationModal.tssrc/client/hud/layers/SettingsModal.ts
Comment on lines
+92
to
+107
| ${this.showCancel | ||
| ? html`<button | ||
| class="px-4 py-2 rounded-md text-sm font-medium text-white bg-slate-600 hover:bg-slate-500 transition-colors" | ||
| @click=${() => this.close(false)} | ||
| > | ||
| ${translateText("common.cancel")} | ||
| </button>` | ||
| : null} | ||
| <button | ||
| class="px-4 py-2 rounded-md text-sm font-medium text-white bg-blue-600 hover:bg-blue-500 transition-colors" | ||
| @click=${() => this.close(true)} | ||
| > | ||
| ${this.showCancel | ||
| ? translateText("common.confirm") | ||
| : translateText("common.close")} | ||
| </button> |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'en.json' resources/lang | xargs -I{} sh -c 'echo "--- {} ---"; jq -e ".\"common.cancel\", .\"common.confirm\", .\"common.close\"" {}'Repository: openfrontio/OpenFrontIO
Length of output: 211
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== translateText key usage in src/client/InGameModal.ts =="
sed -n '80,115p' src/client/InGameModal.ts
echo
echo "== matching keys in resources/lang/en.json =="
rg -n '"common\.(cancel|confirm|close)"' resources/lang/en.json || true
echo
echo "== surrounding common keys in resources/lang/en.json =="
rg -n '"common\.' resources/lang/en.json | head -n 40Repository: openfrontio/OpenFrontIO
Length of output: 1596
Add the new translation keys to resources/lang/en.json The buttons now call translateText("common.cancel"), translateText("common.confirm"), and translateText("common.close"), but those entries are missing from the English resource file.
🤖 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/InGameModal.ts` around lines 92 - 107, Add the missing translation
entries used by InGameModal so translateText("common.cancel"),
translateText("common.confirm"), and translateText("common.close") resolve
correctly. Update the English resource data in resources/lang/en.json to include
these new common keys with appropriate button labels, keeping them alongside the
existing common translations so the InGameModal button text remains localized.
Source: Coding guidelines
| if (event.code === 1002) { | ||
| // TODO: make this a modal | ||
| alert(`connection refused: ${event.reason}`); | ||
| showInGameAlert(`connection refused: ${event.reason}`); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Untranslated user-facing string.
`connection refused: ${event.reason}` is hardcoded English and bypasses translateText(). 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." Every other call site converted in this PR (ClientGameRunner, Main, GameRightSidebar, PlayerModerationModal) correctly wraps its message in translateText().
🌐 Proposed fix
- showInGameAlert(`connection refused: ${event.reason}`);
+ showInGameAlert(
+ translateText("connection_error.refused", { reason: event.reason }),
+ );(Add a matching connection_error.refused entry to 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
| showInGameAlert(`connection refused: ${event.reason}`); | |
| showInGameAlert( | |
| translateText("connection_error.refused", { reason: event.reason }), | |
| ); |
🤖 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/Transport.ts` at line 391, The showInGameAlert call in
Transport.ts is using a hardcoded English message, so replace the inline string
with a translateText() lookup and add the matching connection_error.refused
entry in resources/lang/en.json. Keep the dynamic event.reason part in the
translated message and update the Transport connection error handling path to
use the localized text consistently with the other converted call sites.
Source: Coding guidelines
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
Instead of a hand-rolled modal, the showInGameConfirm/showInGameAlert helpers now drive the existing <confirm-dialog> component so the styling stays consistent with the rest of the app. Keeps the promise-based API (callable from non-Lit contexts) and the gameplayStop/Start reporting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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/InGameModal.ts`:
- Around line 16-32: The showDialog flow currently resumes gameplay as soon as
any single modal closes, which can incorrectly unpause while another dialog is
still open. Update the InGameModal showDialog logic to track the number of open
dialogs with a shared ref-count, incrementing when a dialog is appended and
decrementing in the confirm/cancel completion path before calling
gameplayStart(). Use the existing showDialog, done, and
crazyGamesSDK.gameplayStop/gameplayStart symbols to keep gameplay paused until
the last visible modal is removed.
🪄 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: 829da905-e5a3-44e7-934e-66f1993f6094
📒 Files selected for processing (1)
src/client/InGameModal.ts
Comment on lines
+16
to
+32
| function showDialog(props: Partial<ConfirmDialog>): Promise<boolean> { | ||
| const dialog = document.createElement("confirm-dialog") as ConfirmDialog; | ||
| Object.assign(dialog, props); | ||
|
|
||
| crazyGamesSDK.gameplayStop(); | ||
| document.body.appendChild(dialog); | ||
|
|
||
| return new Promise<boolean>((resolve) => { | ||
| const done = (result: boolean) => { | ||
| dialog.remove(); | ||
| crazyGamesSDK.gameplayStart(); | ||
| resolve(result); | ||
| }; | ||
| dialog.addEventListener("confirm", () => done(true)); | ||
| dialog.addEventListener("cancel", () => done(false)); | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '\bshowInGame(Confirm|Alert)\s*\(' src/client -C2Repository: openfrontio/OpenFrontIO
Length of output: 2901
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## InGameModal.ts\n'
sed -n '1,120p' src/client/InGameModal.ts | cat -n
printf '\n## Caller snippets\n'
sed -n '380,430p' src/client/Transport.ts | cat -n
printf '\n'
sed -n '590,630p' src/client/Main.ts | cat -n
printf '\n'
sed -n '210,235p' src/client/ClientGameRunner.ts | cat -n
printf '\n'
sed -n '60,85p' src/client/hud/layers/PlayerModerationModal.ts | cat -n
printf '\n'
sed -n '190,210p' src/client/hud/layers/GameRightSidebar.ts | cat -nRepository: openfrontio/OpenFrontIO
Length of output: 8339
Ref-count in-game dialogs before resuming gameplay. These dialogs can come from separate flows, so closing one can call gameplayStart() while another is still visible. A small open-dialog counter here would keep gameplay paused until the last modal closes.
🤖 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/InGameModal.ts` around lines 16 - 32, The showDialog flow
currently resumes gameplay as soon as any single modal closes, which can
incorrectly unpause while another dialog is still open. Update the InGameModal
showDialog logic to track the number of open dialogs with a shared ref-count,
incrementing when a dialog is appended and decrementing in the confirm/cancel
completion path before calling gameplayStart(). Use the existing showDialog,
done, and crazyGamesSDK.gameplayStop/gameplayStart symbols to keep gameplay
paused until the last visible modal is removed.
github-project-automation
Bot
moved this from Development
to Complete
in OpenFront Release Management
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
1 participant
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.
Addresses four requests from the CrazyGames platform team.
1. Username reverts to guest on CrazyGames logout
When a player logged into their CrazyGames account and then logged out, the username stayed the CrazyGames name. It now reverts to a fresh guest (
AnonXXX) name. AcrazyGamesLoggedInflag guards this so it only fires on a real login→logout transition (not on the initial "not logged in" state, which would otherwise wipe a stored name).2. Fullscreen button hidden on CrazyGames
CrazyGames provides its own fullscreen control in the game frame, so our in-game fullscreen button is now hidden when running on CrazyGames (and unchanged everywhere else).
3. Native browser prompts → in-game pop-ups
New
InGameModalprovides promise-basedshowInGameConfirm()/showInGameAlert()rendered as a real in-game modal (styled to match the Settings modal). Every nativeconfirm()/alert()shown during gameplay now uses it:// TODO: make this a modal)4.
gameplayStopwhen Settings menu / pop-up is opengameplayStop()while shown andgameplayStart()on dismiss.gameplayStopwhenever the menu is open — previously it only fired when the modal also paused the sim (singleplayer / lobby-creator), so regular multiplayer players never triggered it. Also fixes graphics-settings so gameplay resumes correctly on close for those players.Scope
Per request, this covers in-game dialogs only. The main-menu native alerts (store/checkout, account, subscriptions, friends, login) were intentionally left as-is.
Testing
tsc --noEmit,eslint, andprettier --checkall pass.falseand dismisses the overlay.gameplayStop, fullscreen hiding) are verified by code inspection — they require running inside the actual CrazyGames frame.🤖 Generated with Claude Code