Skip to content

Feature: Elo being displayed on the PlayPage (#3029) #3048

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
xTonai wants to merge 3 commits into from
Closed

Feature: Elo being displayed on the PlayPage (#3029) #3048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
30dcfb9
Feature: Elo now displayed on the main menu (#3029)
xTonai Jan 26, 2026
6e56c53
Feature: Elo now displayed on the main menu (#3029)
xTonai Jan 26, 2026
8112256
Merge remote-tracking branch 'origin/feature/3029-elo-display' into f…
xTonai Jan 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions resources/lang/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -968,6 +968,7 @@
},
"matchmaking_button": {
"play_ranked": "1v1 Ranked Matchmaking",
"elo": "ELO: ",
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

matchmaking_button.elo has a trailing space ("ELO: "). Trailing whitespace in translations is fragile and makes it harder for translators/tools to manage spacing/punctuation.

Prefer removing the trailing space and handling spacing in markup/CSS, or switch this string to a placeholder format (e.g., "ELO: {elo}") and render it as a single translated string.

Suggested change
"elo": "ELO: ",
"elo": "ELO:",

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@evanpelle evanpelle Jan 27, 2026

Choose a reason for hiding this comment

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

please fix, instead use vairables "ELO: {elo}"

"description": "(ALPHA)",
"login_required": "Login to play ranked!",
"must_login": "You must be logged in to play ranked matchmaking."
Expand Down
40 changes: 39 additions & 1 deletion src/client/components/PlayPage.ts
View file
Open in desktop
Copy link
Collaborator

@evanpelle evanpelle Jan 27, 2026

Choose a reason for hiding this comment

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

I think all this logic should be in Matchmaking.ts

Copy link
Contributor Author

@xTonai xTonai Jan 28, 2026

Choose a reason for hiding this comment

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

Fair point. I'll fix it tomorrow. I don't have any time today sadly.

Copy link
Contributor Author

@xTonai xTonai Jan 29, 2026

Choose a reason for hiding this comment

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

The button it is attached to is located in PlayPage.ts. I would have to migrate him to Matchmaking first, unless I am not missing out on something here.

Original file line number Diff line number Diff line change
@@ -1,8 +1,35 @@
import { LitElement, html } from "lit";
import { customElement } from "lit/decorators.js";
import { customElement, state } from "lit/decorators.js";
import { UserMeResponse } from "../../core/ApiSchemas";

@customElement("play-page")
export class PlayPage extends LitElement {
@state() private elo: string = "...";
private _onUserMeResponse: (e: Event) => void;
Comment on lines +7 to +8
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

PR description indicates relevant tests were added, but this change set only updates PlayPage.ts and en.json with no corresponding test changes. If the intent is to add coverage, please include a client/component test that verifies the ELO text updates when a userMeResponse event is dispatched.

Copilot uses AI. Check for mistakes.

connectedCallback() {
super.connectedCallback();

this._onUserMeResponse = (e: Event): void => {
const event = e as CustomEvent<UserMeResponse | false>;

if (!event.detail) {
this.elo = "...";
return;
}

this.elo =
event.detail?.player?.leaderboard?.oneVone?.elo?.toString() ?? "...";
};

document.addEventListener("userMeResponse", this._onUserMeResponse);
}
Comment on lines +10 to +26
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 27, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

rg -n 'userMeResponse' --type ts --type js -C 3

Repository: openfrontio/OpenFrontIO

Length of output: 17824

🏁 Script executed:

rg -n 'dispatchEvent' --type ts --type js | grep -i 'usermeresp\|userme'

Repository: openfrontio/OpenFrontIO

Length of output: 49

🏁 Script executed:

fd -e ts -e js | xargs grep -l 'dispatchEvent.*userMeResponse\|new CustomEvent.*[Uu]serMe'

Repository: openfrontio/OpenFrontIO

Length of output: 85

🏁 Script executed:

rg -n 'onUserMe\s*=' src/client/Main.ts -A 30 | head -80

Repository: openfrontio/OpenFrontIO

Length of output: 1247

🏁 Script executed:

rg -n 'onUserMe\s*\(' src/client/Main.ts -B 5 -A 2

Repository: openfrontio/OpenFrontIO

Length of output: 246

🏁 Script executed:

sed -n '440,550p' src/client/Main.ts

Repository: openfrontio/OpenFrontIO

Length of output: 3619

🏁 Script executed:

rg -n 'getUserMe' src/client/Main.ts -B 2 -A 5

Repository: openfrontio/OpenFrontIO

Length of output: 641

🏁 Script executed:

rg -n 'window.showPage\|page-matchmaking' src/client/Main.ts | head -20

Repository: openfrontio/OpenFrontIO

Length of output: 49

🏁 Script executed:

find src -name "*.ts" -type f | xargs grep -l 'page-matchmaking' | head -5

Repository: openfrontio/OpenFrontIO

Length of output: 111

🏁 Script executed:

cat -n src/client/Api.ts | grep -A 10 'export function getUserMe'

Repository: openfrontio/OpenFrontIO

Length of output: 49

🏁 Script executed:

sed -n '1,50p' src/client/PlayPage.ts

Repository: openfrontio/OpenFrontIO

Length of output: 132

🏁 Script executed:

find src -name "*PlayPage*" -o -name "*playpage*" -o -name "*play-page*"

Repository: openfrontio/OpenFrontIO

Length of output: 100

🏁 Script executed:

rg -l 'class PlayPage' --type ts

Repository: openfrontio/OpenFrontIO

Length of output: 100

🏁 Script executed:

rg 'export function getUserMe' -A 15

Repository: openfrontio/OpenFrontIO

Length of output: 49

🏁 Script executed:

rg 'getUserMe' src/client/Api.ts -A 15

Repository: openfrontio/OpenFrontIO

Length of output: 512

🏁 Script executed:

cat -n src/client/components/PlayPage.ts | head -50

Repository: openfrontio/OpenFrontIO

Length of output: 1947

Read ELO from cached state on connect, not just from event listener.

The userMeResponse event is dispatched asynchronously via getUserMe().then(onUserMe) in Main.ts line 510. If PlayPage connects after this event fires, the listener will miss it and elo stays as "...". Store the last userMeResponse in Main.ts and read it in PlayPage's connectedCallback as fallback.

🤖 Prompt for AI Agents 
In `@src/client/components/PlayPage.ts` around lines 10 - 26,
PlayPage.connectedCallback currently only sets this.elo in the userMeResponse
event handler (_onUserMeResponse) and misses the value if the event already
fired; update the flow so PlayPage reads a cached last userMe response from Main
before attaching the listener: add a simple cache accessor in Main (e.g., store
the last response when getUserMe resolves and expose getLastUserMeResponse or a
module-level variable) and in PlayPage.connectedCallback call that getter (or
read the exported cached value) to initialize this.elo the same way as in
_onUserMeResponse (using event.detail.player.leaderboard.oneVone.elo) as a
fallback, then attach document.addEventListener for future events.


disconnectedCallback() {
super.disconnectedCallback();
document.removeEventListener("userMeResponse", this._onUserMeResponse);
}

createRenderRoot() {
return this;
}
Expand Down Expand Up @@ -159,6 +186,17 @@ export class PlayPage extends LitElement {
class="relative z-10 text-2xl"
data-i18n="matchmaking_button.play_ranked"
></span>
<span>
<span
class="relative z-10 text-xs font-medium text-purple-100 opacity-90 group-hover:opacity-100 transition-opacity"
data-i18n="matchmaking_button.elo"
></span>
Comment on lines +189 to +193
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

The ELO label/value is split across two s and the translation string includes formatting ("ELO: ") to create spacing. This makes localization harder (languages may need to reorder or format the whole "ELO: {value}" differently) and relies on trailing whitespace in the translation.

Consider rendering a single localized string with an ICU placeholder (similar to matchmaking_modal.elo in src/client/Matchmaking.ts) and passing the current elo as a param, or otherwise avoid encoding spacing/punctuation in the translation.

Copilot uses AI. Check for mistakes.
<span
class="relative z-10 text-xs font-medium text-purple-100 opacity-90 group-hover:opacity-100 transition-opacity"
>
${this.elo}</span
>
</span>
<span
class="relative z-10 text-xs font-medium text-purple-100 opacity-90 group-hover:opacity-100 transition-opacity"
data-i18n="matchmaking_button.description"
Expand Down
Loading