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

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

Open
silphur8 opened this issue Oct 8, 2024 · 2 comments
Open

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

silphur8 opened this issue Oct 8, 2024 · 2 comments

Comments

@silphur8
Copy link

silphur8 commented Oct 8, 2024

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

local rate = math.floor(SL.Global.ActiveModifiers.MusicRate * 100)

To recap what happens here:

  • The rate mod is stored as a decimal value (1.15, etc) as determined in Scripts/SL-PlayerOptions.lua.
  • This is then multiplied by 100 and floored to produce an integer value...
  • ...For the purposes of being able to be minified when encoded into the QR's URL (amongst other things like judgements).

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
Copy link
Collaborator

teejusb commented Oct 8, 2024

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
Copy link
Author

silphur8 commented Oct 8, 2024

Nota bene:

  • It's pretty minor given (thankfully?) it only affects the 1.13 – 1.16× range for rates < 2.00×. Perhaps the 1.01× gang do have a point 😅
  • On that note, just curious/checking in, is the R value handled properly for values > 2.56× on GS's end?

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

No branches or pull requests

2 participants