From 29a46b0fc0f617d8ac153d0fa2d97ce0e21a6404 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Thu, 3 Nov 2022 20:54:44 +0100 Subject: [PATCH 1/2] Reimplement --script-all-dialogue to be more useful to modders --- apps/openmw/engine.cpp | 3 +- apps/openmw/mwdialogue/filter.cpp | 82 +++++++++--- apps/openmw/mwdialogue/filter.hpp | 6 +- apps/openmw/mwdialogue/scripttest.cpp | 181 ++++++++++++++++---------- 4 files changed, 182 insertions(+), 90 deletions(-) diff --git a/apps/openmw/engine.cpp b/apps/openmw/engine.cpp index 882be2d950..e3ad2bd996 100644 --- a/apps/openmw/engine.cpp +++ b/apps/openmw/engine.cpp @@ -814,8 +814,7 @@ void OMW::Engine::prepareEngine() { std::pair result = MWDialogue::ScriptTest::compileAll(&mExtensions, mWarningsMode); if (result.first) - Log(Debug::Info) << "compiled " << result.second << " of " << result.first - << " dialogue script/actor combinations a(" + Log(Debug::Info) << "compiled " << result.second << " of " << result.first << " dialogue scripts (" << 100 * static_cast(result.second) / result.first << "%)"; } diff --git a/apps/openmw/mwdialogue/filter.cpp b/apps/openmw/mwdialogue/filter.cpp index 6ecff18205..c036843121 100644 --- a/apps/openmw/mwdialogue/filter.cpp +++ b/apps/openmw/mwdialogue/filter.cpp @@ -24,6 +24,68 @@ #include "selectwrapper.hpp" +namespace +{ + bool matchesStaticFilters(const MWDialogue::SelectWrapper& select, const MWWorld::Ptr& actor) + { + if (select.getFunction() == MWDialogue::SelectWrapper::Function_NotId) + return !Misc::StringUtils::ciEqual(actor.getCellRef().getRefId(), select.getName()); + if (actor.getClass().isNpc()) + { + switch (select.getFunction()) + { + case MWDialogue::SelectWrapper::Function_NotFaction: + return !Misc::StringUtils::ciEqual(actor.getClass().getPrimaryFaction(actor), select.getName()); + case MWDialogue::SelectWrapper::Function_NotClass: + return !Misc::StringUtils::ciEqual(actor.get()->mBase->mClass, select.getName()); + case MWDialogue::SelectWrapper::Function_NotRace: + return !Misc::StringUtils::ciEqual(actor.get()->mBase->mRace, select.getName()); + } + } + return true; + } + + bool matchesStaticFilters(const ESM::DialInfo& info, const MWWorld::Ptr& actor) + { + for (const ESM::DialInfo::SelectStruct& select : info.mSelects) + { + MWDialogue::SelectWrapper wrapper = select; + + switch (wrapper.getType()) + { + case MWDialogue::SelectWrapper::Type_Boolean: + { + if (!wrapper.selectCompare(matchesStaticFilters(wrapper, actor))) + return false; + break; + } + case MWDialogue::SelectWrapper::Type_Inverted: + { + if (!matchesStaticFilters(wrapper, actor)) + return false; + break; + } + case MWDialogue::SelectWrapper::Type_Numeric: + { + if (wrapper.getFunction() == MWDialogue::SelectWrapper::Function_Local) + { + std::string_view scriptName = actor.getClass().getScript(actor); + if (scriptName.empty()) + return false; + const Compiler::Locals& localDefs + = MWBase::Environment::get().getScriptManager()->getLocals(scriptName); + char type = localDefs.getType(wrapper.getName()); + if (type == ' ') + return false; // script does not have a variable of this name. + } + break; + } + } + } + return true; + } +} + bool MWDialogue::Filter::testActor(const ESM::DialInfo& info) const { bool isCreature = (mActor.getType() != ESM::NPC::sRecordId); @@ -144,9 +206,7 @@ bool MWDialogue::Filter::testPlayer(const ESM::DialInfo& info) const { // supports partial matches, just like getPcCell std::string_view playerCell = MWBase::Environment::get().getWorld()->getCellName(player.getCell()); - bool match = playerCell.length() >= info.mCell.length() - && Misc::StringUtils::ciEqual(playerCell.substr(0, info.mCell.length()), info.mCell); - if (!match) + if (!Misc::StringUtils::ciStartsWith(playerCell, info.mCell)) return false; } @@ -183,7 +243,7 @@ bool MWDialogue::Filter::testFunctionLocal(const MWDialogue::SelectWrapper& sele if (scriptName.empty()) return false; // no script - std::string name = Misc::StringUtils::lowerCase(select.getName()); + std::string name = select.getName(); const Compiler::Locals& localDefs = MWBase::Environment::get().getScriptManager()->getLocals(scriptName); @@ -502,8 +562,7 @@ bool MWDialogue::Filter::getSelectStructBoolean(const SelectWrapper& select) con case SelectWrapper::Function_NotCell: { std::string_view actorCell = MWBase::Environment::get().getWorld()->getCellName(mActor.getCell()); - return !(actorCell.length() >= select.getName().length() - && Misc::StringUtils::ciEqual(actorCell.substr(0, select.getName().length()), select.getName())); + return !Misc::StringUtils::ciStartsWith(actorCell, select.getName()); } case SelectWrapper::Function_SameGender: @@ -640,16 +699,9 @@ const ESM::DialInfo* MWDialogue::Filter::search(const ESM::Dialogue& dialogue, c return suitableInfos[0]; } -std::vector MWDialogue::Filter::listAll(const ESM::Dialogue& dialogue) const +bool MWDialogue::Filter::couldPotentiallyMatch(const ESM::DialInfo& info) const { - std::vector infos; - for (ESM::Dialogue::InfoContainer::const_iterator iter = dialogue.mInfo.begin(); iter != dialogue.mInfo.end(); - ++iter) - { - if (testActor(*iter)) - infos.push_back(&*iter); - } - return infos; + return testActor(info) && matchesStaticFilters(info, mActor); } std::vector MWDialogue::Filter::list( diff --git a/apps/openmw/mwdialogue/filter.hpp b/apps/openmw/mwdialogue/filter.hpp index 3db8a97790..7e384be4bf 100644 --- a/apps/openmw/mwdialogue/filter.hpp +++ b/apps/openmw/mwdialogue/filter.hpp @@ -58,9 +58,9 @@ namespace MWDialogue ///< List all infos that could be used on the given actor, using the current runtime state of the actor. /// \note If fallbackToInfoRefusal is used, the returned DialInfo might not be from the supplied ESM::Dialogue. - std::vector listAll(const ESM::Dialogue& dialogue) const; - ///< List all infos that could possibly be used on the given actor, ignoring runtime state filters and ignoring - ///< player filters. + bool couldPotentiallyMatch(const ESM::DialInfo& info) const; + ///< Check if this INFO could potentially be said by the given actor, ignoring runtime state filters and + ///< ignoring player filters. const ESM::DialInfo* search(const ESM::Dialogue& dialogue, const bool fallbackToInfoRefusal) const; ///< Get a matching response for the requested dialogue. diff --git a/apps/openmw/mwdialogue/scripttest.cpp b/apps/openmw/mwdialogue/scripttest.cpp index be10479fdc..e993f21d1b 100644 --- a/apps/openmw/mwdialogue/scripttest.cpp +++ b/apps/openmw/mwdialogue/scripttest.cpp @@ -21,81 +21,63 @@ #include "filter.hpp" +#include +#include #include namespace { - void test( - const MWWorld::Ptr& actor, int& compiled, int& total, const Compiler::Extensions* extensions, int warningsMode) + bool test(const MWWorld::Ptr& actor, const ESM::DialInfo& info, int& compiled, int& total, + const Compiler::Extensions* extensions, const MWScript::CompilerContext& context, + Compiler::StreamErrorHandler& errorHandler) { - MWDialogue::Filter filter(actor, 0, false); - - MWScript::CompilerContext compilerContext(MWScript::CompilerContext::Type_Dialogue); - compilerContext.setExtensions(extensions); - Compiler::StreamErrorHandler errorHandler; - errorHandler.setWarningsMode(warningsMode); - - const MWWorld::Store& dialogues - = MWBase::Environment::get().getWorld()->getStore().get(); - for (MWWorld::Store::iterator it = dialogues.begin(); it != dialogues.end(); ++it) + bool success = true; + ++total; + try { - std::vector infos = filter.listAll(*it); - - for (std::vector::iterator iter = infos.begin(); iter != infos.end(); ++iter) - { - const ESM::DialInfo* info = *iter; - if (!info->mResultScript.empty()) - { - bool success = true; - ++total; - try - { - errorHandler.reset(); - - std::istringstream input(info->mResultScript + "\n"); + std::istringstream input(info.mResultScript + "\n"); - Compiler::Scanner scanner(errorHandler, input, extensions); + Compiler::Scanner scanner(errorHandler, input, extensions); - Compiler::Locals locals; + Compiler::Locals locals; - std::string_view actorScript = actor.getClass().getScript(actor); + std::string_view actorScript = actor.getClass().getScript(actor); + if (!actorScript.empty()) + { + // grab local variables from actor's script, if available. + locals = MWBase::Environment::get().getScriptManager()->getLocals(actorScript); + } - if (!actorScript.empty()) - { - // grab local variables from actor's script, if available. - locals = MWBase::Environment::get().getScriptManager()->getLocals(actorScript); - } + Compiler::ScriptParser parser(errorHandler, context, locals, false); - Compiler::ScriptParser parser(errorHandler, compilerContext, locals, false); + scanner.scan(parser); - scanner.scan(parser); + if (!errorHandler.isGood()) + success = false; - if (!errorHandler.isGood()) - success = false; + ++compiled; + } + catch (const Compiler::SourceException&) + { + // error has already been reported via error handler + success = false; + } + catch (const std::exception& error) + { + Log(Debug::Error) << "Dialogue error: An exception has been thrown: " << error.what(); + success = false; + } - ++compiled; - } - catch (const Compiler::SourceException& /* error */) - { - // error has already been reported via error handler - success = false; - } - catch (const std::exception& error) - { - Log(Debug::Error) - << std::string("Dialogue error: An exception has been thrown: ") + error.what(); - success = false; - } + return success; + } - if (!success) - { - Log(Debug::Error) << "Error: compiling failed (dialogue script): \n" - << info->mResultScript << "\n"; - } - } - } - } + bool superficiallyMatches(const MWWorld::Ptr& ptr, const ESM::DialInfo& info) + { + if (ptr.isEmpty()) + return false; + MWDialogue::Filter filter(ptr, 0, false); + return filter.couldPotentiallyMatch(info); } } @@ -109,20 +91,79 @@ namespace MWDialogue std::pair compileAll(const Compiler::Extensions* extensions, int warningsMode) { int compiled = 0, total = 0; - const MWWorld::Store& npcs = MWBase::Environment::get().getWorld()->getStore().get(); - for (MWWorld::Store::iterator it = npcs.begin(); it != npcs.end(); ++it) - { - MWWorld::ManualRef ref(MWBase::Environment::get().getWorld()->getStore(), it->mId); - test(ref.getPtr(), compiled, total, extensions, warningsMode); - } + const auto& store = MWBase::Environment::get().getWorld()->getStore(); + + MWScript::CompilerContext compilerContext(MWScript::CompilerContext::Type_Dialogue); + compilerContext.setExtensions(extensions); + Compiler::StreamErrorHandler errorHandler; + errorHandler.setWarningsMode(warningsMode); - const MWWorld::Store& creatures - = MWBase::Environment::get().getWorld()->getStore().get(); - for (MWWorld::Store::iterator it = creatures.begin(); it != creatures.end(); ++it) + std::unique_ptr ref; + for (const ESM::Dialogue& topic : store.get()) { - MWWorld::ManualRef ref(MWBase::Environment::get().getWorld()->getStore(), it->mId); - test(ref.getPtr(), compiled, total, extensions, warningsMode); + MWWorld::Ptr ptr; + for (const ESM::DialInfo& info : topic.mInfo) + { + if (info.mResultScript.empty()) + continue; + if (!info.mActor.empty()) + { + // We know it can only ever be this actor + try + { + ref = std::make_unique(store, info.mActor); + ptr = ref->getPtr(); + } + catch (const std::logic_error&) + { + // Too bad it doesn't exist + ptr = {}; + } + } + else + { + // Try to find a matching actor + if (!superficiallyMatches(ptr, info)) + { + ptr = {}; + bool found = false; + for (const auto& npc : store.get()) + { + ref = std::make_unique(store, npc.mId); + if (superficiallyMatches(ref->getPtr(), info)) + { + ptr = ref->getPtr(); + found = true; + break; + } + } + if (!found) + { + for (const auto& creature : store.get()) + { + ref = std::make_unique(store, creature.mId); + if (superficiallyMatches(ref->getPtr(), info)) + { + ptr = ref->getPtr(); + break; + } + } + } + } + } + if (ptr.isEmpty()) + Log(Debug::Error) << "Could not find an actor to test " << info.mId << " in " << topic.mId; + else + { + errorHandler.reset(); + errorHandler.setContext(info.mId + " in " + topic.mId); + if (!test(ptr, info, compiled, total, extensions, compilerContext, errorHandler)) + Log(Debug::Error) << "Test failed for " << info.mId << " in " << topic.mId << '\n' + << info.mResultScript; + } + } } + return std::make_pair(total, compiled); } From f61083d02cd9443ba898f7afcfbe2231a9dabbcf Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Fri, 4 Nov 2022 12:20:37 +0100 Subject: [PATCH 2/2] Replace switch statements --- apps/openmw/mwdialogue/filter.cpp | 60 +++++++++++++------------------ 1 file changed, 25 insertions(+), 35 deletions(-) diff --git a/apps/openmw/mwdialogue/filter.cpp b/apps/openmw/mwdialogue/filter.cpp index c036843121..9e51100da4 100644 --- a/apps/openmw/mwdialogue/filter.cpp +++ b/apps/openmw/mwdialogue/filter.cpp @@ -32,15 +32,12 @@ namespace return !Misc::StringUtils::ciEqual(actor.getCellRef().getRefId(), select.getName()); if (actor.getClass().isNpc()) { - switch (select.getFunction()) - { - case MWDialogue::SelectWrapper::Function_NotFaction: - return !Misc::StringUtils::ciEqual(actor.getClass().getPrimaryFaction(actor), select.getName()); - case MWDialogue::SelectWrapper::Function_NotClass: - return !Misc::StringUtils::ciEqual(actor.get()->mBase->mClass, select.getName()); - case MWDialogue::SelectWrapper::Function_NotRace: - return !Misc::StringUtils::ciEqual(actor.get()->mBase->mRace, select.getName()); - } + if (select.getFunction() == MWDialogue::SelectWrapper::Function_NotFaction) + return !Misc::StringUtils::ciEqual(actor.getClass().getPrimaryFaction(actor), select.getName()); + else if (select.getFunction() == MWDialogue::SelectWrapper::Function_NotClass) + return !Misc::StringUtils::ciEqual(actor.get()->mBase->mClass, select.getName()); + else if (select.getFunction() == MWDialogue::SelectWrapper::Function_NotRace) + return !Misc::StringUtils::ciEqual(actor.get()->mBase->mRace, select.getName()); } return true; } @@ -50,35 +47,28 @@ namespace for (const ESM::DialInfo::SelectStruct& select : info.mSelects) { MWDialogue::SelectWrapper wrapper = select; - - switch (wrapper.getType()) + if (wrapper.getType() == MWDialogue::SelectWrapper::Type_Boolean) { - case MWDialogue::SelectWrapper::Type_Boolean: - { - if (!wrapper.selectCompare(matchesStaticFilters(wrapper, actor))) - return false; - break; - } - case MWDialogue::SelectWrapper::Type_Inverted: + if (!wrapper.selectCompare(matchesStaticFilters(wrapper, actor))) + return false; + } + else if (wrapper.getType() == MWDialogue::SelectWrapper::Type_Inverted) + { + if (!matchesStaticFilters(wrapper, actor)) + return false; + } + else if (wrapper.getType() == MWDialogue::SelectWrapper::Type_Numeric) + { + if (wrapper.getFunction() == MWDialogue::SelectWrapper::Function_Local) { - if (!matchesStaticFilters(wrapper, actor)) + std::string_view scriptName = actor.getClass().getScript(actor); + if (scriptName.empty()) return false; - break; - } - case MWDialogue::SelectWrapper::Type_Numeric: - { - if (wrapper.getFunction() == MWDialogue::SelectWrapper::Function_Local) - { - std::string_view scriptName = actor.getClass().getScript(actor); - if (scriptName.empty()) - return false; - const Compiler::Locals& localDefs - = MWBase::Environment::get().getScriptManager()->getLocals(scriptName); - char type = localDefs.getType(wrapper.getName()); - if (type == ' ') - return false; // script does not have a variable of this name. - } - break; + const Compiler::Locals& localDefs + = MWBase::Environment::get().getScriptManager()->getLocals(scriptName); + char type = localDefs.getType(wrapper.getName()); + if (type == ' ') + return false; // script does not have a variable of this name. } } }