From 252ee7f8c428559a27f4fa811a9898b07545f962 Mon Sep 17 00:00:00 2001
From: Alexei Kotov <alexdobrohotov@yandex.ru>
Date: Fri, 7 Jun 2024 13:10:20 +0300
Subject: [PATCH] Deduplicate effect list verification
Drop Potion-specific range check, it's irrelevant
---
apps/opencs/CMakeLists.txt | 2 +-
apps/opencs/model/tools/effectlistcheck.cpp | 88 +++++++++++++++++++
apps/opencs/model/tools/effectlistcheck.hpp | 30 +++++++
apps/opencs/model/tools/enchantmentcheck.cpp | 46 +---------
.../opencs/model/tools/referenceablecheck.cpp | 66 +-------------
apps/opencs/model/tools/spellcheck.cpp | 45 +---------
6 files changed, 129 insertions(+), 148 deletions(-)
create mode 100644 apps/opencs/model/tools/effectlistcheck.cpp
create mode 100644 apps/opencs/model/tools/effectlistcheck.hpp
diff --git a/apps/opencs/CMakeLists.txt b/apps/opencs/CMakeLists.txt
index d10882e0ae..f1cd246410 100644
--- a/apps/opencs/CMakeLists.txt
+++ b/apps/opencs/CMakeLists.txt
@@ -42,7 +42,7 @@ opencs_units (model/tools
mandatoryid skillcheck classcheck factioncheck racecheck soundcheck regioncheck
birthsigncheck spellcheck referencecheck referenceablecheck scriptcheck bodypartcheck
startscriptcheck search searchoperation searchstage pathgridcheck soundgencheck magiceffectcheck
- mergestages gmstcheck topicinfocheck journalcheck enchantmentcheck
+ mergestages gmstcheck topicinfocheck journalcheck enchantmentcheck effectlistcheck
)
opencs_hdrs (model/tools
diff --git a/apps/opencs/model/tools/effectlistcheck.cpp b/apps/opencs/model/tools/effectlistcheck.cpp
new file mode 100644
index 0000000000..b8695bc419
--- /dev/null
+++ b/apps/opencs/model/tools/effectlistcheck.cpp
@@ -0,0 +1,88 @@
+#include "effectlistcheck.hpp"
+
+#include <components/esm/attr.hpp>
+#include <components/esm3/effectlist.hpp>
+#include <components/esm3/loadingr.hpp>
+#include <components/esm3/loadmgef.hpp>
+#include <components/esm3/loadskil.hpp>
+
+#include <apps/opencs/model/doc/messages.hpp>
+#include <apps/opencs/model/world/universalid.hpp>
+
+namespace CSMTools
+{
+ void effectListCheck(
+ const std::vector<ESM::IndexedENAMstruct>& list, CSMDoc::Messages& messages, const CSMWorld::UniversalId& id)
+ {
+ if (list.empty())
+ {
+ messages.add(id, "No magic effects", "", CSMDoc::Message::Severity_Warning);
+ return;
+ }
+
+ size_t i = 1;
+ for (const ESM::IndexedENAMstruct& effect : list)
+ {
+ const std::string number = std::to_string(i);
+
+ // At the time of writing this effects, attributes and skills are mostly hardcoded
+ if (effect.mData.mEffectID < 0 || effect.mData.mEffectID >= ESM::MagicEffect::Length)
+ messages.add(id, "Effect #" + number + ": invalid effect ID", "", CSMDoc::Message::Severity_Error);
+ if (effect.mData.mSkill < -1 || effect.mData.mSkill >= ESM::Skill::Length)
+ messages.add(id, "Effect #" + number + ": invalid skill", "", CSMDoc::Message::Severity_Error);
+ if (effect.mData.mAttribute < -1 || effect.mData.mAttribute >= ESM::Attribute::Length)
+ messages.add(id, "Effect #" + number + ": invalid attribute", "", CSMDoc::Message::Severity_Error);
+
+ if (effect.mData.mRange < ESM::RT_Self || effect.mData.mRange > ESM::RT_Target)
+ messages.add(id, "Effect #" + number + ": invalid range", "", CSMDoc::Message::Severity_Error);
+
+ if (effect.mData.mArea < 0)
+ messages.add(id, "Effect #" + number + ": negative area", "", CSMDoc::Message::Severity_Error);
+
+ if (effect.mData.mDuration < 0)
+ messages.add(id, "Effect #" + number + ": negative duration", "", CSMDoc::Message::Severity_Error);
+
+ if (effect.mData.mMagnMin < 0)
+ messages.add(
+ id, "Effect #" + number + ": negative minimum magnitude", "", CSMDoc::Message::Severity_Error);
+
+ if (effect.mData.mMagnMax < 0)
+ messages.add(
+ id, "Effect #" + number + ": negative maximum magnitude", "", CSMDoc::Message::Severity_Error);
+ else if (effect.mData.mMagnMax == 0)
+ messages.add(
+ id, "Effect #" + number + ": zero maximum magnitude", "", CSMDoc::Message::Severity_Warning);
+
+ if (effect.mData.mMagnMin > effect.mData.mMagnMax)
+ messages.add(id, "Effect #" + number + ": minimum magnitude is higher than maximum magnitude", "",
+ CSMDoc::Message::Severity_Error);
+
+ ++i;
+ }
+ }
+
+ void ingredientEffectListCheck(
+ const ESM::Ingredient& ingredient, CSMDoc::Messages& messages, const CSMWorld::UniversalId& id)
+ {
+ bool hasEffects = false;
+
+ for (size_t i = 0; i < 4; i++)
+ {
+ if (ingredient.mData.mEffectID[i] == -1)
+ continue;
+
+ hasEffects = true;
+
+ const std::string number = std::to_string(i + 1);
+ if (ingredient.mData.mEffectID[i] < -1 || ingredient.mData.mEffectID[i] >= ESM::MagicEffect::Length)
+ messages.add(id, "Effect #" + number + ": invalid effect ID", "", CSMDoc::Message::Severity_Error);
+ if (ingredient.mData.mSkills[i] < -1 || ingredient.mData.mSkills[i] >= ESM::Skill::Length)
+ messages.add(id, "Effect #" + number + ": invalid skill", "", CSMDoc::Message::Severity_Error);
+ if (ingredient.mData.mAttributes[i] < -1 || ingredient.mData.mAttributes[i] >= ESM::Attribute::Length)
+ messages.add(id, "Effect #" + number + ": invalid attribute", "", CSMDoc::Message::Severity_Error);
+ }
+
+ if (!hasEffects)
+ messages.add(id, "No magic effects", "", CSMDoc::Message::Severity_Warning);
+ }
+}
diff --git a/apps/opencs/model/tools/effectlistcheck.hpp b/apps/opencs/model/tools/effectlistcheck.hpp
new file mode 100644
index 0000000000..832f3650eb
--- /dev/null
+++ b/apps/opencs/model/tools/effectlistcheck.hpp
@@ -0,0 +1,30 @@
+#ifndef CSM_TOOLS_EFFECTLISTCHECK_H
+#define CSM_TOOLS_EFFECTLISTCHECK_H
+
+#include <vector>
+
+namespace ESM
+{
+ struct IndexedENAMstruct;
+ struct Ingredient;
+}
+
+namespace CSMDoc
+{
+ class Messages;
+}
+
+namespace CSMWorld
+{
+ class UniversalId;
+}
+
+namespace CSMTools
+{
+ void effectListCheck(
+ const std::vector<ESM::IndexedENAMstruct>& list, CSMDoc::Messages& messages, const CSMWorld::UniversalId& id);
+ void ingredientEffectListCheck(
+ const ESM::Ingredient& ingredient, CSMDoc::Messages& messages, const CSMWorld::UniversalId& id);
+}
+
+#endif
diff --git a/apps/opencs/model/tools/enchantmentcheck.cpp b/apps/opencs/model/tools/enchantmentcheck.cpp
index ef1def6959..8bd7d6cc4c 100644
--- a/apps/opencs/model/tools/enchantmentcheck.cpp
+++ b/apps/opencs/model/tools/enchantmentcheck.cpp
@@ -10,14 +10,14 @@
#include <apps/opencs/model/world/idcollection.hpp>
#include <apps/opencs/model/world/record.hpp>
-#include <components/esm/attr.hpp>
-#include <components/esm3/effectlist.hpp>
#include <components/esm3/loadench.hpp>
#include "../prefs/state.hpp"
#include "../world/universalid.hpp"
+#include "effectlistcheck.hpp"
+
CSMTools::EnchantmentCheckStage::EnchantmentCheckStage(const CSMWorld::IdCollection<ESM::Enchantment>& enchantments)
: mEnchantments(enchantments)
{
@@ -55,45 +55,5 @@ void CSMTools::EnchantmentCheckStage::perform(int stage, CSMDoc::Messages& messa
if (enchantment.mData.mCost > enchantment.mData.mCharge)
messages.add(id, "Cost is higher than charge", "", CSMDoc::Message::Severity_Error);
- if (enchantment.mEffects.mList.empty())
- {
- messages.add(id, "Enchantment doesn't have any magic effects", "", CSMDoc::Message::Severity_Warning);
- }
- else
- {
- std::vector<ESM::IndexedENAMstruct>::const_iterator effect = enchantment.mEffects.mList.begin();
-
- for (size_t i = 1; i <= enchantment.mEffects.mList.size(); i++)
- {
- const std::string number = std::to_string(i);
- // At the time of writing this effects, attributes and skills are mostly hardcoded
- if (effect->mData.mEffectID < 0 || effect->mData.mEffectID > ESM::MagicEffect::Length)
- messages.add(id, "Effect #" + number + " is invalid", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mSkill < -1 || effect->mData.mSkill > ESM::Skill::Length)
- messages.add(
- id, "Effect #" + number + " affected skill is invalid", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mAttribute < -1 || effect->mData.mAttribute > ESM::Attribute::Length)
- messages.add(
- id, "Effect #" + number + " affected attribute is invalid", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mRange < 0 || effect->mData.mRange > 2)
- messages.add(id, "Effect #" + number + " range is invalid", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mArea < 0)
- messages.add(id, "Effect #" + number + " area is negative", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mDuration < 0)
- messages.add(id, "Effect #" + number + " duration is negative", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mMagnMin < 0)
- messages.add(
- id, "Effect #" + number + " minimum magnitude is negative", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mMagnMax < 0)
- messages.add(
- id, "Effect #" + number + " maximum magnitude is negative", "", CSMDoc::Message::Severity_Error);
- else if (effect->mData.mMagnMax == 0)
- messages.add(
- id, "Effect #" + number + " maximum magnitude is zero", "", CSMDoc::Message::Severity_Warning);
- if (effect->mData.mMagnMin > effect->mData.mMagnMax)
- messages.add(id, "Effect #" + number + " minimum magnitude is higher than maximum magnitude", "",
- CSMDoc::Message::Severity_Error);
- ++effect;
- }
- }
+ effectListCheck(enchantment.mEffects.mList, messages, id);
}
diff --git a/apps/opencs/model/tools/referenceablecheck.cpp b/apps/opencs/model/tools/referenceablecheck.cpp
index 55ed204aef..a00c3acd1c 100644
--- a/apps/opencs/model/tools/referenceablecheck.cpp
+++ b/apps/opencs/model/tools/referenceablecheck.cpp
@@ -38,6 +38,8 @@
#include "../world/record.hpp"
#include "../world/universalid.hpp"
+#include "effectlistcheck.hpp"
+
namespace ESM
{
class Script;
@@ -331,47 +333,7 @@ void CSMTools::ReferenceableCheckStage::potionCheck(
inventoryItemCheck<ESM::Potion>(potion, messages, id.toString());
- if (potion.mEffects.mList.empty())
- {
- messages.add(id, "Potion doesn't have any magic effects", "", CSMDoc::Message::Severity_Warning);
- }
- else
- {
- std::vector<ESM::IndexedENAMstruct>::const_iterator effect = potion.mEffects.mList.begin();
-
- for (size_t i = 1; i <= potion.mEffects.mList.size(); i++)
- {
- const std::string number = std::to_string(i);
- // At the time of writing this effects, attributes and skills are mostly hardcoded
- if (effect->mData.mEffectID < 0 || effect->mData.mEffectID > ESM::MagicEffect::Length)
- messages.add(id, "Effect #" + number + " is invalid", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mSkill < -1 || effect->mData.mSkill > ESM::Skill::Length)
- messages.add(
- id, "Effect #" + number + " affected skill is invalid", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mAttribute < -1 || effect->mData.mAttribute > ESM::Attribute::Length)
- messages.add(
- id, "Effect #" + number + " affected attribute is invalid", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mRange != ESM::RT_Self)
- messages.add(id, "Effect #" + number + " range is not Self", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mArea < 0)
- messages.add(id, "Effect #" + number + " area is negative", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mDuration < 0)
- messages.add(id, "Effect #" + number + " duration is negative", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mMagnMin < 0)
- messages.add(
- id, "Effect #" + number + " minimum magnitude is negative", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mMagnMax < 0)
- messages.add(
- id, "Effect #" + number + " maximum magnitude is negative", "", CSMDoc::Message::Severity_Error);
- else if (effect->mData.mMagnMax == 0)
- messages.add(
- id, "Effect #" + number + " maximum magnitude is zero", "", CSMDoc::Message::Severity_Warning);
- if (effect->mData.mMagnMin > effect->mData.mMagnMax)
- messages.add(id, "Effect #" + number + " minimum magnitude is higher than maximum magnitude", "",
- CSMDoc::Message::Severity_Error);
- ++effect;
- }
- }
+ effectListCheck(potion.mEffects.mList, messages, id);
// Check that mentioned scripts exist
scriptCheck<ESM::Potion>(potion, messages, id.toString());
@@ -607,27 +569,7 @@ void CSMTools::ReferenceableCheckStage::ingredientCheck(
// Check that mentioned scripts exist
scriptCheck<ESM::Ingredient>(ingredient, messages, id.toString());
- bool hasEffects = false;
- for (size_t i = 0; i < 4; i++)
- {
- if (ingredient.mData.mEffectID[i] == -1)
- continue;
-
- hasEffects = true;
-
- const std::string number = std::to_string(i + 1);
- // At the time of writing this effects, attributes and skills are mostly hardcoded
- if (ingredient.mData.mEffectID[i] < -1 || ingredient.mData.mEffectID[i] > ESM::MagicEffect::Length)
- messages.add(id, "Effect #" + number + " is invalid", "", CSMDoc::Message::Severity_Error);
- if (ingredient.mData.mSkills[i] < -1 || ingredient.mData.mSkills[i] > ESM::Skill::Length)
- messages.add(id, "Effect #" + number + " affected skill is invalid", "", CSMDoc::Message::Severity_Error);
- if (ingredient.mData.mAttributes[i] < -1 || ingredient.mData.mAttributes[i] > ESM::Attribute::Length)
- messages.add(
- id, "Effect #" + number + " affected attribute is invalid", "", CSMDoc::Message::Severity_Error);
- }
-
- if (!hasEffects)
- messages.add(id, "Ingredient doesn't have any magic effects", "", CSMDoc::Message::Severity_Warning);
+ ingredientEffectListCheck(ingredient, messages, id);
}
void CSMTools::ReferenceableCheckStage::creaturesLevListCheck(
diff --git a/apps/opencs/model/tools/spellcheck.cpp b/apps/opencs/model/tools/spellcheck.cpp
index e4f7322bcf..07973bf08b 100644
--- a/apps/opencs/model/tools/spellcheck.cpp
+++ b/apps/opencs/model/tools/spellcheck.cpp
@@ -9,11 +9,12 @@
#include <apps/opencs/model/world/record.hpp>
#include <apps/opencs/model/world/universalid.hpp>
-#include <components/esm/attr.hpp>
#include <components/esm3/loadspel.hpp>
#include "../prefs/state.hpp"
+#include "effectlistcheck.hpp"
+
CSMTools::SpellCheckStage::SpellCheckStage(const CSMWorld::IdCollection<ESM::Spell>& spells)
: mSpells(spells)
{
@@ -47,45 +48,5 @@ void CSMTools::SpellCheckStage::perform(int stage, CSMDoc::Messages& messages)
if (spell.mData.mCost < 0)
messages.add(id, "Spell cost is negative", "", CSMDoc::Message::Severity_Error);
- if (spell.mEffects.mList.empty())
- {
- messages.add(id, "Spell doesn't have any magic effects", "", CSMDoc::Message::Severity_Warning);
- }
- else
- {
- std::vector<ESM::IndexedENAMstruct>::const_iterator effect = spell.mEffects.mList.begin();
-
- for (size_t i = 1; i <= spell.mEffects.mList.size(); i++)
- {
- const std::string number = std::to_string(i);
- // At the time of writing this effects, attributes and skills are mostly hardcoded
- if (effect->mData.mEffectID < 0 || effect->mData.mEffectID > ESM::MagicEffect::Length)
- messages.add(id, "Effect #" + number + " is invalid", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mSkill < -1 || effect->mData.mSkill > ESM::Skill::Length)
- messages.add(
- id, "Effect #" + number + " affected skill is invalid", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mAttribute < -1 || effect->mData.mAttribute > ESM::Attribute::Length)
- messages.add(
- id, "Effect #" + number + " affected attribute is invalid", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mRange < 0 || effect->mData.mRange > 2)
- messages.add(id, "Effect #" + number + " range is invalid", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mArea < 0)
- messages.add(id, "Effect #" + number + " area is negative", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mDuration < 0)
- messages.add(id, "Effect #" + number + " duration is negative", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mMagnMin < 0)
- messages.add(
- id, "Effect #" + number + " minimum magnitude is negative", "", CSMDoc::Message::Severity_Error);
- if (effect->mData.mMagnMax < 0)
- messages.add(
- id, "Effect #" + number + " maximum magnitude is negative", "", CSMDoc::Message::Severity_Error);
- else if (effect->mData.mMagnMax == 0)
- messages.add(
- id, "Effect #" + number + " maximum magnitude is zero", "", CSMDoc::Message::Severity_Warning);
- if (effect->mData.mMagnMin > effect->mData.mMagnMax)
- messages.add(id, "Effect #" + number + " minimum magnitude is higher than maximum magnitude", "",
- CSMDoc::Message::Severity_Error);
- ++effect;
- }
- }
+ effectListCheck(spell.mEffects.mList, messages, id);
}