Repository navigation
Fix random.randint() with negative numbers - #215
Conversation
randint(a, b) used int(random()*(b-a+1) + a). int() truncates toward zero, so with a negative lower bound randint almost never returns a and returns 0 twice as often as the other values. With both bounds negative it goes out of range: randint(-5, -1) can return 0. Use randbelow(), the same way randrange() already does. Same one-line change as the randrange patch in 2debbe4, applied to the 3.2 files and the build notes. Upstream report: kovidgoyal/rapydscript-ng#185
|
Thanks, @greatsong. That's a clear write-up, and the before/after counts make the bug obvious. We're working on this: next we'll review the patch, check that randint() is right across negative, mixed-sign and large ranges in a local build, and then merge and redeploy glowscript.org. 🤖 Generated with Claude Code |
sspickle
left a comment
sspickle
left a comment
There was a problem hiding this comment.
Verified, thanks. Each of the four files changes only the randint body. I ran randint through the GlowScriptOffline compile-and-run pipeline with the master build and with this PR's RScompiler.3.2.min.js:
| master | this PR | |
|---|---|---|
randint(-2, 2) |
never −2, 0 twice as often | −2..2, even |
randint(-5, -1) |
never −5, returns 0 | −5..−1, even |
randint(-5, 5) |
never −5, 0 doubled | even |
randint(3, 3) |
3 | 3 |
randint(0, 10**10) |
8,636 of 20,000 out of range | 0 out of range |
Upstream's follow-up (165020b) also makes randint raise TypeError and ValueError for non-integers and empty ranges. Keeping this to the one line is the right call for now, since raising could break existing student programs. We'll look at that change separately, together with a RapydScript refresh.
🤖 Generated with Claude Code
On glowscript.org, randint(-2, 2) never returns -2 and returns 0 about twice as often as the other values. With both bounds negative it goes out of range: randint(-5, -1) can return 0. This is easy to hit in class, for example when picking a random velocity between -5 and 5.
The cause is in the RapydScript random module. randint() uses int(), which truncates toward zero. Bruce reported it upstream in November 2024 (kovidgoyal/rapydscript-ng#185), and the fix there is kovidgoyal/rapydscript-ng#193. GlowScript ships its own copy of the compiler, so it needs the same fix here.
This is the same kind of patch as 2debbe4 for randrange(). The one line changes to
return a + randbelow(b - a + 1)in:Older package versions are left as they are.
I compiled test programs with the current and patched RScompiler.3.2.min.js and called each function 100,000 times.
Before:
After:
@BruceSherwood @sspickle glowscript.org will need a redeploy to pick this up.