From 60a51e98de58b1764e1dc5853cd72afe5acc500a Mon Sep 17 00:00:00 2001 From: Ivan Savenko Date: Sat, 1 Jun 2024 13:36:38 +0000 Subject: [PATCH] Remove usage of std::function from CRandomGenerator --- AI/BattleAI/StackWithBonuses.h | 19 ++++------ include/vstd/RNG.h | 18 ++++----- lib/CRandomGenerator.cpp | 44 +++++++++------------- lib/CRandomGenerator.h | 24 +++--------- lib/CStack.cpp | 4 +- lib/battle/BattleInfo.cpp | 3 +- lib/json/JsonRandom.cpp | 4 +- lib/rewardable/Info.cpp | 2 +- lib/rmg/modificators/ConnectionsPlacer.cpp | 2 +- lib/spells/AdventureSpellMechanics.cpp | 4 +- lib/spells/BattleSpellMechanics.cpp | 4 +- lib/spells/ISpellMechanics.cpp | 4 +- lib/spells/effects/Catapult.cpp | 4 +- server/battles/BattleActionProcessor.cpp | 4 +- test/mock/mock_vstd_RNG.h | 5 ++- test/spells/effects/EffectFixture.cpp | 18 +++------ 16 files changed, 61 insertions(+), 102 deletions(-) diff --git a/AI/BattleAI/StackWithBonuses.h b/AI/BattleAI/StackWithBonuses.h index 6a50f6f84..bd960f295 100644 --- a/AI/BattleAI/StackWithBonuses.h +++ b/AI/BattleAI/StackWithBonuses.h @@ -24,20 +24,17 @@ class HypotheticBattle; class RNGStub : public vstd::RNG { public: - vstd::TRandI64 getInt64Range(int64_t lower, int64_t upper) override + int nextInt(int lower, int upper) override { - return [=]()->int64_t - { - return (lower + upper)/2; - }; + return (lower + upper) / 2; } - - vstd::TRand getDoubleRange(double lower, double upper) override + int64_t nextInt64(int64_t lower, int64_t upper) override { - return [=]()->double - { - return (lower + upper)/2; - }; + return (lower + upper) / 2; + } + double nextDouble(double lower, double upper) override + { + return (lower + upper) / 2; } }; diff --git a/include/vstd/RNG.h b/include/vstd/RNG.h index 60e1dedc0..a230c2773 100644 --- a/include/vstd/RNG.h +++ b/include/vstd/RNG.h @@ -15,18 +15,14 @@ VCMI_LIB_NAMESPACE_BEGIN namespace vstd { -using TRandI64 = std::function; -using TRand = std::function; - class DLL_LINKAGE RNG { public: - virtual ~RNG() = default; - virtual TRandI64 getInt64Range(int64_t lower, int64_t upper) = 0; - - virtual TRand getDoubleRange(double lower, double upper) = 0; + virtual int nextInt(int lower, int upper) = 0; + virtual int64_t nextInt64(int64_t lower, int64_t upper) = 0; + virtual double nextDouble(double lower, double upper) = 0; }; } @@ -39,7 +35,7 @@ namespace RandomGeneratorUtil if(container.empty()) throw std::runtime_error("Unable to select random item from empty container!"); - return std::next(container.begin(), rand.getInt64Range(0, container.size() - 1)()); + return std::next(container.begin(), rand.nextInt64(0, container.size() - 1)); } template @@ -48,7 +44,7 @@ namespace RandomGeneratorUtil if(container.empty()) throw std::runtime_error("Unable to select random item from empty container!"); - return std::next(container.begin(), rand.getInt64Range(0, container.size() - 1)()); + return std::next(container.begin(), rand.nextInt64(0, container.size() - 1)); } template @@ -59,7 +55,7 @@ namespace RandomGeneratorUtil int64_t totalWeight = std::accumulate(container.begin(), container.end(), 0); assert(totalWeight > 0); - int64_t roll = rand.getInt64Range(0, totalWeight - 1)(); + int64_t roll = rand.nextInt64(0, totalWeight - 1); for (size_t i = 0; i < container.size(); ++i) { @@ -77,7 +73,7 @@ namespace RandomGeneratorUtil for(int64_t i = n-1; i>0; --i) { - std::swap(container.begin()[i],container.begin()[rand.getInt64Range(0, i)()]); + std::swap(container.begin()[i],container.begin()[rand.nextInt64(0, i)]); } } } diff --git a/lib/CRandomGenerator.cpp b/lib/CRandomGenerator.cpp index 22ff8c78e..4bb17dd27 100644 --- a/lib/CRandomGenerator.cpp +++ b/lib/CRandomGenerator.cpp @@ -35,28 +35,17 @@ void CRandomGenerator::resetSeed() setSeed(static_cast(threadIdHash * std::time(nullptr))); } -TRandI CRandomGenerator::getIntRange(int lower, int upper) -{ - if (lower <= upper) - return std::bind(TIntDist(lower, upper), std::ref(rand)); - throw std::runtime_error("Invalid range provided: " + std::to_string(lower) + " ... " + std::to_string(upper)); -} - -vstd::TRandI64 CRandomGenerator::getInt64Range(int64_t lower, int64_t upper) -{ - if(lower <= upper) - return std::bind(TInt64Dist(lower, upper), std::ref(rand)); - throw std::runtime_error("Invalid range provided: " + std::to_string(lower) + " ... " + std::to_string(upper)); -} - int CRandomGenerator::nextInt(int upper) { - return getIntRange(0, upper)(); + return nextInt(0, upper); } int CRandomGenerator::nextInt(int lower, int upper) { - return getIntRange(lower, upper)(); + if (lower > upper) + throw std::runtime_error("Invalid range provided: " + std::to_string(lower) + " ... " + std::to_string(upper)); + + return TIntDist(lower, upper)(rand); } int CRandomGenerator::nextInt() @@ -64,28 +53,31 @@ int CRandomGenerator::nextInt() return TIntDist()(rand); } -vstd::TRand CRandomGenerator::getDoubleRange(double lower, double upper) +int64_t CRandomGenerator::nextInt64(int64_t lower, int64_t upper) { - if(lower <= upper) - return std::bind(TRealDist(lower, upper), std::ref(rand)); - throw std::runtime_error("Invalid range provided: " + std::to_string(lower) + " ... " + std::to_string(upper)); + if (lower > upper) + throw std::runtime_error("Invalid range provided: " + std::to_string(lower) + " ... " + std::to_string(upper)); + return TInt64Dist(lower, upper)(rand); } double CRandomGenerator::nextDouble(double upper) { - return getDoubleRange(0, upper)(); + return nextDouble(0, upper); } double CRandomGenerator::nextDouble(double lower, double upper) { - return getDoubleRange(lower, upper)(); + if(lower > upper) + throw std::runtime_error("Invalid range provided: " + std::to_string(lower) + " ... " + std::to_string(upper)); + + return TRealDist(lower, upper)(rand); } -double CRandomGenerator::nextDouble() -{ - return TRealDist()(rand); -} +//double CRandomGenerator::nextDouble() +//{ +// return TRealDist()(rand); +//} CRandomGenerator & CRandomGenerator::getDefault() { diff --git a/lib/CRandomGenerator.h b/lib/CRandomGenerator.h index fe0dd2070..434c63180 100644 --- a/lib/CRandomGenerator.h +++ b/lib/CRandomGenerator.h @@ -15,6 +15,7 @@ VCMI_LIB_NAMESPACE_BEGIN + /// Generator to use for all randomization in game /// minstd_rand is selected due to following reasons: /// 1. Its randomization quality is below mt_19937 however this is unlikely to be noticeable in game @@ -23,12 +24,11 @@ using TGenerator = std::minstd_rand; using TIntDist = std::uniform_int_distribution; using TInt64Dist = std::uniform_int_distribution; using TRealDist = std::uniform_real_distribution; -using TRandI = std::function; /// The random generator randomly generates integers and real numbers("doubles") between /// a given range. This is a header only class and mainly a wrapper for /// convenient usage of the standard random API. An instance of this RNG is not thread safe. -class DLL_LINKAGE CRandomGenerator : public vstd::RNG, boost::noncopyable, public Serializeable +class DLL_LINKAGE CRandomGenerator final : public vstd::RNG, boost::noncopyable, public Serializeable { public: /// Seeds the generator by default with the product of the current time in milliseconds and the @@ -44,37 +44,23 @@ public: /// current thread ID. void resetSeed(); - /// Generate several integer numbers within the same range. - /// e.g.: auto a = gen.getIntRange(0,10); a(); a(); a(); - /// requires: lower <= upper - TRandI getIntRange(int lower, int upper); - - vstd::TRandI64 getInt64Range(int64_t lower, int64_t upper) override; - /// Generates an integer between 0 and upper. /// requires: 0 <= upper int nextInt(int upper); /// requires: lower <= upper - int nextInt(int lower, int upper); + int nextInt(int lower, int upper) override; + int64_t nextInt64(int64_t lower, int64_t upper) override; /// Generates an integer between 0 and the maximum value it can hold. int nextInt(); - /// Generate several double/real numbers within the same range. - /// e.g.: auto a = gen.getDoubleRange(4.5,10.2); a(); a(); a(); - /// requires: lower <= upper - vstd::TRand getDoubleRange(double lower, double upper) override; - /// Generates a double between 0 and upper. /// requires: 0 <= upper double nextDouble(double upper); /// requires: lower <= upper - double nextDouble(double lower, double upper); - - /// Generates a double between 0.0 and 1.0. - double nextDouble(); + double nextDouble(double lower, double upper) override; /// Gets a globally accessible RNG which will be constructed once per thread. For the /// seed a combination of the thread ID and current time in milliseconds will be used. diff --git a/lib/CStack.cpp b/lib/CStack.cpp index a84f8cd0f..21747bfd2 100644 --- a/lib/CStack.cpp +++ b/lib/CStack.cpp @@ -212,11 +212,9 @@ void CStack::prepareAttacked(BattleStackAttacked & bsa, vstd::RNG & rand, const auto resurrectedAdd = static_cast(baseAmount - (resurrectedCount / resurrectFactor)); - auto rangeGen = rand.getInt64Range(0, 99); - for(int32_t i = 0; i < resurrectedAdd; i++) { - if(resurrectValue > rangeGen()) + if(resurrectValue > rand.nextInt(0, 99)) resurrectedCount += 1; } diff --git a/lib/battle/BattleInfo.cpp b/lib/battle/BattleInfo.cpp index 946499a4a..86dcb4f3e 100644 --- a/lib/battle/BattleInfo.cpp +++ b/lib/battle/BattleInfo.cpp @@ -662,10 +662,9 @@ int64_t BattleInfo::getActualDamage(const DamageRange & damage, int32_t attacker int64_t sum = 0; auto howManyToAv = std::min(10, attackerCount); - auto rangeGen = rng.getInt64Range(damage.min, damage.max); for(int32_t g = 0; g < howManyToAv; ++g) - sum += rangeGen(); + sum += rng.nextInt64(damage.min, damage.max); return sum / howManyToAv; } diff --git a/lib/json/JsonRandom.cpp b/lib/json/JsonRandom.cpp index df71283b2..3fbda28ef 100644 --- a/lib/json/JsonRandom.cpp +++ b/lib/json/JsonRandom.cpp @@ -63,7 +63,7 @@ VCMI_LIB_NAMESPACE_BEGIN { const auto & vector = value.Vector(); - size_t index= rng.getIntRange(0, vector.size()-1)(); + size_t index= rng.nextInt64(0, vector.size()-1); return loadValue(vector[index], rng, variables, 0); } if(value.isStruct()) @@ -72,7 +72,7 @@ VCMI_LIB_NAMESPACE_BEGIN return loadValue(value["amount"], rng, variables, defaultValue); si32 min = loadValue(value["min"], rng, variables, 0); si32 max = loadValue(value["max"], rng, variables, 0); - return rng.getIntRange(min, max)(); + return rng.nextInt64(min, max); } return defaultValue; } diff --git a/lib/rewardable/Info.cpp b/lib/rewardable/Info.cpp index 14b73fa6c..00323daf9 100644 --- a/lib/rewardable/Info.cpp +++ b/lib/rewardable/Info.cpp @@ -298,7 +298,7 @@ void Rewardable::Info::configureRewards( { const JsonNode & preset = object.getPresetVariable("dice", diceID); if (preset.isNull()) - object.initVariable("dice", diceID, rng.getIntRange(0, 99)()); + object.initVariable("dice", diceID, rng.nextInt(0, 99)); else object.initVariable("dice", diceID, preset.Integer()); diff --git a/lib/rmg/modificators/ConnectionsPlacer.cpp b/lib/rmg/modificators/ConnectionsPlacer.cpp index e6692e260..f9a34c25c 100644 --- a/lib/rmg/modificators/ConnectionsPlacer.cpp +++ b/lib/rmg/modificators/ConnectionsPlacer.cpp @@ -444,7 +444,7 @@ void ConnectionsPlacer::collectNeighbourZones() bool ConnectionsPlacer::shouldGenerateRoad(const rmg::ZoneConnection& connection) const { return connection.getRoadOption() == rmg::ERoadOption::ROAD_TRUE || - (connection.getRoadOption() == rmg::ERoadOption::ROAD_RANDOM && zone.getRand().nextDouble() >= 0.5f); + (connection.getRoadOption() == rmg::ERoadOption::ROAD_RANDOM && zone.getRand().nextDouble(0, 1) >= 0.5f); } void ConnectionsPlacer::createBorder() diff --git a/lib/spells/AdventureSpellMechanics.cpp b/lib/spells/AdventureSpellMechanics.cpp index db3e97ab1..6d7699ee4 100644 --- a/lib/spells/AdventureSpellMechanics.cpp +++ b/lib/spells/AdventureSpellMechanics.cpp @@ -187,7 +187,7 @@ ESpellCastResult SummonBoatMechanics::applyAdventureEffects(SpellCastEnvironment const auto schoolLevel = parameters.caster->getSpellSchoolLevel(owner); //check if spell works at all - if(env->getRNG()->getInt64Range(0, 99)() >= owner->getLevelPower(schoolLevel)) //power is % chance of success + if(env->getRNG()->nextInt(0, 99) >= owner->getLevelPower(schoolLevel)) //power is % chance of success { InfoWindow iw; iw.player = parameters.caster->getCasterOwner(); @@ -280,7 +280,7 @@ ESpellCastResult ScuttleBoatMechanics::applyAdventureEffects(SpellCastEnvironmen { const auto schoolLevel = parameters.caster->getSpellSchoolLevel(owner); //check if spell works at all - if(env->getRNG()->getInt64Range(0, 99)() >= owner->getLevelPower(schoolLevel)) //power is % chance of success + if(env->getRNG()->nextInt(0, 99) >= owner->getLevelPower(schoolLevel)) //power is % chance of success { InfoWindow iw; iw.player = parameters.caster->getCasterOwner(); diff --git a/lib/spells/BattleSpellMechanics.cpp b/lib/spells/BattleSpellMechanics.cpp index 8ad548bfa..d586bc785 100644 --- a/lib/spells/BattleSpellMechanics.cpp +++ b/lib/spells/BattleSpellMechanics.cpp @@ -377,15 +377,13 @@ void BattleSpellMechanics::beforeCast(BattleSpellCast & sc, vstd::RNG & rng, con std::vector resisted; - auto rangeGen = rng.getInt64Range(0, 99); - auto filterResisted = [&, this](const battle::Unit * unit) -> bool { if(isNegativeSpell() && isMagicalEffect()) { //magic resistance const int prob = std::min(unit->magicResistance(), 100); //probability of resistance in % - if(rangeGen() < prob) + if(rng.nextInt(0, 99) < prob) return true; } return false; diff --git a/lib/spells/ISpellMechanics.cpp b/lib/spells/ISpellMechanics.cpp index 6e260347f..3cec1dd72 100644 --- a/lib/spells/ISpellMechanics.cpp +++ b/lib/spells/ISpellMechanics.cpp @@ -268,11 +268,9 @@ void BattleCast::cast(ServerCallback * server, Target target) const std::string magicMirrorCacheStr = "type_MAGIC_MIRROR"; static const auto magicMirrorSelector = Selector::type()(BonusType::MAGIC_MIRROR); - auto rangeGen = server->getRNG()->getInt64Range(0, 99); - const int mirrorChance = mainTarget->valOfBonuses(magicMirrorSelector, magicMirrorCacheStr); - if(rangeGen() < mirrorChance) + if(server->getRNG()->nextInt(0, 99) < mirrorChance) { auto mirrorTargets = cb->battleGetUnitsIf([this](const battle::Unit * unit) { diff --git a/lib/spells/effects/Catapult.cpp b/lib/spells/effects/Catapult.cpp index 167aaabd6..2a511935d 100644 --- a/lib/spells/effects/Catapult.cpp +++ b/lib/spells/effects/Catapult.cpp @@ -118,7 +118,7 @@ void Catapult::applyTargeted(ServerCallback * server, const Mechanics * m, const auto actualTarget = EWallPart::INVALID; if ( m->battle()->isWallPartAttackable(desiredTarget) && - server->getRNG()->getInt64Range(0, 99)() < getCatapultHitChance(desiredTarget)) + server->getRNG()->nextInt(0, 99) < getCatapultHitChance(desiredTarget)) { actualTarget = desiredTarget; } @@ -172,7 +172,7 @@ int Catapult::getRandomDamage (ServerCallback * server) const { std::array damageChances = { noDmg, hit, crit }; //dmgChance[i] - chance for doing i dmg when hit is successful int totalChance = std::accumulate(damageChances.begin(), damageChances.end(), 0); - int damageRandom = server->getRNG()->getInt64Range(0, totalChance - 1)(); + int damageRandom = server->getRNG()->nextInt(0, totalChance - 1); int dealtDamage = 0; //calculating dealt damage diff --git a/server/battles/BattleActionProcessor.cpp b/server/battles/BattleActionProcessor.cpp index 61e13c26a..0fb275960 100644 --- a/server/battles/BattleActionProcessor.cpp +++ b/server/battles/BattleActionProcessor.cpp @@ -1341,7 +1341,7 @@ void BattleActionProcessor::handleAfterAttackCasting(const CBattleInfoCallback & double chanceToTrigger = attacker->valOfBonuses(BonusType::TRANSMUTATION) / 100.0f; vstd::amin(chanceToTrigger, 1); //cap at 100% - if(gameHandler->getRandomGenerator().getDoubleRange(0, 1)() > chanceToTrigger) + if(gameHandler->getRandomGenerator().nextDouble(0, 1) > chanceToTrigger) return; int bonusAdditionalInfo = attacker->getBonus(Selector::type()(BonusType::TRANSMUTATION))->additionalInfo[0]; @@ -1405,7 +1405,7 @@ void BattleActionProcessor::handleAfterAttackCasting(const CBattleInfoCallback & vstd::amin(chanceToTrigger, 1); //cap trigger chance at 100% - if(gameHandler->getRandomGenerator().getDoubleRange(0, 1)() > chanceToTrigger) + if(gameHandler->getRandomGenerator().nextDouble(0, 1) > chanceToTrigger) return; BattleStackAttacked bsa; diff --git a/test/mock/mock_vstd_RNG.h b/test/mock/mock_vstd_RNG.h index 2fe2b04e1..0064503ad 100644 --- a/test/mock/mock_vstd_RNG.h +++ b/test/mock/mock_vstd_RNG.h @@ -18,8 +18,9 @@ namespace vstd class RNGMock : public RNG { public: - MOCK_METHOD2(getInt64Range, TRandI64(int64_t, int64_t)); - MOCK_METHOD2(getDoubleRange, TRand(double, double)); + MOCK_METHOD2(nextInt, int(int lower, int upper)); + MOCK_METHOD2(nextInt64, int64_t(int64_t lower, int64_t upper)); + MOCK_METHOD2(nextDouble, double(double lower, double upper)); }; } diff --git a/test/spells/effects/EffectFixture.cpp b/test/spells/effects/EffectFixture.cpp index 5cf558a44..cf4478786 100644 --- a/test/spells/effects/EffectFixture.cpp +++ b/test/spells/effects/EffectFixture.cpp @@ -101,27 +101,21 @@ void EffectFixture::setUp() ON_CALL(serverMock, apply(Matcher(_))).WillByDefault(Invoke(battleFake.get(), &battle::BattleFake::accept)); } -static vstd::TRandI64 getInt64RangeDef(int64_t lower, int64_t upper) +static int64_t getInt64Range(int64_t lower, int64_t upper) { - return [=]()->int64_t - { - return (lower + upper)/2; - }; + return (lower + upper)/2; } -static vstd::TRand getDoubleRangeDef(double lower, double upper) +static double getDoubleRange(double lower, double upper) { - return [=]()->double - { - return (lower + upper)/2; - }; + return (lower + upper)/2; } void EffectFixture::setupDefaultRNG() { EXPECT_CALL(serverMock, getRNG()).Times(AtLeast(0)); - EXPECT_CALL(rngMock, getInt64Range(_,_)).WillRepeatedly(Invoke(&getInt64RangeDef)); - EXPECT_CALL(rngMock, getDoubleRange(_,_)).WillRepeatedly(Invoke(&getDoubleRangeDef)); + EXPECT_CALL(rngMock, nextInt64(_,_)).WillRepeatedly(Invoke(&getInt64Range)); + EXPECT_CALL(rngMock, nextDouble(_,_)).WillRepeatedly(Invoke(&getDoubleRange)); } }