Conversation
Replace the three lobby buttons (pattern, flag, effects) with two: Flag and Cosmetics. The new cosmetics-modal has Skins and Effects tabs (absorbing TerritoryPatternsModal and EffectsModal, which are removed along with their input buttons). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WalkthroughThe client replaces separate territory-pattern and effects controls with a combined cosmetics input and modal. The modal supports skin and effects tabs, updated selection behavior, new routing and event names, refreshed localization keys, and mobile and desktop layout integration. ChangesCosmetics selection flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client/CosmeticsInput.ts (1)
119-132: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winDuplicate ID in the document.
Because
CosmeticsInputusescreateRenderRoot() { return this; }, it renders its content directly into the Light DOM. SincePlayPage.tsplaces two of these components on the screen (one for mobile, one for desktop), we end up with multiple buttons sharingid="cosmetics-input". Having duplicate IDs is invalid HTML and can cause weird bugs with styling or scripting. Let's just remove theidproperty from both buttons!
src/client/CosmeticsInput.ts#L119-L132: Removeid="cosmetics-input"from the loading state button.src/client/CosmeticsInput.ts#L150-L156: Removeid="cosmetics-input"from the primary preview 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/CosmeticsInput.ts` around lines 119 - 132, Remove the id="cosmetics-input" attribute from both buttons rendered by CosmeticsInput: the loading-state button and the primary preview button. Update both affected sites in src/client/CosmeticsInput.ts (lines 119-132 and 150-156), leaving their other attributes and rendering behavior unchanged.
🤖 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/Main.ts`:
- Around line 445-451: Update the cosmetics-input click listener to safely
invoke the modal through optional chaining, so the callback does nothing when
cosmeticsModal is unavailable while preserving the existing open behavior when
it exists.
---
Outside diff comments:
In `@src/client/CosmeticsInput.ts`:
- Around line 119-132: Remove the id="cosmetics-input" attribute from both
buttons rendered by CosmeticsInput: the loading-state button and the primary
preview button. Update both affected sites in src/client/CosmeticsInput.ts
(lines 119-132 and 150-156), leaving their other attributes and rendering
behavior unchanged.
🪄 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: b0dd4c6c-41b6-460e-985e-e2d6e456c314
📒 Files selected for processing (9)
index.htmlresources/lang/en.jsonsrc/client/CosmeticsInput.tssrc/client/CosmeticsModal.tssrc/client/EffectsInput.tssrc/client/EffectsModal.tssrc/client/LangSelector.tssrc/client/Main.tssrc/client/components/PlayPage.ts
💤 Files with no reviewable changes (2)
- src/client/EffectsModal.ts
- src/client/EffectsInput.ts
| // Attach listener to any cosmetics-input component | ||
| document.querySelectorAll("cosmetics-input").forEach((cosmeticsInput) => { | ||
| cosmeticsInput.addEventListener("cosmetics-input-click", () => { | ||
| cosmeticsModal.open(); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a safe check before calling open.
If for some reason cosmeticsModal is not found, calling .open() will cause a crash. It is best to use optional chaining here, just like how it is safely checked for the flag input modal above!
🛡️ Proposed fix
// Attach listener to any cosmetics-input component
document.querySelectorAll("cosmetics-input").forEach((cosmeticsInput) => {
cosmeticsInput.addEventListener("cosmetics-input-click", () => {
- cosmeticsModal.open();
+ cosmeticsModal?.open();
});
});📝 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.
| // Attach listener to any cosmetics-input component | |
| document.querySelectorAll("cosmetics-input").forEach((cosmeticsInput) => { | |
| cosmeticsInput.addEventListener("cosmetics-input-click", () => { | |
| cosmeticsModal.open(); | |
| }); | |
| }); | |
| // Attach listener to any cosmetics-input component | |
| document.querySelectorAll("cosmetics-input").forEach((cosmeticsInput) => { | |
| cosmeticsInput.addEventListener("cosmetics-input-click", () => { | |
| cosmeticsModal?.open(); | |
| }); | |
| }); |
🤖 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 445 - 451, Update the cosmetics-input click
listener to safely invoke the modal through optional chaining, so the callback
does nothing when cosmeticsModal is unavailable while preserving the existing
open behavior when it exists.
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
github-project-automation
Bot
moved this from Development
to Complete
in OpenFront Release Management
## What Adds **crowns** as a new cosmetic category, wired end-to-end like flags/skins, plus a Crowns tab in the lobby cosmetics modal (#4617). Catalog shape in cosmetics.json: ```json "crowns": { "gold_crown": { "name": "gold_crown", "url": "...", "affiliateCode": null, "product": null, "priceHard": 5, "artist": "...", "rarity": "common" } } ``` ## Changes - **Core**: `CrownSchema` + optional `crowns` record in `CosmeticsSchema`; `crownName` in `PlayerCosmeticRefs`; resolved `crown: { name, url }` in `PlayerCosmetics` - **Server**: `isCrownAllowed` in the privilege checker — requires a `crown:*` or `crown:<name>` flare, resolves the ref to the catalog URL - **Client**: crown selection persisted under the `crown` localStorage key (stale/unowned selections self-clear), `crownRelationship` + resolve/refs wiring, `"crown"` purchase type, Crowns tab in the store, and a Crowns tab in the cosmetics modal (owned crowns + Default tile; selecting persists and keeps the modal open) - **i18n**: `store.crowns`, `store.no_crowns` - Tests for schema parsing, privilege checks, and cosmetic resolution ## Not in this PR - In-game rendering of `player.cosmetics.crown` (name pass / leaderboards / player panel) - API-side support (`/shop/purchase` accepting `cosmeticType: "crown"`, granting `crown:<name>` flares, serving `crowns` in cosmetics.json) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
What
Replaces the three lobby cosmetic buttons (skin, flag, effects) with two: Flag and Cosmetics. The new Cosmetics button opens a single tabbed modal (
#modal=cosmetics) with:Changes
CosmeticsModalandCosmeticsInput(lobby button previews the selected skin/pattern, shows a "Cosmetics" label on defaults, hidden on CrazyGames viano-crazygames)TerritoryPatternsModal,EffectsModal,PatternInput,EffectsInputand their wiring in Main.ts / index.html / LangSelectorcosmetics.title/button_title/search; removed the keys orphaned by the deleted componentsNotes
#modal=territory-patternsand#modal=effectsno longer resolve🤖 Generated with Claude Code