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

Fix rounding error in player count #43

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

bluelhf
Copy link

@bluelhf bluelhf commented Mar 20, 2023

This PR updates index.tsx to use Math.floor() instead of Math.round() when displaying bStats player data. This ensures values like 30,600 do not get displayed as 31K+

P.S. Love the new website :)

use Math.floor() instead of Math.round() to fix values like "30600" being displayed as "31K+"
@MiniDigger
Copy link
Member

I think this was done intentionally
bstats is only the lower limit on player count, we dont know the actual (but know its much higher), so rounding up is fine

@olijeffers0n
Copy link
Member

olijeffers0n commented Jul 19, 2023

It’s still incorrect though. Let’s say that bstats reports 10,600 players and the true amount is 10,800. Rounding to 11k+ is wrong and this PR would fix it.

I know you said that we can assume the real amount is much higher but this isn’t always the case.

@MiniDigger
Copy link
Member

conservative estimates say that 50%+ of all paper servers have bstats disabled (many hosts disable it for some dum reason).

@bluelhf
Copy link
Author

bluelhf commented Jul 20, 2023

I think this was done intentionally bstats is only the lower limit on player count, we dont know the actual (but know its much higher), so rounding up is fine

Looks to be intentional: 20385f3 (see commit message). Still, it feels wrong to report a lower bound that could be higher than the actual value, however slim that possibility may be. What do you think?

@PaperMC PaperMC deleted a comment from vercel bot Aug 6, 2024
@PaperMC PaperMC deleted a comment from vercel bot Aug 6, 2024
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.

4 participants