Simply-Love / Simply-Love-SM5

a StepMania 5 theme for the post-ITG community
Other
204 stars 144 forks source link

QR code pane: off-by-one floating error with rate mod #573

Open silphur8 opened 1 month ago

silphur8 commented 1 month ago

Context/Issue/Replication Steps

During SRPG 8, the FE daily that occurred on the 8th of September required players to pass a song of a certain difficulty or higher with a rate mod of at least 1.15×.

If someone were to, exactly as directed, play at 1.15×, then scan the QR code to submit their score (perhaps because they are at an arcade where the cab cannot be connected to the Internet for "security reasons"), they will find that their 1.15× was in fact submitted as 1.14×, and thus not meet the requirements of the daily.

Thankfully, this was an FE daily, but it would especially be concerning if it was an SN daily involving an hour-long marathon...

Affected Lines/Investigation

https://github.com/Simply-Love/Simply-Love-SM5/blob/affce8d2406de715bdf60bd002df857bdc43f4fa/BGAnimations/ScreenEvaluation%20common/Panes/Pane7/GrooveStatsURL.lua#L40

To recap what happens here:

The issue lies in how math.floor treats these decimal-converted-to-integers, so it's not a Lua specific thing.

// Javascript
> (115/100 * 100).toFixed(20)
'114.99999999999998578915'
// and so the floor of that is...
-- Lua
for i = 100, 300 do
  local rateInted = (i / 100) * 100
  print(i
    .. ": " 
    .. string.format("%.0f", rateInted) 
    .. " / "
    .. math.floor(rateInted)
    .. " "
    .. string.format("(%.20f)", rateInted)
  )
end

I ran the above Lua snippet to determine which rate mods covered by SL-PlayerOptions were affected (i.e. those where the fractional part has a bunch of 9s) and test what their values were as reported by the QR code in-game:

Rate Float R Value Post Fix
113 112.999[...]8578 112 113
114 113.999[...]8578 113 114
115 114.999[...]8578 114 115
116 115.999[...]8578 115 116
201 200.999[...]7157 200 201
203 202.999[...]7157 202 203
205 204.999[...]7157 204 205
207 206.999[...]7157 206 207
226 225.999[...]7157 225 226
228 227.999[...]7157 227 228
230 229.999[...]7157 229 230
232 231.999[...]7157 231 232
251 250.999[...]7157 250 251
253 252.999[...]7157 252 253
255 254.999[...]7157 254 255

(0.28, 0.57 and 0.58 are also affected, but since these values place the red X over the QR for being less than a rate mod of 1.00×, I didn't have to check them.)

As expected, the R value for all of the rates affected are reflected in the QR code.

Proposed Solution

With the absence of a round function, whilst it does feel "hack"-y, apparently string.format can deal with these floating point moments. So, when combined with converting the string back to a number for good measure, perhaps we could consider using

local rate = tonumber(string.format("%.0f", SL.Global.ActiveModifiers.MusicRate * 100))

I can confirm this fixes the affected rate mods in-game, as noted in the "Post Fix" column of the table above. I haven't tested every other rate mod, though I think it's safe for us to assume we can trust the output of the code snippet.

teejusb commented 1 month ago

Using string.format to adjust rounding issues is actually totally fine and is used in various parts of the code base already. Thanks for all the tests and description! This is great!

silphur8 commented 1 month ago

Nota bene: