Add /stats career-stats page - #46
Open
tuggernuts1123 wants to merge 2 commits into
Open
Conversation
Public IP-resolved page (same flow as /namechange) showing the visitor's career move usage, accuracy, parries, ringouts, and movement stats from their PlayerStats.aggregate document. - New /stats route in server.ts: resolves player by IP via the existing resolvePlayerForWeb helper (renders the account picker if multiple accounts share an IP), loads PlayerStats by account_id, returns a rendered page with the aggregate JSON inlined. - New static page src/static/my_stats.html with the moves table (sorted by Used desc, computed Hit % and % of all client-side) plus defense / damage / movement summary cards. Charge Attack shows its own row but is excluded from the % of all denominator. - "My Stats" tile added to /home. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reviewer's Guide/stats career stats page added, resolving the caller via the existing IP-based account flow, rendering a new stats HTML template with aggregated player stats and client-side computed move/summary tables, and exposing it via a new home tile. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
my_stats.htmlpage reimplements a lot of layout/styling that appears to be shared with the name-change flow (background, wrapper, card styles); consider extracting a shared partial or CSS so future tweaks to that look-and-feel only need to be made in one place. - The client-side stats script hardcodes a large set of aggregate field keys (e.g.,
totalNeutralGroundNormalUsed,totalRingoutsEnemyLowPercent); it may be more maintainable to centralize these mappings (or derive them from a shared schema/module) so changes toPlayerStats.aggregatedon’t require updating multiple scattered string literals.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `my_stats.html` page reimplements a lot of layout/styling that appears to be shared with the name-change flow (background, wrapper, card styles); consider extracting a shared partial or CSS so future tweaks to that look-and-feel only need to be made in one place.
- The client-side stats script hardcodes a large set of aggregate field keys (e.g., `totalNeutralGroundNormalUsed`, `totalRingoutsEnemyLowPercent`); it may be more maintainable to centralize these mappings (or derive them from a shared schema/module) so changes to `PlayerStats.aggregate` don’t require updating multiple scattered string literals.
## Individual Comments
### Comment 1
<location path="src/static/my_stats.html" line_range="366-373" />
<code_context>
+ const platDrops = num("totalPlatformDropThroughs");
+ const taunts = num("totalTauntsUsed");
+
+ const moveCards = [
+ ["Total Jumps", fmt(jumps)],
+ ["Double Jumps", fmt(doubleJumps)],
+ ["Air Time (s)", fmt(Math.round(airTime))],
+ ["Walk Time (s)", fmt(Math.round(walkTime))],
+ ["Wall Hang Time (s)", fmt(Math.round(wallHang))],
+ ["Platform Drops", fmt(platDrops)],
+ ["Taunts", fmt(taunts)],
+ ];
+ for (const [label, value] of moveCards) {
</code_context>
<issue_to_address>
**suggestion:** Consider reusing a helper to render stat cards to reduce repetition across grids.
These sections all iterate over `[label, value]` pairs to build identical `.stat_card` elements. Pulling that logic into a helper like `renderCards(containerId, cards)` would centralize the structure and make future layout/style changes easier to maintain.
Suggested implementation:
```
// Shared helper to render stat cards into a grid container
function renderStatCards(container, cards) {
for (const [label, value] of cards) {
const c = document.createElement("div");
c.className = "stat_card";
c.innerHTML = '<div class="label">' + label + '</div><div class="value">' + value + '</div>';
container.appendChild(c);
}
}
// ─── Movement grid ─────────────────────────────────────────────
const movementGrid = document.getElementById("movement_grid");
const jumps = num("totalJumps");
const doubleJumps = num("totalDoubleJumps");
const airTime = num("totalAirTime");
const walkTime = num("totalWalkTime");
const wallHang = num("totalWallHangTime");
const platDrops = num("totalPlatformDropThroughs");
const taunts = num("totalTauntsUsed");
const moveCards = [
["Total Jumps", fmt(jumps)],
["Double Jumps", fmt(doubleJumps)],
["Air Time (s)", fmt(Math.round(airTime))],
["Walk Time (s)", fmt(Math.round(walkTime))],
["Wall Hang Time (s)", fmt(Math.round(wallHang))],
["Platform Drops", fmt(platDrops)],
["Taunts", fmt(taunts)],
];
renderStatCards(movementGrid, moveCards);
})();
</script>
{{/if}}
</body>
</html>
```
There are likely other sections in this file that build `.stat_card` grids by looping over `[label, value]` pairs (e.g., for damage, knockouts, etc.). For each of those sections, replace the inline `for (const [label, value] of someCards)` loops that create `.stat_card` elements with calls to `renderStatCards(targetGridElement, someCards)` to fully centralize the rendering logic.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Sourcery feedback: the defense / damage / movement grids each duplicated the same loop that built .stat_card elements. Centralize it in a single renderStatCards(containerId, cards) helper. Bonus: switch from innerHTML to textContent for the label and value elements so future data shifts can't inject markup. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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
/statspage that resolves the visitor by IP (same flow as/namechange— account picker shown if multiple accounts share an IP) and renders their career stats fromPlayerStats.aggregate./home.Test plan
/statsfrom an IP with an existing PlayerStats document → moves table + summary cards render correctly with real data./statsfrom a fresh IP / account with no aggregate → friendly "No stats yet" screen./statsfrom an IP with multiple accounts → existing account picker is shown.—in the % of all column./homeand links to/stats./statsreturns to/home.🤖 Generated with Claude Code
Summary by Sourcery
Add a public career stats page resolved by IP and link it from the home screen.
New Features:
Enhancements: