Skip to content
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.

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

Add stats #275

Merged
merged 3 commits into from
Jul 1, 2023
Merged

Add stats #275

merged 3 commits into from
Jul 1, 2023

Conversation

dlakata
Copy link
Contributor

@dlakata dlakata commented Jun 21, 2023

Similar to #247, I thought it could be helpful to see the times of one's previous solves. As far as I could tell, the stats modules weren't used, so I repurposed them for this.

Let me know what you think!

crossword_stats

@vercel
Copy link

vercel bot commented Jun 21, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
downforacross.com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 24, 2023 3:02am

@dlakata
Copy link
Contributor Author

dlakata commented Jun 21, 2023

from the preview link above, it seems that the new stats endpoint isn't accessible? it's working locally for me, so I'm not sure what the cause might be

@stevenhao
Copy link
Member

Hey thanks for this PR! Love the idea, I'll take a look. The vercel preview is frontend-only, so it makes sense that your server-side changes aren't reflected.

@stevenhao
Copy link
Member

At first glance things look good -- maybe can add # players to History? + whether the game is in-progress or solved (and change the link to /replay/gid if it's completed)

@dlakata
Copy link
Contributor Author

dlakata commented Jun 21, 2023

I also was interested in getting the player count! But I was finding that the backend isn't aware of multiple users, where game_events don't actually have a user populated on them. Could you give me some pointers here?

If you'd prefer this, I can definitely rework things to handle incomplete games. My original thinking was that "stats" would only be useful for complete games, and https://downforacross.com/?complete=0&in_progress=1&new=0 could be used to see incomplete games. What do you think?

I updated all the links to the replay link!

}, []);
user.listUserHistory().then((history: any) => {
console.log(`keys: ${_.keys(history)}`);
fetchStats({gids: _.keys(history)}).then((s) => {
Copy link
Member

Choose a reason for hiding this comment

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

can we cap this at like, last 500 games? i'm not an expert in database performance but i'm worried that the backend could get slow if we try to do too many at once since the query involves joining with the game_events table (and there's no index on game event type afaik)

className="molester-moon"
style={darkModePreference !== '0' ? {opacity: 1} : {}}
onClick={toggleMolesterMoons}
>
Dark Mode (beta): {darkModePreferenceText(darkModePreference)}
</div>
<div>
<a href="/stats">Stats</a>
Copy link
Member

Choose a reason for hiding this comment

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

could we style this a bit differently? On my computer it becomes purple after clicked which is a bit hard to see. Just making it white w/ underline is probably fine.

and name the link "Your Stats" or something so it's not confused with global site stats

one final thing i noticed is that the secret dark mode button's text kinda overlaps with this, so maybe we need some margin-left (maybe 40px?)
image

@stevenhao
Copy link
Member

i checked this out locally and it looks great! i left some small nitpicky comments inline, happy to merge after those are addressed.

as a follow-up after this is merged, i think it'd be a good idea to do a backfill and eagerly store & maintain the stats in the db to avoid having to do expensive queries. this would also allow us to track stats like number of distinct users + anything else we want to include in the stats page (there may be user feedback that we can act on).

@dlakata
Copy link
Contributor Author

dlakata commented Jun 24, 2023

I think I made all your suggested changes! and added another log line to help measure the performance of the endpoint. another index that might be good would be on puzzle_solves prefixed with gid rather than pid

idk what your database maintenance looks like, but yeah would be happy to help. when marking a puzzle solve, we could probably record the revealed/checked count. I wonder what stats other users would want (filtering different conditions, per time period, longest/current streak, day-of-week stats), and user feedback + performance could tell us what to improve

@stevenhao
Copy link
Member

great, thanks! merging this, i'll try to keep an eye on performance but not overly worried for now since there's limits on how many games are considered at once for stats

@stevenhao stevenhao merged commit 78f22a7 into downforacross:master Jul 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants