Which uses of the PRNG in particular? Was this issue 3.2 specific? Perhaps Windows specific or something?
Some of my players used to be suspicious about the generator in an earlier version of Vassal and I did some testing. However, repeatedly rolling large numbers of dice (randomized layers) and checking reports proved that the distribution of results was ok. Or so it seemed. Since my private fork is still based on Vassal 3.1, I’m curious which uses of the PRNG actually produce this problem.
Thus spake Filip:
“VASSAL 3.2.15 released” wrote:
- 11404: Some uses of the PRNG introduce a minuscule amount of modulo
biasWhich uses of the PRNG in particular?
Any roll of a die having a number of faces which is not a power of two.
Was this issue 3.2 specific? Perhaps Windows specific or something?
No, the issue existed in all versions of VASSAL to that point, and
was not platform-specific.
Some of my players used to be suspicious about the generator in an
earlier version of Vassal and I did some testing. However, repeatedly
rolling large numbers of dice (randomized layers) and checking reports
proved that the distribution of results was ok. Or so it seemed. Since
my private fork is still based on Vassal 3.1, I’m curious which uses of
the PRNG actually produce this problem.
To give you a concrete example, if you rolled about 715 million six-
sided dice, the bug we corrected would give you on average one fewer
each of 5 and 6 than you should expect. It’s exceedingly unlikely that
any single individual has rolled enough dice with VASSAL that this
would have affected them.
I fixed this bug because it annoyed me that we were doing something
incorrect, not because it could have caused anyone a problem.
–
J.
Ah, that’s what was meant by “miniscule”.
I’ve been comparing some code today and I notice that while DiceButton changed a line of code from (ran.nextFloat() * nSides + 1) to ran.nextInt(nSides) + 1, a few other classes (RandomTextButton and DieServer) still use (ran.nextFloat() * nSides + 1). Are those two other classes fine as they are, or was it an omission?
Also, I notice deck shuffling code is different in 3.2 than in 3.1.20. What was this change about? Something related?
Thus spake Filip:
I’ve been comparing some code today and I notice that while DiceButton
changed a line of code from (ran.nextFloat() * nSides + 1) to
ran.nextInt(nSides) + 1, a few other classes (RandomTextButtin and
DieServer) still use (ran.nextFloat() * nSides + 1). Are those two other
classes fine as they are, or was it an omission?
That was an oversight. Those should have been changed along with
DiceButton. I’ve fixed this in trunk@9235 and it will go out in 3.2.17.
(I’m aiming for sometime in the next week to ten days.)
Thanks for spotting this.
Also, I notice deck shuffling code is different in 3.2 than in 3.1.20.
What was this change about?
The shuffle used prior to 3.2.14 is not obviously correct. This is not to
say that it is incorrect—just that once I noticed it, it would have
been a waste of time to prove it correct rather than simply swapping it
out for a Fisher-Yates shuffle, which is easy to show is correct and is
only a few lines of code.
–
J.
I notice you updated this, but when I checked the trunk here and here to make sure I applied the changes correctly to my 3.1 “fork”, it seems like the old nextFloat code is still there. Am I missing something (I’m not really familiar with how this svn thing works)?
Those links are not to the trunk, they’re on a branch called uckelman-dice. Try this for the trunk version (adjust similarly for the other file): svn.code.sf.net/p/vassalengine/s … utton.java
Thanks.
Thus spake Filip:
I notice you updated this,[1] but when I checked the trunk here[2] and
here[3] to make sure I applied the changes correctly to my 3.1 “fork”,
Why are you maintaining a fork of 3.1? We have fixed litterally hundreds
of bugs since the last 3.1 release.
–
J.
Well, I’ve been explaining this to you about once a year on this forum.
TL;DR, I don’t like certain features of 3.2 and I’ve already fixed a number of issues my group had with 3.1.20. Also, the fork is tailored specifically to the module we use regularly (by which I mean 2-3 games each week since 2008). At this point it’s more practical for us to keep working with 3.1 rather than try to apply those changes to 3.2.
Thus spake Filip:
[This message has been edited.]
Well, I’ve been explaining this to you about once a year on this forum.
I can’t possibly keep track of everything everyone tells me.
TL;DR, I don’t like certain features of 3.2 and I’ve already fixed a
number of issues my group had with 3.1.20. Also, the fork is tailored
specifically to the module we use regularly (by which I mean 2-3 games
each week since 2008). At this point it’s more practical for us to keep
working with 3.1 rather than try to apply those changes to 3.2.
Ok. Sounds like you’re doing fine, then. Thanks for the explanation.
–
J.