diff --git a/common/rng_abstract.cpp b/common/rng_abstract.cpp index 2919d437..4c51143a 100644 --- a/common/rng_abstract.cpp +++ b/common/rng_abstract.cpp @@ -7,9 +7,9 @@ QVector RNG_Abstract::makeNumbersVector(int n, int min, int max) const int bins = max - min + 1; QVector result(bins); for (int i = 0; i < n; ++i) { - int number = getNumber(min, max); + int number = rand(min, max); if ((number < min) || (number > max)) - qDebug() << "getNumber(" << min << "," << max << ") returned " << number; + qDebug() << "rand(" << min << "," << max << ") returned " << number; else result[number - min]++; } diff --git a/common/rng_abstract.h b/common/rng_abstract.h index 5d60033f..f2017819 100644 --- a/common/rng_abstract.h +++ b/common/rng_abstract.h @@ -8,7 +8,7 @@ class RNG_Abstract : public QObject { Q_OBJECT public: RNG_Abstract(QObject *parent = 0) : QObject(parent) { } - virtual unsigned int getNumber(unsigned int min, unsigned int max) = 0; + virtual unsigned int rand(int min, int max) = 0; QVector makeNumbersVector(int n, int min, int max); double testRandom(const QVector &numbers) const; }; diff --git a/common/rng_sfmt.cpp b/common/rng_sfmt.cpp index ce734c4d..d3d55d1c 100644 --- a/common/rng_sfmt.cpp +++ b/common/rng_sfmt.cpp @@ -17,6 +17,44 @@ RNG_SFMT::RNG_SFMT(QObject *parent) sfmt_init_gen_rand(&sfmt, QDateTime::currentDateTime().toTime_t()); } +/** + * 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) { + throw std::invalid_argument( + QString("Invalid bounds for RNG: Got min " + + QString::number(min) + " < 0!\n").toStdString()); + // at this point, the method exits. No return value is needed, because + // basically the exception itself is returned. + } + + // For complete fairness and equal timing, this should be a roll, but let's skip it anyway + if(min == max) + return max; + + // This is actually not used in Cockatrice: + // 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. @@ -33,7 +71,7 @@ RNG_SFMT::RNG_SFMT(QObject *parent) * * To get out the random variable, solve for X: * floor(X) = SFMT(X; min, max) * (max - min + 1) + min - 1 - * So this is, what getNumber(min, max) should look like. + * So this is, what rand(min, max) should look like. * Problem: SFMT(X; min, max) * (max - min + 1) could produce an integer overflow, * so it is not safe. * @@ -56,19 +94,17 @@ RNG_SFMT::RNG_SFMT(QObject *parent) * then you _need_ to change the UINT64_MAX constant to the largest possible random * number which can be created by the new rand() function. This value is often defined * in a RAND_MAX constant. - * Otherwise you will probably skew the outcome of the getNumber() method or worsen the + * Otherwise you will probably skew the outcome of the rand() method or worsen the * performance of the application. */ -unsigned int RNG_SFMT::getNumber(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) { throw std::invalid_argument( QString("Invalid bounds for RNG: min > max! Values were: min = " + QString::number(min) + ", max = " + - QString::number(max) + - ". This is either a bug or something even more serious happened." - ).toStdString()); + QString::number(max)).toStdString()); // at this point, the method exits. No return value is needed, because // basically the exception itself is returned. } diff --git a/common/rng_sfmt.h b/common/rng_sfmt.h index 3134e91d..d35ce97d 100644 --- a/common/rng_sfmt.h +++ b/common/rng_sfmt.h @@ -1,27 +1,28 @@ #ifndef RNG_SFMT_H #define RNG_SFMT_H +#include +#include #include "sfmt/SFMT.h" #include "rng_abstract.h" -#include /** - * This class represents the random number generator. - * Usage instructions: - * Create an instance of RNG_SFMT, then call getNumber(min, max). - * You should never call this function with min > max, this throws a - * std::invalid_argument exception. It is best practice to use getNumber() in a try block - * and catch the exception if min, max are entered by the user or computed somehow. + * This class encapsulates a state of the art PRNG and can be used + * 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 actually unused in + * Cockatrice. * * 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 - * To use this RNG, the class needs a sfmt_t structure for the RNG's internal state. - * It has to be initialized by sfmt_init_gen_rand() which is done in the constructor. - * The function sfmt_genrand_uint64() can then be used to create a 64 bit unsigned int - * pseudorandom number. This is done in getNumber(). - * For more information see the author's website and look at the documentation and - * examples that are part of the official downloads. + * The SFMT RNG creates unsigned int 64bit pseudo random numbers. + * + * 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". */ class RNG_SFMT : public RNG_Abstract { @@ -29,9 +30,11 @@ class RNG_SFMT : public RNG_Abstract { private: QMutex mutex; sfmt_t sfmt; + // The discrete cumulative distribution function for the RNG + unsigned int cdf(unsigned int min, unsigned int max); public: RNG_SFMT(QObject *parent = 0); - unsigned int getNumber(unsigned int min, unsigned int max); + unsigned int rand(int min, int max); }; #endif diff --git a/common/server_cardzone.cpp b/common/server_cardzone.cpp index 018abae8..4a9345b3 100644 --- a/common/server_cardzone.cpp +++ b/common/server_cardzone.cpp @@ -45,7 +45,7 @@ void Server_CardZone::shuffle() { QList temp; for (int i = cards.size(); i; i--) - temp.append(cards.takeAt(rng->getNumber(0, i - 1))); + temp.append(cards.takeAt(rng->rand(0, i - 1))); cards = temp; playersWithWritePermission.clear(); diff --git a/common/server_player.cpp b/common/server_player.cpp index 489c3dcb..a029abed 100644 --- a/common/server_player.cpp +++ b/common/server_player.cpp @@ -844,7 +844,7 @@ Response::ResponseCode Server_Player::cmdRollDie(const Command_RollDie &cmd, Res Event_RollDie event; event.set_sides(cmd.sides()); - event.set_value(rng->getNumber(1, cmd.sides())); + event.set_value(rng->rand(1, cmd.sides())); ges.enqueueGameEvent(event, playerId); return Response::RespOk; @@ -1524,7 +1524,7 @@ Response::ResponseCode Server_Player::cmdRevealCards(const Command_RevealCards & else if (cmd.card_id() == -2) { if (zone->getCards().isEmpty()) return Response::RespContextError; - cardsToReveal.append(zone->getCards().at(rng->getNumber(0, zone->getCards().size() - 1))); + cardsToReveal.append(zone->getCards().at(rng->rand(0, zone->getCards().size() - 1))); } else { Server_Card *card = zone->getCard(cmd.card_id()); if (!card)