WebHost: Round percentage of checks, fix possible 500 error #2270
Conversation
|
I approve of the concept of this change, though I'm not familiar enough with the Jinja to make an educated assessment. |
|
the | operator works similar to a unix command line, it "pipes" the left stuff further with that modification. So in this case the number is rounded and then converted to an integer. |
|
Ah, okay. Seems good to me then, though I have not tested it myself. |
There was a problem hiding this comment.
I'm wondering if the flooring (i.e. int, not round) was intentional so it wouldn't show 100% unless you really have 100%?
Other than that, it might be nicer to move some processing from jinja to python (i.e. pass in the data differently), but i think reworking that is probably out of scope for this PR.
Maybe, but I feel like rounding rather than truncating is still more intuitive to non-programmers If someone sees 299/300 and then percent 100, I'd expect them to say "oh okay not every check is done but the percentage of checks is rounded, makes sense, that's what the raw numbers to the left are there for" Whereas seeing 57/100 show 56% tends to make heads explode for folks who don't understand IEEE 754 floats - it looks like a bug. I guess we could say "round except if it would make it 100% with checks done != total locations" but that seems messy to me |
|
Could also multiply first and then do integer division to avoid the pesky float errors |
|
What if we rounded the percentage to two decimal places? Then we could give a more accurate representation without taking up too much space. |
Here's what that would look like:
I'm happy to swap this PR to that if folks prefer it. |
|
I like it. Others? |
ThePhar
added
is: bug/fix
|
I feel like two decimal places is the appropriate number. People stop paying attention after the 2nd and this is still intuitive to people even if they struggled with mathematics |
How the percentage of checks done should be displayed is a display concern, so it makes sense to just always do it in the template. That way, along with using .format() instead of .round, means we always get exactly the same presentation regardless of whether it ends in .00 (which would not round to two decimal places), is an int (which `round(2)` wouldn't touch at all), etc.
Done. I've switched from using Also made more sense to me to just always do the rounding in the template as it's a presentation concern so probably should be in the template and that way we dont need the |
|
I was all ready to approve, but then I decided to test things. The LttP tracker gives too many decimals.
|
Crap. Thanks for catching that... I'll fix the lttp tracker and double check the Factorio tracker as well tomorrow. |
|
Fixed the rounding: Generic
LttP:
Factorio
The LttP multitracker showing 0.0 for non-LttP games isn't a regression it shows 0 on main too. I'm going to see if I can fix it while I'm here though |
|
And, fixed |
…agoMW#2270) * WebHost: Round percentage of checks, fix possible 500 error * Round using str.format in the template How the percentage of checks done should be displayed is a display concern, so it makes sense to just always do it in the template. That way, along with using .format() instead of .round, means we always get exactly the same presentation regardless of whether it ends in .00 (which would not round to two decimal places), is an int (which `round(2)` wouldn't touch at all), etc. * Round percent_total_checks_done in lttp multitracker * Fix non-LttP games showing as 0% done in LttP MultiTracker
…agoMW#2270) * WebHost: Round percentage of checks, fix possible 500 error * Round using str.format in the template How the percentage of checks done should be displayed is a display concern, so it makes sense to just always do it in the template. That way, along with using .format() instead of .round, means we always get exactly the same presentation regardless of whether it ends in .00 (which would not round to two decimal places), is an int (which `round(2)` wouldn't touch at all), etc. * Round percent_total_checks_done in lttp multitracker * Fix non-LttP games showing as 0% done in LttP MultiTracker
What is this fixing or adding?
Round the percentage of total checks done instead of truncating to an int
Playing the Inscryption beta test, the world has exactly 100 checks and thanks to truncating and floats, sometimes they wouldn't match which was bothering me:
Don't return a 500 error if a multiworld has no locations
If the multiworld has no locations at all, the summary row added in WebHost: Add a summary row to the Multiworld Tracker #1965 would cause a divide-by-zero and return a 500 error. While a multiworld with 0 locations isn't really a real use case, I still don't like returning 500 errors.
How was this tested?
Ran WebHost.py locally
If this makes graphical changes, please attach screenshots.