site stats page alpha by king-of-the-hive · Pull Request #660 · hiveboardgame/hive

king-of-the-hive · 2025-11-06T06:24:56Z

Just a very simple rough first try for the site stats page.

If you guys can try on live data and post the screenshot would be great. Not in the pages directory yet akin to puzzles and whatnot.

I have some ideas on what else can be added but I need think how to get some more fake data in my local db...

IongIer · 2025-11-07T08:05:14Z

A few quick thoughts:

Grey text doesn't work well in darkmode.
The wins by color table should probably filter out casual games if it doesn't already.
The White > 0 and Black > 0 are confusing to me and the data looks off, what do they represent and how come they don't have the same number of games?
4.Also the numbers don't seem to sum up properly. 35 total white wins but adding em up in the table leads to less, same for the black ones. Is it because the table actually only sums up rated games whereas the totals include casual?....yeah this seems to be the case, the totals should probably mention % of which are rated or something like that.

As for what to add. Not sure would need to think about it.

IongIer · 2025-11-07T08:16:50Z

db/src/models/game.rs

+            ""
+        };
+
+        let query = format!(r#"


We never use raw sql, but diesel functions. That said this might be hard to achieve without raw sql but give it a try

I tried! It cannot do multiple group bys one of which is a case statement...

king-of-the-hive · 2025-11-07T12:21:57Z

The wins by color table should probably filter out casual games if it doesn't already.

Yes, those are rated. I'll add a note.

The White > 0 and Black > 0 are confusing to me and the data looks off, what do they represent and how come they don't have the same number of games?

Those are basically Black or White has a small advantage between 0 and 100. I'll think about labels. But the number is not the same, either white has higher rating in a game or black.

Also the numbers don't seem to sum up properly. 35 total white wins but adding em up in the table leads to less, same for the black ones. Is it because the table actually only sums up rated games whereas the totals include casual?....yeah this seems to be the case, the totals should probably mention % of which are rated or something like that.

Yup yup will add a clarification!

IongIer · 2025-11-11T09:28:23Z

I really dislike the huge type and function names like SiteStatisticsWinrateByRatingDifferenceResponse, totally a taste thing tho dunno if felix agrees.
Also this seems like a lot more code than I would expect is needed for this feature but I could be wrong.

king-of-the-hive · 2025-11-11T12:25:00Z

I really dislike the huge type and function names like SiteStatisticsWinrateByRatingDifferenceResponse, totally a taste thing tho dunno if felix agrees. Also this seems like a lot more code than I would expect is needed for this feature but I could be wrong.

I'm happy to optimize names and code, but need hints for the direction 🙂

king-of-the-hive · 2025-11-13T04:26:01Z

I guess I could move all the aggregate functions out of Game or User objects directly into the response file. Not that it actually reduces the amount of code but at least keeps the objects free of complex functions. (Might be not possible though, all the diesel stuff should be in db?)

I will reduce the names too.

Other than that I don't see a lot of potential for the code to be more concise. It does a lot of aggregations to display a ton of data...

IongIer · 2025-11-13T07:17:18Z

I guess I could move all the aggregate functions out of Game or User objects directly into the response file. Not that it actually reduces the amount of code but at least keeps the objects free of complex functions. (Might be not possible though, all the diesel stuff should be in db?)

I will reduce the names too.

Other than that I don't see a lot of potential for the code to be more concise. It does a lot of aggregations to display a ton of data...

I will get back to you on this when I've had a more thorough think. But yeah all the diesel stuff needs to stay in the db so don't move that out.

IongIer · 2025-11-13T07:52:25Z

apis/src/pages/statistics.rs

+
+#[component]
+pub fn Statistics() -> impl IntoView {
+    let (selected_speed, set_selected_speed) = signal("All speeds".to_string());


1.We have types for speed and gametype don't use strings like "All speeds" if the type can't easily do what you want it to do you can add methods to the type for that. This probably happens elsewhere as well, wherever you use a string you need to use a type, the type get to_stringed when going into the db and get converted back into types in the responses and handed back as types to the frontend.

2.Unless you have a good reason to have separate read and write signals (like you are using a library that needs separate signals) use RwSignal instead with RwSignal::new(...) and that way you have both get and set on the same signal. The codebase uses RwSignals everywhere and separate read and write rarely.

IongIer · 2025-11-13T08:41:08Z

apis/src/components/atoms/site_statistics_generic_card.rs

+                                            let label_for_fallback = label.clone();
+                                            let label_for_error = label.clone();
+
+                                            let user_resource = LocalResource::new(move || {


Maybe instead of looping over n users here and doing another db call for each, the response for this type should contain the complete user data (get a vector of users from the user model based on what the game model returns and that way it's two queries not n+1)

IongIer · 2025-11-13T08:47:38Z

apis/src/components/atoms/site_statistics_selectors.rs

+                        <button
+                            type="button"
+                            class=move || {
+                                if selected_period.get() == period {


In this case this is fine because SiteStatisticsTimePeriod is just a plain enum but if were anything more complex you want to use .with and that way you do the comparison without cloning the value.

selected_period.with(|p| p == period)

IongIer · 2025-11-13T08:51:11Z

apis/src/components/atoms/site_statistics_selectors.rs

+            {speeds
+                .iter()
+                .map(|speed| {
+                    let speed_clone = speed.clone();


switching to types instead of strings here will avoid all the needing to clone multiple times.
In general if you find yourself needing to do a bunch of let x,x1,x2 = y.clone()... consider using let x = StoredValue::new(y) and wherever you need that value you call x.get_value()

IongIer · 2025-11-13T08:52:50Z

apis/src/components/atoms/site_statistics_single_stat_card.rs

+                .map(|add_val| {
+                    view! {
+                        <div class="text-m text-gray-500 dark:text-gray-300 ml-1">
+                            {format!("{:.1}", add_val)} "%"


Clippy will complain about this, use the add_value inside the format string where it's possible. Once you are done with the pr check yourself by running cargo clippy and follow it's suggestions.

king-of-the-hive · 2025-11-18T19:12:13Z

I'm yet to tackle issues raised by Ion, so don't look at it yet :D

king-of-the-hive · 2025-11-21T08:31:20Z

I think I fixed all issues and rebased to the current code so marking ready for review.

king-of-the-hive · 2025-11-21T08:33:54Z

Some of the files touched are formatters' work. I can revert those if necessary.

IongIer · 2025-11-27T12:50:59Z

I think I fixed all issues and rebased to the current code so marking ready for review.

Please check that it still compiles after a rebase. In this case it doesn't as there are duplicate definitions

error[E0428]: the name `GameRatings` is defined multiple times
--> db/src/models/game.rs:127:1
|
53  | pub struct GameRatings {
| ---------------------- previous definition of the type `GameRatings` here
...
127 | pub struct GameRatings {
| ^^^^^^^^^^^^^^^^^^^^^^ `GameRatings` redefined here
|
= note: `GameRatings` must be defined only once in the type namespace of this module

error[E0592]: duplicate definitions with name `get_rating_history_for_player`
--> db/src/models/game.rs:1706:5
|
1303 | /     pub async fn get_rating_history_for_player(
1304 | |         player: Uuid,
1305 | |         game_speed: &GameSpeed,
1306 | |         conn: &mut DbConn<'_>,
1307 | |     ) -> Result<Vec<GameRatings>, DbError> {
| |__________________________________________- other definition for `get_rating_history_for_player`
...
1706 | /     pub async fn get_rating_history_for_player(
1707 | |         player: Uuid,
1708 | |         game_speed: &GameSpeed,
1709 | |         conn: &mut DbConn<'_>,
1710 | |     ) -> Result<Vec<GameRatings>, DbError> {
| |__________________________________________^ duplicate definitions for `get_rating_history_for_player`

king-of-the-hive · 2025-11-27T18:13:12Z

I think I fixed all issues and rebased to the current code so marking ready for review.

Please check that it still compiles after a rebase. In this case it doesn't as there are duplicate definitions

Oops, deleted the duplicates.

klautcomputing

I only skimmed this, there is some stuff I would like to change. Most importantly there should not be changes to the engine, the raw SQL queries (this is why we use an ORM) and the VeryLongJavaLikeNames.
I will give this a better look tomorrow with Ion and leave you some more detailed comments.

klautcomputing · 2025-12-01T12:33:00Z

engine/src/game_type.rs

    MLP,
 }

+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Copy, Default)]


I don't think you should touch the engine in this PR :)

I just did what Ion asked me to do... it's just some extra methods on the enum, it doesn't touch any substantial logic. I could move this new logic into a new shared type or something, but that mean breaking DRY. Anyhow, any specific suggestions appreciated.

I might not have expressed myself clearly enough, but when I said add methods to the types, I didn't also mean extend an enum with an extra variant for example, like GameSpeed:AllSpeeds. I was trying to say use what's already there from that type instead of using special strings to achieve your goal.

The engine has no need for GameTypeFilter or the SQL stuff, why would we keep it in the engine then? Also it doesn't make the code DRYer, the enum could live somewhere else. If you compile the the engine's code now, it is actually dead code...

I might not have expressed myself clearly enough, but when I said add methods to the types, I didn't also mean extend an enum with an extra variant for example, like GameSpeed:AllSpeeds. I was trying to say use what's already there from that type instead of using special strings to achieve your goal.

If the speeds should be passed as a type between the db through responses to the component as you suggested then I'm not sure how to do it without adding to the type itself. I need AllSpeeds for almost all aggregates where you see UNION ALL. I'd really appreciate specific suggestions instead of just "you can't do this" comments.

(This also extends to the raw SQL comments: I checked the documentation and different GPTs, there's no way to have multiple group bys by calculated columns. They themselves admit that in their readme: https://github.com/diesel-rs/diesel?tab=readme-ov-file#raw-sql . If you have a specific way in mind on how to do it, please let me know, I'm happy to rewrite it. )

king-of-the-hive · 2025-12-01T21:39:11Z

I understand the raw SQL concern but it would be significantly more complex code and instead of pulling aggregates it would load thousand of games into memory without any substantial benefit.

The long names I shortened some already, if you have any specific you don't like I can F2 no problem :)

IongIer · 2025-12-02T09:49:10Z

I understand the raw SQL concern but it would be significantly more complex code and instead of pulling aggregates it would load thousand of games into memory without any substantial benefit.

The long names I shortened some already, if you have any specific you don't like I can F2 no problem :)

More complex maybe but diesel can express the queries you wrote and writing them in diesel wouldn't mean loading the games in memory, it can load exactly what the raw sql is loading.

IongIer · 2025-12-02T09:51:08Z

One thing that is missing is this /statistics page is never exposed as a link anywhere internally. There should be buttons in the dropdowns leading to the page.

klautcomputing

@IongIer and I checked out the feature and we have some feedback for you on how it work.
I am going to look at the code a bit more when it's working nicely :)

klautcomputing · 2025-12-02T10:10:03Z

shared_types/src/site_statistics_time_period.rs

+#[derive(
+    Debug, Serialize, PartialEq, Eq, Deserialize, Clone, Copy, Hash, PartialOrd, Ord, Default,
+)]
+pub enum SiteStatisticsTimePeriod {


Suggested change

pub enum SiteStatisticsTimePeriod {

pub enum TimePeriod {

klautcomputing · 2025-12-02T10:22:09Z

apis/src/pages/statistics.rs

+
+                                        view! {
+                                            <h1 class="text-xl font-bold text-center">
+                                                "Player ratings distribution"


I don't understand why it's always 1.0, this doesn't make sense to me, doesn't it need to add up to 1 in total? And 1.0 is 100%?

This just means the users in the db don't have enough games probably and there's exactly one user for each bucket (buckets are step 10). Try to create 200-300 games per user?

All users have 50+ games, also then the scale doesn't make sense because there can't be fractional users :)
I am gonna retry with more users and games and see whether I can get different results.

It should look something like this with enough rankable users. The graph is fractional only because of super small test db.

klautcomputing · 2025-12-02T10:25:13Z

apis/src/components/atoms/site_statistics_selectors.rs

+}
+
+#[component]
+pub fn SpeedSelector(speeds: Vec<GameSpeed>, selected_speed: RwSignal<GameSpeed>) -> impl IntoView {


I think it doesn't make sense to repeat all the buttons if they are all linked.

Also clicking one of them makes the whole layout change and jump. I think the right thing to do here is to have the speedselectors and the other selectors (time period, include bot, ...) all be in one floating header so that you always have access to them and don't need to scroll around.

klautcomputing · 2025-12-02T10:26:08Z

engine/src/game_type.rs


+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Copy, Default)]
+pub enum GameTypeFilter {
+    Base,


Not sure where to leave this comment so here it goes: Base games should not display elo statistics as we have no elo for base games :)

klautcomputing · 2025-12-02T10:27:05Z

apis/src/components/atoms/rating_buckets_chart.rs

+                fallback=|| {
+                    view! {
+                        <div class="text-center py-8 text-gray-500 dark:text-gray-400">
+                            "No data available"


It's weird that the layout changes and that there's not even a heading anymore when you don't have data available.

yeah I guess I can try to make a common floating selector and that would actually allow to get rid of the "Games by type and speed" table, the speed selector will just update the single stat cards. And will fix the headers when data is missing.

king-of-the-hive · 2025-12-07T02:07:01Z

I made filters collapsible and floating and moved stuff out of the engine, plus cleaned up some minor things. I don't see a clean way to get rid of AllSpeeds and raw SQL, but, again, if you do, please let me know what it looks like.

king-of-the-hive · 2025-12-07T02:09:56Z

/statistics page is never exposed as a link anywhere internally.

I think it's ok if it's not exposed at first tbh, I can make a further diff for that. I'd like to see what the page looks like on live data.

IongIer reviewed Nov 7, 2025

View reviewed changes

king-of-the-hive force-pushed the site-stats-alpha branch 2 times, most recently from 32c74c2 to 248951b Compare November 11, 2025 05:57

IongIer reviewed Nov 13, 2025

View reviewed changes

king-of-the-hive force-pushed the site-stats-alpha branch from 248951b to 5e98a88 Compare November 18, 2025 19:10

king-of-the-hive marked this pull request as draft November 18, 2025 19:11

king-of-the-hive force-pushed the site-stats-alpha branch 6 times, most recently from 2880d42 to 9085c08 Compare November 21, 2025 08:28

king-of-the-hive marked this pull request as ready for review November 21, 2025 08:29

king-of-the-hive force-pushed the site-stats-alpha branch from 9085c08 to 1318693 Compare November 27, 2025 18:11

klautcomputing requested changes Dec 1, 2025

View reviewed changes

klautcomputing requested changes Dec 2, 2025

View reviewed changes

king-of-the-hive force-pushed the site-stats-alpha branch from 1318693 to dce4f5f Compare December 7, 2025 01:17

site stats page alpha

0178ae7

king-of-the-hive force-pushed the site-stats-alpha branch from dce4f5f to 0178ae7 Compare December 7, 2025 02:04

Conversation

king-of-the-hive commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IongIer commented Nov 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

king-of-the-hive commented Nov 7, 2025

Uh oh!

IongIer commented Nov 11, 2025

Uh oh!

king-of-the-hive commented Nov 11, 2025

Uh oh!

king-of-the-hive commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IongIer commented Nov 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

king-of-the-hive commented Nov 18, 2025

Uh oh!

king-of-the-hive commented Nov 21, 2025

Uh oh!

king-of-the-hive commented Nov 21, 2025

Uh oh!

IongIer commented Nov 27, 2025

Uh oh!

king-of-the-hive commented Nov 27, 2025

Uh oh!

klautcomputing left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

king-of-the-hive commented Dec 1, 2025

Uh oh!

IongIer commented Dec 2, 2025

Uh oh!

IongIer commented Dec 2, 2025

Uh oh!

klautcomputing left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

king-of-the-hive commented Nov 6, 2025 •

edited

Loading

king-of-the-hive commented Nov 13, 2025 •

edited

Loading

king-of-the-hive Dec 2, 2025 •

edited

Loading

king-of-the-hive commented Dec 7, 2025 •

edited

Loading