Skip to content

Commit

Permalink
Fix infinite loop in unique item randomization
Browse files Browse the repository at this point in the history
  • Loading branch information
obligaron authored and StephenCWills committed Sep 4, 2024
1 parent ca16398 commit 138f937
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 43 deletions.
4 changes: 2 additions & 2 deletions Source/itemdat.h
Original file line number Diff line number Diff line change
Expand Up @@ -645,10 +645,10 @@ struct UniqueItem {
ItemPower powers[6];
};

extern std::vector<ItemData> AllItemsList;
extern DVL_API_FOR_TEST std::vector<ItemData> AllItemsList;
extern std::vector<PLStruct> ItemPrefixes;
extern std::vector<PLStruct> ItemSuffixes;
extern std::vector<UniqueItem> UniqueItems;
extern DVL_API_FOR_TEST std::vector<UniqueItem> UniqueItems;

void LoadItemData();

Expand Down
106 changes: 66 additions & 40 deletions Source/items.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1419,19 +1419,25 @@ _item_indexes RndTypeItems(ItemType itemType, int imid, int lvl)
});
}

_unique_items CheckUnique(Item &item, int lvl, int uper, int uidOffset = 0)
std::vector<uint8_t> GetValidUniques(int lvl, unique_base_item baseItemId)
{
if (GenerateRnd(100) > uper)
return UITEM_INVALID;

std::vector<uint8_t> validUniques;
for (int j = 0; j < static_cast<int>(UniqueItems.size()); ++j) {
if (!IsUniqueAvailable(j))
break;
if (UniqueItems[j].UIItemId == AllItemsList[item.IDidx].iItemId && lvl >= UniqueItems[j].UIMinLvl) {
if (UniqueItems[j].UIItemId == baseItemId && lvl >= UniqueItems[j].UIMinLvl) {
validUniques.push_back(j);
}
}
return validUniques;
}

_unique_items CheckUnique(Item &item, int lvl, int uper, int uidOffset = 0)
{
if (GenerateRnd(100) > uper)
return UITEM_INVALID;

auto validUniques = GetValidUniques(lvl, AllItemsList[item.IDidx].iItemId);

if (validUniques.empty())
return UITEM_INVALID;
Expand Down Expand Up @@ -3284,22 +3290,21 @@ void TryRandomUniqueItem(Item &item, _item_indexes idx, int8_t mLevel, int uper,
if ((item._iCreateInfo & CF_UNIQUE) == 0 || item._iMiscId == IMISC_UNIQUE)
return;

std::vector<int> uids;
SetRndSeed(item._iSeed);

// Get item base level, which is used in CheckUnique to get the correct valid uniques for the base item.
DiscardRandomValues(1); // GetItemAttrs
int blvl = GetItemBLevel(mLevel, item._iMiscId, onlygood, uper == 15);

// Gather all potential unique items. uid is the index into UniqueItems.
for (size_t i = 0; i < UniqueItems.size(); ++i) {
const UniqueItem &uniqueItem = UniqueItems[i];
// Verify the unique item base item matches idx.
bool isMatchingItemId = uniqueItem.UIItemId == AllItemsList[static_cast<size_t>(idx)].iItemId;
// Verify itemLvl is at least the unique's minimum required level.
// mLevel remains unadjusted when arriving in this function, and there is no call to set iblvl until CheckUnique(), so we adjust here.
bool meetsLevelRequirement = ((uper == 15) ? mLevel + 4 : mLevel) >= uniqueItem.UIMinLvl;
auto validUniques = GetValidUniques(blvl, AllItemsList[static_cast<size_t>(idx)].iItemId);
assert(!validUniques.empty());
std::vector<int> uids;
for (auto &possibleUid : validUniques) {
// Verify item hasn't been dropped yet. We set this to true in MP, since uniques previously dropping shouldn't prevent further identical uniques from dropping.
bool uniqueNotDroppedAlready = !UniqueItemFlags[i] || gbIsMultiplayer;

int uid = static_cast<int>(i);
if (IsUniqueAvailable(uid) && isMatchingItemId && meetsLevelRequirement && uniqueNotDroppedAlready)
uids.emplace_back(uid);
if (!UniqueItemFlags[possibleUid] || gbIsMultiplayer) {
uids.emplace_back(possibleUid);
}
}

// If we find at least one unique in uids that hasn't been obtained yet, we can proceed getting a random unique.
Expand All @@ -3324,45 +3329,66 @@ void TryRandomUniqueItem(Item &item, _item_indexes idx, int8_t mLevel, int uper,

int32_t uidsIdx = std::max<int32_t>(0, GenerateRnd(static_cast<int32_t>(uids.size()))); // Index into uids, used to get a random uid from the uids vector.
int uid = uids[uidsIdx]; // Actual unique id.
const UniqueItem &uniqueItem = UniqueItems[uid];

// If the selected unique was already generated, there is no need to fiddle with its parameters.
if (item._iUid == uid) {
UniqueItemFlags[uid] = true;
if (!gbIsMultiplayer) {
UniqueItemFlags[uid] = true;
}
return;
}

const UniqueItem &uniqueItem = UniqueItems[uid];
int targetLvl = 1; // Target level for reverse compatibility, since vanilla always takes the last applicable uid in the list.

// Set target level. Ideally we use uper 15 to have a 16% chance of generating a unique item.
if (uniqueItem.UIMinLvl - 4 > 0) { // Negative level will underflow. Lvl 0 items may have unintended consequences.
uper = 15;
targetLvl = uniqueItem.UIMinLvl - 4;
} else {
uper = 1;
targetLvl = uniqueItem.UIMinLvl;
// Find our own id to calculate the offset in validUniques and check if we can generate a reverse-compatible version of the item.
int uidOffset = -1;
bool canGenerateReverseCompatible = true;
for (size_t i = 0; i < validUniques.size(); i++) {
if (validUniques[i] == uid) {
// Vanilla always picks the last unique, so the offset is calculated from the back of the valid unique list.
uidOffset = static_cast<int>(validUniques.size() - i - 1);
} else if (uidOffset != -1 && UniqueItems[validUniques[i]].UIMinLvl <= uniqueItem.UIMinLvl) {
// Found an item with same or lower level as our desired unique after our unique.
// This means that we cannot possibly generate the item in reverse compatible mode and must rely on an offset.
canGenerateReverseCompatible = false;
}
}
assert(uidOffset != -1);

// Amount to decrease the final uid by in CheckUnique() to get the desired unique.
const auto uidOffset = static_cast<int>(std::count_if(UniqueItems.begin() + uid + 1, UniqueItems.end(), [&uniqueItem](UniqueItem &potentialMatch) {
return uniqueItem.UIItemId == potentialMatch.UIItemId && uniqueItem.UIMinLvl == potentialMatch.UIMinLvl;
}));
const Point itemPos = item.position;
if (canGenerateReverseCompatible) {
int targetLvl = 1; // Target level for reverse compatibility, since vanilla always takes the last applicable uid in the list.

Point itemPos = item.position;
// Set target level. Ideally we use uper 15 to have a 16% chance of generating a unique item.
if (uniqueItem.UIMinLvl - 4 > 0) { // Negative level will underflow. Lvl 0 items may have unintended consequences.
uper = 15;
targetLvl = uniqueItem.UIMinLvl - 4;
} else {
uper = 1;
targetLvl = uniqueItem.UIMinLvl;
}

// Force generate items until we find a uid match.
DiabloGenerator itemGenerator(item._iSeed);
do {
// Force generate items until we find a uid match.
DiabloGenerator itemGenerator(item._iSeed);
do {
item = {}; // Reset item data
item.position = itemPos;
// Set onlygood = true, to always get the required item base level for the unique.
SetupAllItems(*MyPlayer, item, idx, itemGenerator.advanceRndSeed(), targetLvl, uper, true, pregen);
} while (item._iUid != uid);
} else {
// Recreate the item with new offset, this creates the desired unique item but is not reverse compatible.
const int seed = item._iSeed;
item = {}; // Reset item data
item.position = itemPos;
SetupAllItems(*MyPlayer, item, idx, itemGenerator.advanceRndSeed(), targetLvl, uper, onlygood, pregen, uidOffset);
} while (item._iUid != uid);
SetupAllItems(*MyPlayer, item, idx, seed, mLevel, uper, onlygood, pregen, uidOffset);
item.dwBuff |= (uidOffset << 1) & CF_UIDOFFSET;
assert(item._iUid == uid);
}

// Set item as obtained to prevent it from being dropped again in SP.
if (!gbIsMultiplayer) {
UniqueItemFlags[uid] = true;
}
item.dwBuff |= (uidOffset << 1) & CF_UIDOFFSET;
}

void SpawnItem(Monster &monster, Point position, bool sendmsg, bool spawn /*= false*/)
Expand Down
2 changes: 1 addition & 1 deletion Source/items.h
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ extern uint8_t ActiveItemCount;
extern int8_t dItem[MAXDUNX][MAXDUNY];
extern bool ShowUniqueItemInfoBox;
extern CornerStoneStruct CornerStone;
extern bool UniqueItemFlags[128];
extern DVL_API_FOR_TEST bool UniqueItemFlags[128];

uint8_t GetOutlineColor(const Item &item, bool checkReq);
bool IsItemAvailable(int i);
Expand Down
1 change: 1 addition & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ set(tests
drlg_l4_test
effects_test
inv_test
items_test
math_test
missiles_test
pack_test
Expand Down
120 changes: 120 additions & 0 deletions test/items_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
#include <gtest/gtest.h>
#include <random>

#include "engine/random.hpp"
#include "items.h"
#include "player.h"
#include "spells.h"
#include "utils/str_cat.hpp"

namespace devilution {
namespace {

class ItemsTest : public ::testing::Test {
public:
void SetUp() override
{
Players.resize(1);
MyPlayer = &Players[0];
for (auto &flag : UniqueItemFlags)
flag = false;
}

static void SetUpTestSuite()
{
LoadItemData();
LoadSpellData();
gbIsSpawn = false;
gbIsMultiplayer = false; // to also test UniqueItemFlags
}
};

void GenerateAllUniques(bool hellfire, const size_t expectedUniques)
{
gbIsHellfire = hellfire;

std::mt19937 betterRng;
std::uniform_int_distribution<uint32_t> dist(0, std::numeric_limits<uint32_t>::max());

// Get seed for test run from time. If a test run fails, remember the seed and hardcode it here.
uint32_t testRunSeed = static_cast<uint32_t>(time(nullptr));
betterRng.seed(testRunSeed);

std::set<int> foundUniques;

constexpr int max_iterations = 1000000;
int iteration = 0;

for (int uniqueIndex = 0, n = static_cast<int>(UniqueItems.size()); uniqueIndex < n; ++uniqueIndex) {

if (!IsUniqueAvailable(uniqueIndex))
continue;

if (foundUniques.contains(uniqueIndex))
continue;

auto &uniqueItem = UniqueItems[uniqueIndex];

_item_indexes uniqueBaseIndex = IDI_GOLD;
for (std::underlying_type_t<_item_indexes> j = IDI_GOLD; j <= IDI_LAST; j++) {
if (!IsItemAvailable(j))
continue;
if (AllItemsList[j].iItemId != uniqueItem.UIItemId)
continue;
if (AllItemsList[j].iRnd != IDROP_NEVER)
uniqueBaseIndex = static_cast<_item_indexes>(j);
break;
}

if (uniqueBaseIndex == IDI_GOLD)
continue; // Unique not dropable (Quest-Unique)

auto &baseItemData = AllItemsList[static_cast<size_t>(uniqueBaseIndex)];

while (true) {
iteration += 1;

if (iteration > max_iterations)
FAIL() << StrCat("Item ", uniqueItem.UIName, " not found in ", max_iterations, " tries with test run seed ", testRunSeed);

Item testItem = {};
testItem._iMiscId = baseItemData.iMiscId;
std::uniform_int_distribution<int32_t> dist(0, INT_MAX);
SetRndSeed(dist(betterRng));

int targetLvl = uniqueItem.UIMinLvl;
int uper = 1;
if (uniqueItem.UIMinLvl - 4 > 0) {
uper = 15;
targetLvl = uniqueItem.UIMinLvl - 4;
}

SetupAllItems(*MyPlayer, testItem, uniqueBaseIndex, AdvanceRndSeed(), targetLvl, uper, true, false);
TryRandomUniqueItem(testItem, uniqueBaseIndex, targetLvl, uper, true, false);
SetupItem(testItem);

if (testItem._iMagical != ITEM_QUALITY_UNIQUE)
continue;

foundUniques.insert(testItem._iUid);

if (testItem._iUid == uniqueIndex)
break;
}
}

EXPECT_EQ(foundUniques.size(), expectedUniques) << StrCat("test run seed ", testRunSeed);
}

TEST_F(ItemsTest, AllDiabloUniquesCanDrop)
{
GenerateAllUniques(false, 79);
}

TEST_F(ItemsTest, AllHellfireUniquesCanDrop)
{
GenerateAllUniques(true, 99);
}

} // namespace
} // namespace devilution

0 comments on commit 138f937

Please sign in to comment.