diff --git a/apps/opencs/CMakeLists.txt b/apps/opencs/CMakeLists.txt index 80aa04d42c..7b1e3ad930 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 +#include +#include +#include +#include + +#include +#include + +namespace CSMTools +{ + void effectListCheck( + const std::vector& 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 + +namespace ESM +{ + struct IndexedENAMstruct; + struct Ingredient; +} + +namespace CSMDoc +{ + class Messages; +} + +namespace CSMWorld +{ + class UniversalId; +} + +namespace CSMTools +{ + void effectListCheck( + const std::vector& 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 48cee579be..8bd7d6cc4c 100644 --- a/apps/opencs/model/tools/enchantmentcheck.cpp +++ b/apps/opencs/model/tools/enchantmentcheck.cpp @@ -10,13 +10,14 @@ #include #include -#include #include #include "../prefs/state.hpp" #include "../world/universalid.hpp" +#include "effectlistcheck.hpp" + CSMTools::EnchantmentCheckStage::EnchantmentCheckStage(const CSMWorld::IdCollection& enchantments) : mEnchantments(enchantments) { @@ -54,47 +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::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 hardcoded - if (effect->mData.mEffectID < 0 || effect->mData.mEffectID > 142) - { - messages.add(id, "Effect #" + number + " is invalid", "", CSMDoc::Message::Severity_Error); - ++effect; - continue; - } - - if (effect->mData.mSkill < -1 || effect->mData.mSkill > 26) - messages.add( - id, "Effect #" + number + " affected skill is invalid", "", CSMDoc::Message::Severity_Error); - if (effect->mData.mAttribute < -1 || effect->mData.mAttribute > 7) - 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); - 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 a692bc10d1..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; @@ -330,7 +332,8 @@ void CSMTools::ReferenceableCheckStage::potionCheck( CSMWorld::UniversalId id(CSMWorld::UniversalId::Type_Potion, potion.mId); inventoryItemCheck(potion, messages, id.toString()); - /// \todo Check magic effects for validity + + effectListCheck(potion.mEffects.mList, messages, id); // Check that mentioned scripts exist scriptCheck(potion, messages, id.toString()); @@ -565,6 +568,8 @@ void CSMTools::ReferenceableCheckStage::ingredientCheck( // Check that mentioned scripts exist scriptCheck(ingredient, messages, id.toString()); + + 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 f91438dc22..07973bf08b 100644 --- a/apps/opencs/model/tools/spellcheck.cpp +++ b/apps/opencs/model/tools/spellcheck.cpp @@ -13,6 +13,8 @@ #include "../prefs/state.hpp" +#include "effectlistcheck.hpp" + CSMTools::SpellCheckStage::SpellCheckStage(const CSMWorld::IdCollection& spells) : mSpells(spells) { @@ -46,5 +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); - /// \todo check data members that can't be edited in the table view + effectListCheck(spell.mEffects.mList, messages, id); } diff --git a/apps/opencs/model/world/nestedcoladapterimp.hpp b/apps/opencs/model/world/nestedcoladapterimp.hpp index e18cda9611..f5c5501889 100644 --- a/apps/opencs/model/world/nestedcoladapterimp.hpp +++ b/apps/opencs/model/world/nestedcoladapterimp.hpp @@ -319,12 +319,7 @@ namespace CSMWorld switch (subColIndex) { case 0: - { - if (effect.mEffectID >= 0 && effect.mEffectID < ESM::MagicEffect::Length) - return effect.mEffectID; - else - throw std::runtime_error("Magic effects ID unexpected value"); - } + return effect.mEffectID; case 1: { switch (effect.mEffectID) @@ -354,12 +349,7 @@ namespace CSMWorld } } case 3: - { - if (effect.mRange >= 0 && effect.mRange <= 2) - return effect.mRange; - else - throw std::runtime_error("Magic effects range unexpected value"); - } + return effect.mRange; case 4: return effect.mArea; case 5: diff --git a/apps/opencs/model/world/refidadapterimp.cpp b/apps/opencs/model/world/refidadapterimp.cpp index bdccab9cda..c25ad6e511 100644 --- a/apps/opencs/model/world/refidadapterimp.cpp +++ b/apps/opencs/model/world/refidadapterimp.cpp @@ -194,6 +194,26 @@ void CSMWorld::IngredEffectRefIdAdapter::setNestedData( { case 0: ingredient.mData.mEffectID[subRowIndex] = value.toInt(); + switch (ingredient.mData.mEffectID[subRowIndex]) + { + case ESM::MagicEffect::DrainSkill: + case ESM::MagicEffect::DamageSkill: + case ESM::MagicEffect::RestoreSkill: + case ESM::MagicEffect::FortifySkill: + case ESM::MagicEffect::AbsorbSkill: + ingredient.mData.mAttributes[subRowIndex] = -1; + break; + case ESM::MagicEffect::DrainAttribute: + case ESM::MagicEffect::DamageAttribute: + case ESM::MagicEffect::RestoreAttribute: + case ESM::MagicEffect::FortifyAttribute: + case ESM::MagicEffect::AbsorbAttribute: + ingredient.mData.mSkills[subRowIndex] = -1; + break; + default: + ingredient.mData.mSkills[subRowIndex] = -1; + ingredient.mData.mAttributes[subRowIndex] = -1; + } break; case 1: ingredient.mData.mSkills[subRowIndex] = value.toInt(); diff --git a/components/compiler/declarationparser.cpp b/components/compiler/declarationparser.cpp index d23e3c141a..19d2bae976 100644 --- a/components/compiler/declarationparser.cpp +++ b/components/compiler/declarationparser.cpp @@ -2,7 +2,9 @@ #include +#include "context.hpp" #include "errorhandler.hpp" +#include "extensions.hpp" #include "locals.hpp" #include "scanner.hpp" #include "skipparser.hpp" @@ -70,6 +72,15 @@ bool Compiler::DeclarationParser::parseKeyword(int keyword, const TokenLoc& loc, else if (mState == State_Name) { // allow keywords to be used as local variable names. MW script compiler, you suck! + if (const Extensions* extensions = getContext().getExtensions()) + { + std::string argumentType; + bool hasExplicit = false; + char returnType; + if (extensions->isFunction(keyword, returnType, argumentType, hasExplicit)) + getErrorHandler().warning("Local variable declaration shadows a function", loc); + } + return parseName(loc.mLiteral, loc, scanner); } else if (mState == State_End) @@ -104,6 +115,7 @@ bool Compiler::DeclarationParser::parseInt(int value, const TokenLoc& loc, Scann if (mState == State_Name) { // Allow integers to be used as variable names + getErrorHandler().warning("Integer is used as a local variable name", loc); return parseName(loc.mLiteral, loc, scanner); } return Parser::parseInt(value, loc, scanner);