Refactor dialogue responses to make sure messages from scripts are printer afterwards, not before the dialogue response (Fixes #4166)

Don't delete Link objects prematurely (Fixes #4171)

apps/openmw/mwbase/dialoguemanager.hpp

@@ -36,6 +36,12 @@ namespace MWBase
 
         public:
 
+            class ResponseCallback
+            {
+            public:
+                virtual void addResponse(const std::string& title, const std::string& text) = 0;
+            };
+
             DialogueManager() {}
 
             virtual void clear() = 0;
@@ -44,8 +50,7 @@ namespace MWBase
 
             virtual bool isInChoice() const = 0;
 
-            typedef std::pair<std::string, std::string> Response; // title, text
+            virtual bool startDialogue (const MWWorld::Ptr& actor, ResponseCallback* callback) = 0;
-            virtual bool startDialogue (const MWWorld::Ptr& actor, Response& response) = 0;
 
             virtual void addTopic (const std::string& topic) = 0;
 
@@ -58,15 +63,15 @@ namespace MWBase
 
             virtual void say(const MWWorld::Ptr &actor, const std::string &topic) = 0;
 
-            virtual Response keywordSelected (const std::string& keyword) = 0;
+            virtual void keywordSelected (const std::string& keyword, ResponseCallback* callback) = 0;
             virtual void goodbyeSelected() = 0;
-            virtual Response questionAnswered (int answer) = 0;
+            virtual void questionAnswered (int answer, ResponseCallback* callback) = 0;
 
             virtual std::list<std::string> getAvailableTopics() = 0;
 
-            virtual bool checkServiceRefused (Response& response) = 0;
+            virtual bool checkServiceRefused (ResponseCallback* callback) = 0;
 
-            virtual Response persuade (int type) = 0;
+            virtual void persuade (int type, ResponseCallback* callback) = 0;
             virtual int getTemporaryDispositionChange () const = 0;
 
             /// @note This change is temporary and gets discarded when dialogue ends.

apps/openmw/mwdialogue/dialoguemanagerimp.cpp

@@ -101,7 +101,7 @@ namespace MWDialogue
         }
     }
 
-    bool DialogueManager::startDialogue (const MWWorld::Ptr& actor, Response& response)
+    bool DialogueManager::startDialogue (const MWWorld::Ptr& actor, ResponseCallback* callback)
     {
         updateGlobals();
 
@@ -151,7 +151,7 @@ namespace MWDialogue
                     parseText (info->mResponse);
 
                     MWScript::InterpreterContext interpreterContext(&mActor.getRefData().getLocals(),mActor);
-                    response = Response ("", Interpreter::fixDefinesDialog(info->mResponse, interpreterContext));
+                    callback->addResponse("", Interpreter::fixDefinesDialog(info->mResponse, interpreterContext));
                     executeScript (info->mResultScript, mActor);
                     mLastTopic = it->mId;
 
@@ -240,9 +240,8 @@ namespace MWDialogue
         }
     }
 
-    DialogueManager::Response DialogueManager::executeTopic (const std::string& topic)
+    void DialogueManager::executeTopic (const std::string& topic, ResponseCallback* callback)
     {
-        DialogueManager::Response response;
         Filter filter (mActor, mChoice, mTalkedTo);
 
         const MWWorld::Store<ESM::Dialogue> &dialogues =
@@ -274,7 +273,7 @@ namespace MWDialogue
                 title = topic;
 
             MWScript::InterpreterContext interpreterContext(&mActor.getRefData().getLocals(),mActor);
-            response = Response(title, Interpreter::fixDefinesDialog(info->mResponse, interpreterContext));
+            callback->addResponse(title, Interpreter::fixDefinesDialog(info->mResponse, interpreterContext));
 
             if (dialogue.mType == ESM::Dialogue::Topic)
             {
@@ -295,7 +294,6 @@ namespace MWDialogue
 
             mLastTopic = topic;
         }
-        return response;
     }
 
     const ESM::Dialogue *DialogueManager::searchDialogue(const std::string& id)
@@ -350,18 +348,16 @@ namespace MWDialogue
         return keywordList;
     }
 
-    DialogueManager::Response DialogueManager::keywordSelected (const std::string& keyword)
+    void DialogueManager::keywordSelected (const std::string& keyword, ResponseCallback* callback)
     {
-        Response response;
         if(!mIsInChoice)
         {
             const ESM::Dialogue* dialogue = searchDialogue(keyword);
             if (dialogue && dialogue->mType == ESM::Dialogue::Topic)
             {
-                response = executeTopic (keyword);
+                executeTopic (keyword, callback);
             }
         }
-        return response;
     }
 
     bool DialogueManager::isInChoice() const
@@ -386,10 +382,9 @@ namespace MWDialogue
         mTemporaryDispositionChange = 0;
     }
 
-    DialogueManager::Response DialogueManager::questionAnswered (int answer)
+    void DialogueManager::questionAnswered (int answer, ResponseCallback* callback)
     {
         mChoice = answer;
-        DialogueManager::Response response;
 
         const ESM::Dialogue* dialogue = searchDialogue(mLastTopic);
         if (dialogue)
@@ -408,7 +403,7 @@ namespace MWDialogue
                     mChoices.clear();
 
                     MWScript::InterpreterContext interpreterContext(&mActor.getRefData().getLocals(),mActor);
-                    response = Response("", Interpreter::fixDefinesDialog(text, interpreterContext));
+                    callback->addResponse("", Interpreter::fixDefinesDialog(text, interpreterContext));
 
                     // Make sure the returned DialInfo is from the Dialogue we supplied. If could also be from the Info refusal group,
                     // in which case it should not be added to the journal.
@@ -434,7 +429,6 @@ namespace MWDialogue
         }
 
         updateActorKnownTopics();
-        return response;
     }
 
     void DialogueManager::addChoice (const std::string& text, int choice)
@@ -460,7 +454,7 @@ namespace MWDialogue
         mGoodbye = true;
     }
 
-    DialogueManager::Response DialogueManager::persuade(int type)
+    void DialogueManager::persuade(int type, ResponseCallback* callback)
     {
         bool success;
         float temp, perm;
@@ -509,7 +503,7 @@ namespace MWDialogue
             text = "Bribe";
         }
 
-        return executeTopic (text + (success ? " Success" : " Fail"));
+        executeTopic (text + (success ? " Success" : " Fail"), callback);
     }
 
     int DialogueManager::getTemporaryDispositionChange() const
@@ -522,7 +516,7 @@ namespace MWDialogue
         mTemporaryDispositionChange += delta;
     }
 
-    bool DialogueManager::checkServiceRefused(Response& response)
+    bool DialogueManager::checkServiceRefused(ResponseCallback* callback)
     {
         Filter filter (mActor, mChoice, mTalkedTo);
 
@@ -543,7 +537,7 @@ namespace MWDialogue
 
             MWScript::InterpreterContext interpreterContext(&mActor.getRefData().getLocals(),mActor);
 
-            response = Response(gmsts.find ("sServiceRefusal")->getString(), Interpreter::fixDefinesDialog(info->mResponse, interpreterContext));
+            callback->addResponse(gmsts.find ("sServiceRefusal")->getString(), Interpreter::fixDefinesDialog(info->mResponse, interpreterContext));
 
             executeScript (info->mResultScript, mActor);
             return true;

apps/openmw/mwdialogue/dialoguemanagerimp.hpp

@@ -57,7 +57,7 @@ namespace MWDialogue
             bool compile (const std::string& cmd, std::vector<Interpreter::Type_Code>& code, const MWWorld::Ptr& actor);
             void executeScript (const std::string& script, const MWWorld::Ptr& actor);
 
-            Response executeTopic (const std::string& topic);
+            void executeTopic (const std::string& topic, ResponseCallback* callback);
 
             const ESM::Dialogue* searchDialogue(const std::string& id);
 
@@ -69,7 +69,7 @@ namespace MWDialogue
 
             virtual bool isInChoice() const;
 
-            virtual bool startDialogue (const MWWorld::Ptr& actor, Response& response);
+            virtual bool startDialogue (const MWWorld::Ptr& actor, ResponseCallback* callback);
 
             std::list<std::string> getAvailableTopics();
 
@@ -82,16 +82,16 @@ namespace MWDialogue
 
             virtual void goodbye();
 
-            virtual bool checkServiceRefused (Response& response);
+            virtual bool checkServiceRefused (ResponseCallback* callback);
 
             virtual void say(const MWWorld::Ptr &actor, const std::string &topic);
 
             //calbacks for the GUI
-            virtual Response keywordSelected (const std::string& keyword);
+            virtual void keywordSelected (const std::string& keyword, ResponseCallback* callback);
             virtual void goodbyeSelected();
-            virtual Response questionAnswered (int answer);
+            virtual void questionAnswered (int answer, ResponseCallback* callback);
 
-            virtual Response persuade (int type);
+            virtual void persuade (int type, ResponseCallback* callback);
             virtual int getTemporaryDispositionChange () const;
 
             /// @note This change is temporary and gets discarded when dialogue ends.

apps/openmw/mwgui/dialogue.cpp

@@ -30,8 +30,29 @@
 namespace MWGui
 {
 
-    PersuasionDialog::PersuasionDialog()
+    class ResponseCallback : public MWBase::DialogueManager::ResponseCallback
+    {
+    public:
+        ResponseCallback(DialogueWindow* win, bool needMargin=true)
+            : mWindow(win)
+            , mNeedMargin(needMargin)
+        {
+
+        }
+
+        void addResponse(const std::string& title, const std::string& text)
+        {
+            mWindow->addResponse(title, text, mNeedMargin);
+        }
+
+    private:
+        DialogueWindow* mWindow;
+        bool mNeedMargin;
+    };
+
+    PersuasionDialog::PersuasionDialog(ResponseCallback* callback)
         : WindowModal("openmw_persuasion_dialog.layout")
+        , mCallback(callback)
     {
         getWidget(mCancelButton, "CancelButton");
         getWidget(mAdmireButton, "AdmireButton");
@@ -69,9 +90,7 @@ namespace MWGui
         else /*if (sender == mBribe1000Button)*/
             type = MWBase::MechanicsManager::PT_Bribe1000;
 
-        MWBase::DialogueManager::Response response = MWBase::Environment::get().getDialogueManager()->persuade(type);
+        MWBase::Environment::get().getDialogueManager()->persuade(type, mCallback.get());
-
-        eventPersuadeMsg(response.first, response.second);
 
         setVisible(false);
     }
@@ -244,13 +263,14 @@ namespace MWGui
         : WindowBase("openmw_dialogue_window.layout")
         , mIsCompanion(false)
         , mGoodbye(false)
-        , mPersuasionDialog()
+        , mPersuasionDialog(new ResponseCallback(this))
+        , mCallback(new ResponseCallback(this))
+        , mGreetingCallback(new ResponseCallback(this, false))
     {
         // Centre dialog
         center();
 
         mPersuasionDialog.setVisible(false);
-        mPersuasionDialog.eventPersuadeMsg += MyGUI::newDelegate(this, &DialogueWindow::onPersuadeResult);
 
         //History view
         getWidget(mHistory, "History");
@@ -277,8 +297,6 @@ namespace MWGui
 
     DialogueWindow::~DialogueWindow()
     {
-        mPersuasionDialog.eventPersuadeMsg.clear();
-
         deleteLater();
         for (Link* link : mLinks)
             delete link;
@@ -356,12 +374,11 @@ namespace MWGui
             const MWWorld::Store<ESM::GameSetting> &gmst =
                 MWBase::Environment::get().getWorld()->getStore().get<ESM::GameSetting>();
 
-            MWBase::DialogueManager::Response response;
             if (topic == gmst.find("sPersuasion")->getString())
                 mPersuasionDialog.setVisible(true);
             else if (topic == gmst.find("sCompanionShare")->getString())
                 MWBase::Environment::get().getWindowManager()->pushGuiMode(GM_Companion, mPtr);
-            else if (!MWBase::Environment::get().getDialogueManager()->checkServiceRefused(response))
+            else if (!MWBase::Environment::get().getDialogueManager()->checkServiceRefused(mCallback.get()))
             {
                 if (topic == gmst.find("sBarter")->getString())
                     MWBase::Environment::get().getWindowManager()->pushGuiMode(GM_Barter, mPtr);
@@ -378,19 +395,34 @@ namespace MWGui
                 else if (topic == gmst.find("sRepair")->getString())
                     MWBase::Environment::get().getWindowManager()->pushGuiMode(GM_MerchantRepair, mPtr);
             }
-            else
-                addResponse(response.first, response.second);
         }
     }
 
     void DialogueWindow::setPtr(const MWWorld::Ptr& actor)
     {
-        MWBase::DialogueManager::Response response;
+        bool sameActor = (mPtr == actor);
-        if (!MWBase::Environment::get().getDialogueManager()->startDialogue(actor, response))
+        if (!sameActor)
+        {
+            for (std::vector<DialogueText*>::iterator it = mHistoryContents.begin(); it != mHistoryContents.end(); ++it)
+                delete (*it);
+            mHistoryContents.clear();
+            mKeywords.clear();
+            mTopicsList->clear();
+            for (std::vector<Link*>::iterator it = mLinks.begin(); it != mLinks.end(); ++it)
+                mDeleteLater.push_back(*it); // Links are not deleted right away to prevent issues with event handlers
+            mLinks.clear();
+        }
+
+        mPtr = actor;
+        mGoodbye = false;
+        mTopicsList->setEnabled(true);
+
+        if (!MWBase::Environment::get().getDialogueManager()->startDialogue(actor, mGreetingCallback.get()))
         {
             // No greetings found. The dialogue window should not be shown.
             // If this is a companion, we must show the companion window directly (used by BM_bear_be_unique).
             MWBase::Environment::get().getWindowManager()->removeGuiMode(MWGui::GM_Dialogue);
+            mPtr = MWWorld::Ptr();
             if (isCompanion(actor))
                 MWBase::Environment::get().getWindowManager()->pushGuiMode(MWGui::GM_Companion, actor);
             return;
@@ -398,32 +430,11 @@ namespace MWGui
 
         MWBase::Environment::get().getWindowManager()->setKeyFocusWidget(mGoodbyeButton);
 
-        mGoodbye = false;
-        bool sameActor = (mPtr == actor);
-        mPtr = actor;
-        mTopicsList->setEnabled(true);
         setTitle(mPtr.getClass().getName(mPtr));
 
-        mTopicsList->clear();
+        updateTopicsPane();
-
-        if (!sameActor)
-        {
-            for (std::vector<DialogueText*>::iterator it = mHistoryContents.begin(); it != mHistoryContents.end(); ++it)
-                delete (*it);
-            mHistoryContents.clear();
-
-            mKeywords.clear();
-            updateTopicsPane();
-        }
-
-        for (std::vector<Link*>::iterator it = mLinks.begin(); it != mLinks.end(); ++it)
-            mDeleteLater.push_back(*it); // Links are not deleted right away to prevent issues with event handlers
-        mLinks.clear();
-
         updateDisposition();
         restock();
-
-        addResponse(response.first, response.second, false);
     }
 
     void DialogueWindow::restock()
@@ -608,14 +619,12 @@ namespace MWGui
 
     void DialogueWindow::onTopicActivated(const std::string &topicId)
     {
-        MWBase::DialogueManager::Response response = MWBase::Environment::get().getDialogueManager()->keywordSelected(topicId);
+        MWBase::Environment::get().getDialogueManager()->keywordSelected(topicId, mCallback.get());
-        addResponse(response.first, response.second);
     }
 
     void DialogueWindow::onChoiceActivated(int id)
     {
-        MWBase::DialogueManager::Response response = MWBase::Environment::get().getDialogueManager()->questionAnswered(id);
+        MWBase::Environment::get().getDialogueManager()->questionAnswered(id, mCallback.get());
-        addResponse(response.first, response.second);
     }
 
     void DialogueWindow::onGoodbyeActivated()
@@ -707,8 +716,4 @@ namespace MWGui
                 && actor.getRefData().getLocals().getIntVar(actor.getClass().getScript(actor), "companion");
     }
 
-    void DialogueWindow::onPersuadeResult(const std::string &title, const std::string &text)
-    {
-        addResponse(title, text);
-    }
 }

apps/openmw/mwgui/dialogue.hpp

@@ -22,19 +22,20 @@ namespace MWGui
 
 namespace MWGui
 {
+    class ResponseCallback;
+
     class PersuasionDialog : public WindowModal
     {
     public:
-        PersuasionDialog();
+        PersuasionDialog(ResponseCallback* callback);
-
-        typedef MyGUI::delegates::CMultiDelegate2<const std::string&, const std::string&> EventHandle_Result;
-        EventHandle_Result eventPersuadeMsg;
 
         virtual void onOpen();
 
         virtual MyGUI::Widget* getDefaultKeyFocus();
 
     private:
+        std::unique_ptr<ResponseCallback> mCallback;
+
         MyGUI::Button* mCancelButton;
         MyGUI::Button* mAdmireButton;
         MyGUI::Button* mIntimidateButton;
@@ -136,7 +137,6 @@ namespace MWGui
         bool isCompanion(const MWWorld::Ptr& actor);
         bool isCompanion();
 
-        void onPersuadeResult(const std::string& title, const std::string& text);
         void onSelectListItem(const std::string& topic, int id);
         void onByeClicked(MyGUI::Widget* _sender);
         void onMouseWheel(MyGUI::Widget* _sender, int _rel);
@@ -180,6 +180,9 @@ namespace MWGui
         PersuasionDialog mPersuasionDialog;
 
         MyGUI::IntSize mCurrentWindowSize;
+
+        std::unique_ptr<ResponseCallback> mCallback;
+        std::unique_ptr<ResponseCallback> mGreetingCallback;
     };
 }
 #endif