Skip to content

Commit

Permalink
Add cyclic detection for loot reference
Browse files Browse the repository at this point in the history 
Choosed to set the invalid reference to 0 instead of shutting down the server.

Error in starting log should be enough clear for db Devs.
  • Loading branch information
@cyberium
cyberium committed Jan 5, 2024
1 parent b337f6f commit 8100292
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 34 deletions.
126 changes: 94 additions & 32 deletions src/game/Loot/LootMgr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class LootTemplate::LootGroup // A set of loot def
float TotalChance() const; // Overall chance for the group

void Verify(LootStore const& lootstore, uint32 id, uint32 group_id) const;
void CheckLootRefs(LootIdSet* ref_set) const;
bool CheckLootRefs(LootIdSet* ref_set, LootIdSet& prevRefs);
private:
LootStoreItemList ExplicitlyChanced; // Entries with chances defined in DB
LootStoreItemList EqualChanced; // Zero chances - every entry takes the same chance
Expand All @@ -95,6 +95,56 @@ void LootStore::Verify() const
m_LootTemplate.second->Verify(*this, m_LootTemplate.first);
}

// check if that loot template does not contain a ref to itself (should be called by CheckLootRefs())
bool IsValidReference(LootStoreItem& lsi, LootIdSet* refSet, LootIdSet& prevRefs)
{
if (lsi.mincountOrRef < 0)
{
auto lootRef = LootTemplates_Reference.GetLootFor(-lsi.mincountOrRef);
if (!lootRef)
{
LootTemplates_Reference.ReportNotExistedId(-lsi.mincountOrRef);
return false;
}
else
{
auto insSuccess = prevRefs.insert(-lsi.mincountOrRef);
if (!insSuccess.second)
{
std::string refsList = "";
for (auto id : prevRefs)
refsList += std::to_string(id) + " > ";
refsList += std::to_string(-lsi.mincountOrRef) + "!";
sLog.outErrorDb("Critical: Table '%s' reference %d (%s) is pointing to itself. Pathway: %s",
LootTemplates_Reference.GetName(), -lsi.mincountOrRef, LootTemplates_Reference.GetEntryName(), refsList.c_str());

// we can set it as used anyway
if (refSet)
refSet->erase(-lsi.mincountOrRef);

// set it to 0 so it will be ignored from now
lsi.mincountOrRef = 0;
return false;
}
else
{
// recursive call
bool success = const_cast<LootTemplate*>(lootRef)->CheckLootRefs(refSet, prevRefs);
prevRefs.erase(-lsi.mincountOrRef); // we can now remove this from the callee list
if (success)
{
// we can set it as used
if (refSet)
refSet->erase(-lsi.mincountOrRef);
}
else
return false;
}
}
}
return true;
};

// Loads a *_loot_template DB table into loot store
// All checks of the loaded template are called from here, no error reports at loot generation required
void LootStore::LoadLootTable()
Expand Down Expand Up @@ -257,10 +307,22 @@ void LootStore::LoadAndCheckReferenceNames()
}
}

void LootStore::CheckLootRefs(LootIdSet* ref_set) const
bool LootStore::CheckLootRefs(LootIdSet* ref_set /*= nullptr*/)
{
for (const auto& m_LootTemplate : m_LootTemplates)
m_LootTemplate.second->CheckLootRefs(ref_set);
LootIdSet prevRefs;
bool noIssue = true;
for (const auto& lTpl : m_LootTemplates)
{
prevRefs.clear();
if (!lTpl.second->CheckLootRefs(ref_set, prevRefs))
{
noIssue = false;

sLog.outErrorDb("Critical error found in '%s' for %s %d!", GetName(), GetEntryName(), lTpl.first);
}
}

return noIssue;
}

void LootStore::ReportUnusedIds(LootIdSet const& ids_set) const
Expand Down Expand Up @@ -352,6 +414,11 @@ bool LootStoreItem::IsValid(LootStore const& store, uint32 entry) const
sLog.outErrorDb("Table '%s' entry %d item %d: negative chance is given for a reference, skipped", store.GetName(), entry, itemid);
return false;
}
if (chance == 0 && group == 0) // no chance for the reference
{
sLog.outErrorDb("Table '%s' entry %d item %d: zero chance is given for a reference, reference will never be used, skipped", store.GetName(), entry, itemid);
return false;
}
}
return true; // Referenced template existence is checked at whole store level
}
Expand Down Expand Up @@ -2612,29 +2679,22 @@ void LootTemplate::LootGroup::Verify(LootStore const& lootstore, uint32 id, uint
}
}

void LootTemplate::LootGroup::CheckLootRefs(LootIdSet* ref_set) const
// Will try to find invalid reference and looped reference
// If loop is detected (Reference call itself) the reference will be set to invalid one
bool LootTemplate::LootGroup::CheckLootRefs(LootIdSet* ref_set, LootIdSet& prevRefs)
{
for (auto ieItr : ExplicitlyChanced)
for (auto& lsi : ExplicitlyChanced)
{
if (ieItr.mincountOrRef < 0)
{
if (!LootTemplates_Reference.GetLootFor(-ieItr.mincountOrRef))
LootTemplates_Reference.ReportNotExistedId(-ieItr.mincountOrRef);
else if (ref_set)
ref_set->erase(-ieItr.mincountOrRef);
}
if (!IsValidReference(lsi, ref_set, prevRefs))
return false;
}

for (auto ieItr : EqualChanced)
for (auto& lsi : EqualChanced)
{
if (ieItr.mincountOrRef < 0)
{
if (!LootTemplates_Reference.GetLootFor(-ieItr.mincountOrRef))
LootTemplates_Reference.ReportNotExistedId(-ieItr.mincountOrRef);
else if (ref_set)
ref_set->erase(-ieItr.mincountOrRef);
}
if (!IsValidReference(lsi, ref_set, prevRefs))
return false;
}
return true;
}

//
Expand Down Expand Up @@ -2786,21 +2846,23 @@ void LootTemplate::Verify(LootStore const& lootstore, uint32 id) const
// TODO: References validity checks
}

void LootTemplate::CheckLootRefs(LootIdSet* ref_set) const
// Will try to find invalid reference and looped reference
// If loop is detected (Reference call itself) the reference will be set to invalid one
bool LootTemplate::CheckLootRefs(LootIdSet* ref_set, LootIdSet& prevRefs)
{
for (auto Entrie : Entries)
for (auto& Entrie : Entries)
{
if (Entrie.mincountOrRef < 0)
{
if (!LootTemplates_Reference.GetLootFor(-Entrie.mincountOrRef))
LootTemplates_Reference.ReportNotExistedId(-Entrie.mincountOrRef);
else if (ref_set)
ref_set->erase(-Entrie.mincountOrRef);
}
if (!IsValidReference(Entrie, ref_set, prevRefs))
return false;
}

for (const auto& Group : Groups)
Group.CheckLootRefs(ref_set);
for (auto& Group : Groups)
{
if (!Group.CheckLootRefs(ref_set, prevRefs))
return false;
}

return true;
}

void LoadLootTemplates_Creature()
Expand Down
4 changes: 2 additions & 2 deletions src/game/Loot/LootMgr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ class LootStore

void LoadAndCollectLootIds(LootIdSet& ids_set);
void LoadAndCheckReferenceNames();
void CheckLootRefs(LootIdSet* ref_set = nullptr) const; // check existence reference and remove it from ref_set
bool CheckLootRefs(LootIdSet* ref_set = nullptr); // check existence reference and remove it from ref_set
void ReportUnusedIds(LootIdSet const& ids_set) const;
void ReportNotExistedId(uint32 id) const;

Expand Down Expand Up @@ -274,7 +274,7 @@ class LootTemplate

// Checks integrity of the template
void Verify(LootStore const& lootstore, uint32 id) const;
void CheckLootRefs(LootIdSet* ref_set) const;
bool CheckLootRefs(LootIdSet* ref_set, LootIdSet& prevRefs);
private:
LootStoreItemList Entries; // not grouped only
LootGroups Groups; // groups have own (optimized) processing, grouped entries go there
Expand Down

0 comments on commit 8100292

Please sign in to comment.