Rework Lua bindings for journal

apps/openmw/mwbase/journal.hpp

@@ -49,9 +49,9 @@ namespace MWBase
 
         virtual ~Journal() {}
 
-        virtual MWDialogue::Quest& getQuest(const ESM::RefId& id) = 0;
+        virtual MWDialogue::Quest& getOrStartQuest(const ESM::RefId& id) = 0;
-        ///< Gets the quest requested. Creates it and inserts it in quests if it does not yet exist.
+        ///< Gets the quest requested. Creates it and inserts it in quests if it is not yet started.
-        virtual MWDialogue::Quest* getQuestPtr(const ESM::RefId& id) = 0;
+        virtual MWDialogue::Quest* getQuestOrNull(const ESM::RefId& id) = 0;
         ///< Gets a pointer to the requested quest. Will return nullptr if the quest has not been started.
 
         virtual void addEntry(const ESM::RefId& id, int index, const MWWorld::Ptr& actor) = 0;
@@ -61,9 +61,6 @@ namespace MWBase
         virtual void setJournalIndex(const ESM::RefId& id, int index) = 0;
         ///< Set the journal index without adding an entry.
 
-        virtual int getQuestCount() const = 0;
-        ///< Get the count of quests stored.
-
         virtual int getJournalIndex(const ESM::RefId& id) const = 0;
         ///< Get the journal index.
 

apps/openmw/mwdialogue/journalimp.cpp

@@ -18,20 +18,17 @@
 
 namespace MWDialogue
 {
-    Quest& Journal::getQuest(const ESM::RefId& id)
+    Quest& Journal::getOrStartQuest(const ESM::RefId& id)
     {
         TQuestContainer::iterator iter = mQuests.find(id);
 
         if (iter == mQuests.end())
-        {
+            iter = mQuests.emplace(id, Quest(id)).first;
-            std::pair<TQuestContainer::iterator, bool> result = mQuests.insert(std::make_pair(id, Quest(id)));
-
-            iter = result.first;
-        }
 
         return iter->second;
     }
-    Quest* Journal::getQuestPtr(const ESM::RefId& id)
+
+    Quest* Journal::getQuestOrNull(const ESM::RefId& id)
     {
         TQuestContainer::iterator iter = mQuests.find(id);
         if (iter == mQuests.end())
@@ -99,7 +96,7 @@ namespace MWDialogue
 
         StampedJournalEntry entry = StampedJournalEntry::makeFromQuest(id, index, actor);
 
-        Quest& quest = getQuest(id);
+        Quest& quest = getOrStartQuest(id);
         if (quest.addEntry(entry)) // we are doing slicing on purpose here
         {
             // Restart all "other" quests with the same name as well
@@ -121,7 +118,7 @@ namespace MWDialogue
 
     void Journal::setJournalIndex(const ESM::RefId& id, int index)
     {
-        Quest& quest = getQuest(id);
+        Quest& quest = getOrStartQuest(id);
 
         quest.setIndex(index);
     }
@@ -145,10 +142,6 @@ namespace MWDialogue
             mTopics.erase(mTopics.find(topicId)); // All responses removed -> remove topic
     }
 
-    int Journal::getQuestCount() const
-    {
-        return static_cast<int>(mQuests.size());
-    }
     int Journal::getJournalIndex(const ESM::RefId& id) const
     {
         TQuestContainer::const_iterator iter = mQuests.find(id);
@@ -267,7 +260,7 @@ namespace MWDialogue
                 {
                     case ESM::JournalEntry::Type_Quest:
 
-                        getQuest(record.mTopic).insertEntry(record);
+                        getOrStartQuest(record.mTopic).insertEntry(record);
                         break;
 
                     case ESM::JournalEntry::Type_Journal:

apps/openmw/mwdialogue/journalimp.hpp

@@ -24,19 +24,16 @@ namespace MWDialogue
 
         void clear() override;
 
-        Quest* getQuestPtr(const ESM::RefId& id) override;
+        Quest* getQuestOrNull(const ESM::RefId& id) override;
         ///< Gets a pointer to the requested quest. Will return nullptr if the quest has not been started.
 
-        Quest& getQuest(const ESM::RefId& id) override;
+        Quest& getOrStartQuest(const ESM::RefId& id) override;
-        ///< Gets the quest requested. Attempts to create it and inserts it in quests if it does not yet exist.
+        ///< Gets the quest requested. Attempts to create it and inserts it in quests if it is not yet started.
 
         void addEntry(const ESM::RefId& id, int index, const MWWorld::Ptr& actor) override;
         ///< Add a journal entry.
         /// @param actor Used as context for replacing of escape sequences (%name, etc).
 
-        int getQuestCount() const override;
-        ///< Get the count of saved quests.
-
         void setJournalIndex(const ESM::RefId& id, int index) override;
         ///< Set the journal index without adding an entry.
 

apps/openmw/mwlua/types/player.cpp

@@ -11,13 +11,6 @@ namespace MWLua
     struct Quests
     {
         bool mMutable = false;
-        MWWorld::SafePtr::Id playerId;
-        using Iterator = typename MWBase::Journal::TQuestIter;
-        Iterator mIterator;
-        MWBase::Journal* const journal = MWBase::Environment::get().getJournal();
-        void reset() { mIterator = journal->questBegin(); }
-        bool isEnd() const { return mIterator == journal->questEnd(); }
-        void advance() { mIterator++; }
     };
     struct Quest
     {
@@ -45,96 +38,84 @@ namespace MWLua
     {
         MWBase::Journal* const journal = MWBase::Environment::get().getJournal();
 
-        // Quests
         player["quests"] = [](const Object& player) {
-            MWBase::World* world = MWBase::Environment::get().getWorld();
+            if (player.ptr() != MWBase::Environment::get().getWorld()->getPlayerPtr())
-            Quests q = {};
+                throw std::runtime_error("The argument must be a player!");
-            if (player.ptr() != world->getPlayerPtr())
+            bool allowChanges = dynamic_cast<const GObject*>(&player) != nullptr
-                throw std::runtime_error("Must provide a player!");
+                || dynamic_cast<const SelfObject*>(&player) != nullptr;
-            if (dynamic_cast<const GObject*>(&player))
+            return Quests{ .mMutable = allowChanges };
-                q.mMutable = true;
-            q.playerId = player.id();
-            return q;
         };
         sol::usertype<Quests> quests = context.mLua->sol().new_usertype<Quests>("Quests");
-        quests[sol::meta_function::to_string]
+        quests[sol::meta_function::to_string] = [](const Quests& quests) { return "Quests"; };
-            = [](const Quests& quests) { return "Quests[" + quests.playerId.toString() + "]"; };
+        quests[sol::meta_function::index] = sol::overload([](const Quests& quests, std::string_view questId) -> Quest {
-        quests[sol::meta_function::length] = [journal]() { return journal->getQuestCount(); };
+            ESM::RefId quest = ESM::RefId::deserializeText(questId);
-        quests[sol::meta_function::index] = sol::overload([](const Quests& quests, std::string_view index) -> Quest {
+            const ESM::Dialogue* dial = MWBase::Environment::get().getESMStore()->get<ESM::Dialogue>().find(quest);
-            Quest q;
+            if (dial->mType != ESM::Dialogue::Journal)
-            q.mQuestId = ESM::RefId::deserializeText(index);
+                throw std::runtime_error("Not a quest:" + std::string(questId));
-            q.mMutable = quests.mMutable;
+            return Quest{ .mQuestId = quest, .mMutable = quests.mMutable };
-            return q;
         });
-        quests[sol::meta_function::pairs] = [](sol::this_state ts, Quests& self) {
+        quests[sol::meta_function::pairs] = [journal](const Quests& quests) {
-            sol::state_view lua(ts);
+            std::vector<ESM::RefId> ids;
-            self.reset();
+            for (auto it = journal->questBegin(); it != journal->questEnd(); ++it)
-            return sol::as_function([lua, &self]() mutable -> std::pair<sol::object, sol::object> {
+                ids.push_back(it->first);
-                if (!self.isEnd())
+            size_t i = 0;
-                {
+            return [ids = std::move(ids), i,
-                    Quest q;
+                       allowChanges = quests.mMutable]() mutable -> sol::optional<std::tuple<std::string, Quest>> {
-                    q.mQuestId = (self.mIterator->first);
+                if (i >= ids.size())
-                    q.mMutable = self.mMutable;
+                    return sol::nullopt;
-                    auto result = sol::make_object(lua, q);
+                const ESM::RefId& id = ids[i++];
-                    auto index = sol::make_object(lua, self.mIterator->first);
+                return std::make_tuple(id.serializeText(), Quest{ .mQuestId = id, .mMutable = allowChanges });
-                    self.advance();
+            };
-                    return { index, result };
-                }
-                else
-                {
-                    return { sol::lua_nil, sol::lua_nil };
-                }
-            });
         };
 
-        // Quest Functions
+        sol::usertype<Quest> quest = context.mLua->sol().new_usertype<Quest>("Quest");
+        quest[sol::meta_function::to_string]
+            = [](const Quest& quest) { return "Quest[" + quest.mQuestId.serializeText() + "]"; };
+
         auto getQuestStage = [journal](const Quest& q) -> int {
-            auto quest = journal->getQuestPtr(q.mQuestId);
+            const MWDialogue::Quest* quest = journal->getQuestOrNull(q.mQuestId);
             if (quest == nullptr)
-                return -1;
+                return 0;
             return journal->getJournalIndex(q.mQuestId);
         };
         auto setQuestStage = [context](const Quest& q, int stage) {
             if (!q.mMutable)
-                throw std::runtime_error("Value can only be changed in global scripts!");
+                throw std::runtime_error("Value can only be changed in global or player scripts!");
             context.mLuaManager->addAction(
                 [q, stage] { MWBase::Environment::get().getJournal()->setJournalIndex(q.mQuestId, stage); },
                 "setQuestStageAction");
         };
-
-        // Player quests
-        sol::usertype<Quest> quest = context.mLua->sol().new_usertype<Quest>("Quest");
-        quest[sol::meta_function::to_string]
-            = [](const Quest& quest) { return "Quest [" + quest.mQuestId.serializeText() + "]"; };
         quest["stage"] = sol::property(getQuestStage, setQuestStage);
-        quest["name"] = sol::readonly_property([journal](const Quest& q) -> sol::optional<std::string_view> {
+
-            auto quest = journal->getQuestPtr(q.mQuestId);
-            if (quest == nullptr)
-                return sol::nullopt;
-            return quest->getName();
-        });
         quest["id"] = sol::readonly_property([](const Quest& q) -> std::string { return q.mQuestId.serializeText(); });
-        quest["isFinished"] = sol::property(
+        quest["started"] = sol::readonly_property(
+            [journal](const Quest& q) { return journal->getQuestOrNull(q.mQuestId) != nullptr; });
+        quest["finished"] = sol::property(
             [journal](const Quest& q) -> bool {
-                auto quest = journal->getQuestPtr(q.mQuestId);
+                const MWDialogue::Quest* quest = journal->getQuestOrNull(q.mQuestId);
                 if (quest == nullptr)
                     return false;
                 return quest->isFinished();
             },
             [journal, context](const Quest& q, bool finished) {
                 if (!q.mMutable)
-                    throw std::runtime_error("Value can only be changed in global scripts!");
+                    throw std::runtime_error("Value can only be changed in global or player scripts!");
                 context.mLuaManager->addAction(
-                    [q, finished, journal] { journal->getQuest(q.mQuestId).setFinished(finished); },
+                    [q, finished, journal] { journal->getOrStartQuest(q.mQuestId).setFinished(finished); },
                     "setQuestFinishedAction");
             });
-        quest["addJournalEntry"] = [context](const Quest& q, const GObject& actor, int stage) {
+        quest["addJournalEntry"] = [context](const Quest& q, int stage, sol::optional<GObject> actor) {
-            MWWorld::Ptr ptr = actor.ptr();
+            if (!q.mMutable)
-
+                throw std::runtime_error("Can only be used in global or player scripts!");
             // The journal mwscript function has a try function here, we will make the lua function throw an
             // error. However, the addAction will cause it to error outside of this function.
             context.mLuaManager->addAction(
-                [ptr, q, stage] { MWBase::Environment::get().getJournal()->addEntry(q.mQuestId, stage, ptr); },
+                [actor, q, stage] {
+                    MWWorld::Ptr actorPtr;
+                    if (actor)
+                        actorPtr = actor->ptr();
+                    MWBase::Environment::get().getJournal()->addEntry(q.mQuestId, stage, actorPtr);
+                },
                 "addJournalEntryAction");
         };
     }
@@ -147,4 +128,4 @@ namespace MWLua
         };
         addPlayerQuestBindings(player, context);
     }
-}
+}

docs/source/reference/lua-scripting/engine_handlers.rst

@@ -89,7 +89,7 @@ Engine handler is a function defined by a script, that can be called by the engi
     - | `Key <openmw_input.html##(KeyboardEvent)>`_ is pressed.
       | Usage example:
       | ``if key.symbol == 'z' and key.withShift then ...``
-  * - onQuestUpdate(questId,stage)
+  * - onQuestUpdate(questId, stage)
     - | Called when a quest is updated.
   * - onKeyRelease(key)
     - | `Key <openmw_input.html##(KeyboardEvent)>`_ is released.

files/lua_api/openmw/types.lua

@@ -697,7 +697,9 @@
 -- @field #string head Path to the head body part model
 -- @field #bool isMale The gender setting of the NPC
 
---- @{#Player} functions
+
+--------------------------------------------------------------------------------
+-- @{#Player} functions
 -- @field [parent=#types] #Player Player
 
 ---
@@ -714,38 +716,38 @@
 ---
 -- Returns the bounty or crime level of the player
 -- @function [parent=#Player] getCrimeLevel
--- @param openmw.core#GameObject actor
+-- @param openmw.core#GameObject player
 -- @return #number
 
 ---
--- Returns a list containing quests @{<#PlayerQuest>}  for the specified player, indexed by quest ID.
+-- Returns a list containing quests @{#PlayerQuest} for the specified player, indexed by quest ID.
 -- @function [parent=#Player] quests
 -- @param openmw.core#GameObject player
 -- @return #list<#PlayerQuest>
--- @usage -- Getting the quest for a specified index
+-- @usage -- Get stage of a specific quest
--- stage = types.Player.quests(playerRef)["ms_fargothring].stage
+-- stage = types.Player.quests(player)["ms_fargothring"].stage
--- --Get the name of all started quests
+-- @usage -- Get names of all started quests
--- for x, quest in pairs(types.Player.quests(playerRef)) do print (quest.name) end
+-- for x, quest in pairs(types.Player.quests(player)) do print (quest.name) end
--- --Start a new quest, add it to the player's quest list but don't add any journal entries
+-- @usage -- Start a new quest, add it to the player's quest list but don't add any journal entries
--- types.Player.quests(playerRef)["ms_fargothring].stage = 0
+-- types.Player.quests(player)["ms_fargothring"].stage = 0
-
---- @{#PlayerQuest}
 
 ---
 -- @type PlayerQuest
--- @field #string id The quest ID.
+-- @field #string id The quest id.
--- @field #number stage The quest Stage. May only be changed by global scripts. Returns -1 if the quest has not been started or does not exist.
+-- @field #number stage The quest stage (global and player scripts can change it). Changing the stage starts the quest if it wasn't started.
--- @field #bool isFinished Returns true if the quest is complete, false if not.
+-- @field #bool started Whether the quest is started.
--- @field #string name The Quest's user friendly name. Not all quests have this. Will be nil if the quest has not been started.
+-- @field #bool finished Whether the quest is finished (global and player scripts can change it).
 
 ---
--- Sets the quest stage for the given quest, on the given player, and adds the entry to the journal, if there is an entry at the specified stage. Can only be used in global scripts.
+-- Sets the quest stage for the given quest, on the given player, and adds the entry to the journal, if there is an entry at the specified stage. Can only be used in global or player scripts.
 -- @function [parent=#PlayerQuest] addJournalEntry
 -- @param self
--- @param openmw.core#GameObject actor The actor who is the source of the journal entry, can be the same as player, their name is used in a similar manner as in dialogue.
+-- @param #number stage Quest stage
--- @param #number stage Quest Stage
+-- @param openmw.core#GameObject actor (optional) The actor who is the source of the journal entry, it may be used in journal entries with variables such as `%name(The speaker's name)` or `%race(The speaker's race)`.
 
---- @{#Armor} functions
+
+--------------------------------------------------------------------------------
+-- @{#Armor} functions
 -- @field [parent=#types] #Armor Armor
 
 ---