Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Handle special cases for negative numbers, updated comments.

  • Loading branch information...
commit 2bef958ad2000571139a70dfe8a686b9e8e7d99b 1 parent af5ad22
@VanNostrand VanNostrand authored
Showing with 53 additions and 11 deletions.
  1. +36 −3 gemrb/core/RNG/RNG_SFMT.cpp
  2. +17 −8 gemrb/core/RNG/RNG_SFMT.h
View
39 gemrb/core/RNG/RNG_SFMT.cpp
@@ -44,6 +44,39 @@ RNG_SFMT* RNG_SFMT::getInstance() {
}
/**
+ * This method is the rand() equivalent which calls the cdf with proper bounds.
+ *
+ * It is possible to generate random numbers from [-min, +/-max] though the RNG uses
+ * unsigned numbers only, so this wrapper handles some special cases for min and max.
+ *
+ * It is only necessary that the upper bound is larger or equal to the lower bound - with the exception
+ * that someone wants something like rand() % -foo.
+ */
+unsigned int RNG_SFMT::rand(int min, int max) {
+ /* If min is negative, it would be possible to calculate
+ * cdf(0, max - min) + min
+ * There has been no use for negative random numbers with rand() though, so it's treated as error.
+ */
+ if(min < 0) {
+ GemRB::error("RNG", "Invalid bounds for RNG! Got min %d < 0!\n", min);
+ }
+
+ // For complete fairness and equal timing, this should be a roll, but let's skip it anyway
+ if(min == max)
+ return max;
+
+ // Someone wants rand() % -foo, so we compute -rand(0, +foo)
+ // This is the only time where min > max is (sort of) legal.
+ // Not handling this will cause the application to crash.
+ if(min == 0 && max < 0) {
+ return -cdf(0, -max);
+ }
+
+ // No special cases are left, except !(min > max) which is caught in the cdf itself.
+ return cdf(min, max);
+}
+
+/**
* Much thought went into this, please read this comment before you modify the
* code.
* Let SFMT() be an alias for sfmt_genrand_uint64() aka SFMT's rand() function.
@@ -85,12 +118,12 @@ RNG_SFMT* RNG_SFMT::getInstance() {
* defined in a RAND_MAX constant.
* Otherwise you will probably skew the outcome of the rand() method or worsen
* the performance of the application.
- *
+ *
* In threaded environments check the method's code for information on mutexes!
*/
-unsigned int RNG_SFMT::rand(unsigned int min, unsigned int max) {
+unsigned int RNG_SFMT::cdf(unsigned int min, unsigned int max) {
// This all makes no sense if min > max, which should never happen.
- if (min > max || max >= UINT_MAX || min >= UINT_MAX) {
+ if (min > max) {
GemRB::error("RNG", "Invalid bounds for RNG! Got min %d, max %d\n", min, max);
// at this point, the method exits. No return value is needed, because
// basically the exception itself is returned.
View
25 gemrb/core/RNG/RNG_SFMT.h
@@ -8,7 +8,12 @@
/**
* This class encapsulates a state of the art PRNG in a singleton class and can be used
- * to return uniformly distributed integer random numbers from a range [min, max]
+ * to return uniformly distributed integer random numbers from a range [min, max].
+ * Though technically possible, min must be >= 0 and max should always be > 0.
+ * If max < 0 and min == 0 it is assumed that rand() % -max is wanted and the result will
+ * be -rand(0, -max).
+ * This is the only exception to the rule that !(min > max) and is used as a workaround
+ * for some parts of the code where negative modulo could occur.
*
* As the class is a singleton, only one instance exists. Access it by using the
* getInstance() method, e.g. by writing
@@ -16,10 +21,6 @@
* which may be abbreviated by
* RAND(1,6);
*
- * You should never call this function with min > max, this throws a
- * std::invalid_argument exception. It is best practice to use rand() in a try block and
- * catch the exception if min, max are entered by the user or computed somehow.
- *
* Technical details:
* The RNG uses the SIMD-oriented Fast Mersenne Twister code v1.4.1 from
* http://www.math.sci.hiroshima-u.ac.jp/~%20m-mat/MT/SFMT/index.html
@@ -28,9 +29,9 @@
* These are mapped to values from the interval [min, max] without bias by using Knuth's
* "Algorithm S (Selection sampling technique)" from "The Art of Computer Programming 3rd
* Edition Volume 2 / Seminumerical Algorithms".
- *
+ *
* In a threaded environment you probably must use mutexes to make this class thread-safe.
- * There are more comments in the code about that.
+ * There are comments in the cdf-method's code about that.
*/
class RNG_SFMT {
@@ -38,11 +39,19 @@ class RNG_SFMT {
// only one instance will be allowed, use getInstance
RNG_SFMT();
+ // The discrete cumulative distribution function for the RNG
+ unsigned int cdf(unsigned int min, unsigned int max);
+
// SFMT's internal state
sfmt_t sfmt;
public:
- unsigned int rand(unsigned int min = 0, unsigned int max = UINT_MAX-1);
+ /* The RNG function to use via
+ * RNG_SFMT::getInstance()->rand(min, max);
+ * or
+ * RAND(min, max);
+ */
+ unsigned int rand(int min = 0, int max = INT_MAX-1);
static RNG_SFMT* getInstance();
};
Please sign in to comment.
Something went wrong with that request. Please try again.