From 08acd4aac05158e09655a54ee60fd5e15014993a Mon Sep 17 00:00:00 2001 From: ephphatha Date: Mon, 25 Jul 2022 18:00:45 +1000 Subject: [PATCH] Tidy up T_FitObj5 --- Source/levels/themes.cpp | 85 ++++++++++++++++------------------------ test/random_test.cpp | 50 +++++++++++++++++++++++ 2 files changed, 83 insertions(+), 52 deletions(-) diff --git a/Source/levels/themes.cpp b/Source/levels/themes.cpp index 9e8c32b54..de44bafd3 100644 --- a/Source/levels/themes.cpp +++ b/Source/levels/themes.cpp @@ -36,22 +36,6 @@ bool bFountainFlag; /** Specifies the set of special theme IDs from which one will be selected at random. */ theme_id ThemeGood[4] = { THEME_GOATSHRINE, THEME_SHRINE, THEME_SKELROOM, THEME_LIBRARY }; -/** Specifies a 5x5 area to fit theme objects. */ -int trm5x[] = { - -2, -1, 0, 1, 2, - -2, -1, 0, 1, 2, - -2, -1, 0, 1, 2, - -2, -1, 0, 1, 2, - -2, -1, 0, 1, 2 -}; -/** Specifies a 5x5 area to fit theme objects. */ -int trm5y[] = { - -2, -2, -2, -2, -2, - -1, -1, -1, -1, -1, - 0, 0, 0, 0, 0, - 1, 1, 1, 1, 1, - 2, 2, 2, 2, 2 -}; bool TFit_Shrine(int i) { int xp = 0; @@ -97,49 +81,46 @@ bool TFit_Shrine(int i) return true; } -bool TFit_Obj5(int t) +bool CheckThemeObj5(Point origin, int8_t regionId) { - int xp = 0; - int yp = 0; - int r = GenerateRnd(5) + 1; - int rs = r; - - while (r > 0) { - bool found = false; - if (dTransVal[xp][yp] == themes[t].ttval && IsTileNotSolid({ xp, yp })) { - found = true; - for (int i = 0; found && i < 25; i++) { - if (TileHasAny(dPiece[xp + trm5x[i]][yp + trm5y[i]], TileProperties::Solid)) { - found = false; - } - if (dTransVal[xp + trm5x[i]][yp + trm5y[i]] != themes[t].ttval) { - found = false; - } - } + const PointsInRectangleRange searchArea { Rectangle { origin, 2 } }; + return std::all_of(searchArea.cbegin(), searchArea.cend(), [regionId](Point testPosition) { + // note out-of-bounds tiles are not solid, this function relies on the guard in TFit_Obj5 and dungeon border + if (IsTileSolid(testPosition)) { + return false; } + // If the theme object would extend into a different region then it doesn't fit. + if (dTransVal[testPosition.x][testPosition.y] != regionId) { + return false; + } + return true; + }); +} - if (!found) { - xp++; - if (xp == MAXDUNX) { - xp = 0; - yp++; - if (yp == MAXDUNY) { - if (r == rs) { - return false; - } - yp = 0; +bool TFit_Obj5(int t) +{ + int skipCandidates = GenerateRnd(5); + if (skipCandidates < 0) { + // vanilla rng can return -3 for GenerateRnd(5), default behaviour is to set the output to 0,0 and return true in this case... + themex = 0; + themey = 0; + return true; + } + + for (int yp = 0; yp < MAXDUNY; yp++) { + for (int xp = 0; xp < MAXDUNX; xp++) { + if (dTransVal[xp][yp] == themes[t].ttval && IsTileNotSolid({ xp, yp }) && CheckThemeObj5({ xp, yp }, themes[t].ttval)) { + if (skipCandidates > 0) { + skipCandidates--; + continue; } + themex = xp; + themey = yp; + return true; } - continue; } - - r--; } - - themex = xp; - themey = yp; - - return true; + return false; } bool TFit_SkelRoom(int t) diff --git a/test/random_test.cpp b/test/random_test.cpp index dc2f6ea26..65d5ff59c 100644 --- a/test/random_test.cpp +++ b/test/random_test.cpp @@ -237,4 +237,54 @@ TEST(RandomTest, ModDistributionSignPreserving) << "Distribution must map negative numbers using sign preserving modulo"; } +TEST(RandomTest, NegativeReturnValues) +{ + // The bug in vanilla RNG stemming from mod instead of bitmasking means that negative values are possible for + // non-power of 2 arguments to GenerateRnd + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(3), -2) << "Unexpected return value for a limit of 3"; + + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(5), -3) << "Unexpected return value for a limit of 5"; + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(6), -2) << "Unexpected return value for a limit of 6"; + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(7), -1) << "Unexpected return value for a limit of 7"; + + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(11), -10) << "Unexpected return value for a limit of 11"; + + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(17), -9) << "Unexpected return value for a limit of 17"; + + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(19), -12) << "Unexpected return value for a limit of 19"; + + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(22), -10) << "Unexpected return value for a limit of 22"; + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(23), -16) << "Unexpected return value for a limit of 23"; + + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(25), -18) << "Unexpected return value for a limit of 25"; + + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(27), -17) << "Unexpected return value for a limit of 27"; + + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(29), -27) << "Unexpected return value for a limit of 29"; + + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(31), -1) << "Unexpected return value for a limit of 31"; + + for (int i : { 9, 10, 12, 13, 14, 15, 18, 20, 21, 24, 26, 28, 30 }) { + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(i), -8) << "Unexpected return value for a limit of " << i; + } + + for (int i = 1; i < 32768; i *= 2) { + SetRndSeed(1457187811); + EXPECT_EQ(GenerateRnd(i), 0) << "Expect powers of 2 such as " << i << " to cleanly divide the int_min RNG value "; + } +} } // namespace devilution