From 15eecec1967f0ed0f025ff7f9c6b78e061159bdb Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Tue, 19 Aug 2025 20:44:43 +0200 Subject: [PATCH] Remove journal iterator methods --- apps/openmw/mwbase/journal.hpp | 27 +--- apps/openmw/mwdialogue/dialoguemanagerimp.cpp | 21 +-- apps/openmw/mwdialogue/journalimp.cpp | 73 +++------- apps/openmw/mwdialogue/journalimp.hpp | 24 +--- apps/openmw/mwgui/journalviewmodel.cpp | 129 ++++++++---------- apps/openmw/mwgui/journalviewmodel.hpp | 2 +- apps/openmw/mwgui/journalwindow.cpp | 8 +- apps/openmw/mwlua/types/player.cpp | 12 +- 8 files changed, 97 insertions(+), 199 deletions(-) diff --git a/apps/openmw/mwbase/journal.hpp b/apps/openmw/mwbase/journal.hpp index 15b693892a..50be0959bf 100644 --- a/apps/openmw/mwbase/journal.hpp +++ b/apps/openmw/mwbase/journal.hpp @@ -36,11 +36,8 @@ namespace MWBase public: typedef std::deque TEntryContainer; - typedef TEntryContainer::const_iterator TEntryIter; typedef std::map TQuestContainer; // topic, quest - typedef TQuestContainer::const_iterator TQuestIter; typedef std::map TTopicContainer; // topic-id, topic-content - typedef TTopicContainer::const_iterator TTopicIter; public: Journal() {} @@ -71,32 +68,12 @@ namespace MWBase ///< Removes the last topic response added for the given topicId and actor name. /// \note topicId must be lowercase - virtual TEntryIter begin() const = 0; - ///< Iterator pointing to the begin of the main journal. - /// - /// \note Iterators to main journal entries will never become invalid. - - virtual TEntryIter end() const = 0; - ///< Iterator pointing past the end of the main journal. - virtual const TEntryContainer& getEntries() const = 0; - virtual TQuestIter questBegin() const = 0; - ///< Iterator pointing to the first quest (sorted by topic ID) - - virtual TQuestIter questEnd() const = 0; - ///< Iterator pointing past the last quest. - - virtual TTopicIter topicBegin() const = 0; - ///< Iterator pointing to the first topic (sorted by topic ID) - /// - /// \note The topic ID is identical with the user-visible topic string. - - virtual TTopicIter topicEnd() const = 0; - ///< Iterator pointing past the last topic. - virtual const TTopicContainer& getTopics() const = 0; + virtual const TQuestContainer& getQuests() const = 0; + virtual int countSavedGameRecords() const = 0; virtual void write(ESM::ESMWriter& writer, Loading::Listener& progress) const = 0; diff --git a/apps/openmw/mwdialogue/dialoguemanagerimp.cpp b/apps/openmw/mwdialogue/dialoguemanagerimp.cpp index dc3887435e..f57519f29d 100644 --- a/apps/openmw/mwdialogue/dialoguemanagerimp.cpp +++ b/apps/openmw/mwdialogue/dialoguemanagerimp.cpp @@ -264,24 +264,13 @@ namespace MWDialogue bool DialogueManager::inJournal(const ESM::RefId& topicId, const ESM::RefId& infoId) const { - const MWDialogue::Topic* topicHistory = nullptr; MWBase::Journal* journal = MWBase::Environment::get().getJournal(); - for (auto it = journal->topicBegin(); it != journal->topicEnd(); ++it) + const auto topic = journal->getTopics().find(topicId); + if (topic != journal->getTopics().end()) { - if (it->first == topicId) - { - topicHistory = &it->second; - break; - } - } - - if (!topicHistory) - return false; - - for (const auto& topic : *topicHistory) - { - if (topic.mInfoId == infoId) - return true; + return std::ranges::find_if(topic->second, [&](const MWDialogue::Entry& entry) { + return entry.mInfoId == infoId; + }) != topic->second.end(); } return false; } diff --git a/apps/openmw/mwdialogue/journalimp.cpp b/apps/openmw/mwdialogue/journalimp.cpp index 04574e92c5..c607c5aa25 100644 --- a/apps/openmw/mwdialogue/journalimp.cpp +++ b/apps/openmw/mwdialogue/journalimp.cpp @@ -61,9 +61,8 @@ namespace MWDialogue if (infoId.empty()) return true; - for (ESM::Dialogue::InfoContainer::const_iterator iter(dialogue->mInfo.begin()); - iter != dialogue->mInfo.end(); ++iter) - if (iter->mId == infoId) + for (const ESM::DialInfo& info : dialogue->mInfo) + if (info.mId == infoId) return true; } @@ -83,8 +82,8 @@ namespace MWDialogue { // bail out if we already have heard this... const ESM::RefId& infoId = JournalEntry::idFromIndex(id, index); - for (TEntryIter i = mJournal.begin(); i != mJournal.end(); ++i) - if (i->mTopic == id && i->mInfoId == infoId) + for (const JournalEntry& entry : mJournal) + if (entry.mTopic == id && entry.mInfoId == infoId) { if (getJournalIndex(id) < index) { @@ -152,95 +151,61 @@ namespace MWDialogue return iter->second.getIndex(); } - Journal::TEntryIter Journal::begin() const - { - return mJournal.begin(); - } - - Journal::TEntryIter Journal::end() const - { - return mJournal.end(); - } - - Journal::TQuestIter Journal::questBegin() const - { - return mQuests.begin(); - } - - Journal::TQuestIter Journal::questEnd() const - { - return mQuests.end(); - } - - Journal::TTopicIter Journal::topicBegin() const - { - return mTopics.begin(); - } - - Journal::TTopicIter Journal::topicEnd() const - { - return mTopics.end(); - } - int Journal::countSavedGameRecords() const { - int count = static_cast(mQuests.size()); + std::size_t count = mQuests.size(); - for (TQuestIter iter(mQuests.begin()); iter != mQuests.end(); ++iter) - count += std::distance(iter->second.begin(), iter->second.end()); + for (const auto& [_, quest] : mQuests) + count += quest.size(); - count += std::distance(mJournal.begin(), mJournal.end()); + count += mJournal.size(); - for (TTopicIter iter(mTopics.begin()); iter != mTopics.end(); ++iter) - count += std::distance(iter->second.begin(), iter->second.end()); + for (const auto& [_, topic] : mTopics) + count += topic.size(); - return count; + return static_cast(count); } void Journal::write(ESM::ESMWriter& writer, Loading::Listener& progress) const { - for (TQuestIter iter(mQuests.begin()); iter != mQuests.end(); ++iter) + for (const auto& [_, quest] : mQuests) { - const Quest& quest = iter->second; - ESM::QuestState state; quest.write(state); writer.startRecord(ESM::REC_QUES); state.save(writer); writer.endRecord(ESM::REC_QUES); - for (Topic::TEntryIter entryIter(quest.begin()); entryIter != quest.end(); ++entryIter) + for (const Entry& questEntry : quest) { ESM::JournalEntry entry; entry.mType = ESM::JournalEntry::Type_Quest; entry.mTopic = quest.getTopic(); - entryIter->write(entry); + questEntry.write(entry); writer.startRecord(ESM::REC_JOUR); entry.save(writer); writer.endRecord(ESM::REC_JOUR); } } - for (TEntryIter iter(mJournal.begin()); iter != mJournal.end(); ++iter) + for (const StampedJournalEntry& journalEntry : mJournal) { ESM::JournalEntry entry; entry.mType = ESM::JournalEntry::Type_Journal; - iter->write(entry); + journalEntry.write(entry); writer.startRecord(ESM::REC_JOUR); entry.save(writer); writer.endRecord(ESM::REC_JOUR); } - for (TTopicIter iter(mTopics.begin()); iter != mTopics.end(); ++iter) + for (const auto& [_, topic] : mTopics) { - const Topic& topic = iter->second; - - for (Topic::TEntryIter entryIter(topic.begin()); entryIter != topic.end(); ++entryIter) + for (const Entry& topicEntry : topic) { ESM::JournalEntry entry; entry.mType = ESM::JournalEntry::Type_Topic; entry.mTopic = topic.getTopic(); - entryIter->write(entry); + topicEntry.write(entry); writer.startRecord(ESM::REC_JOUR); entry.save(writer); writer.endRecord(ESM::REC_JOUR); diff --git a/apps/openmw/mwdialogue/journalimp.hpp b/apps/openmw/mwdialogue/journalimp.hpp index 8545ac398b..9531652222 100644 --- a/apps/openmw/mwdialogue/journalimp.hpp +++ b/apps/openmw/mwdialogue/journalimp.hpp @@ -47,32 +47,12 @@ namespace MWDialogue ///< Removes the last topic response added for the given topicId and actor name. /// \note topicId must be lowercase - TEntryIter begin() const override; - ///< Iterator pointing to the begin of the main journal. - /// - /// \note Iterators to main journal entries will never become invalid. - - TEntryIter end() const override; - ///< Iterator pointing past the end of the main journal. - const TEntryContainer& getEntries() const override { return mJournal; } - TQuestIter questBegin() const override; - ///< Iterator pointing to the first quest (sorted by topic ID) - - TQuestIter questEnd() const override; - ///< Iterator pointing past the last quest. - - TTopicIter topicBegin() const override; - ///< Iterator pointing to the first topic (sorted by topic ID) - /// - /// \note The topic ID is identical with the user-visible topic string. - - TTopicIter topicEnd() const override; - ///< Iterator pointing past the last topic. - const TTopicContainer& getTopics() const override { return mTopics; } + const TQuestContainer& getQuests() const override { return mQuests; } + int countSavedGameRecords() const override; void write(ESM::ESMWriter& writer, Loading::Listener& progress) const override; diff --git a/apps/openmw/mwgui/journalviewmodel.cpp b/apps/openmw/mwgui/journalviewmodel.cpp index 68711ac479..626d423fd4 100644 --- a/apps/openmw/mwgui/journalviewmodel.cpp +++ b/apps/openmw/mwgui/journalviewmodel.cpp @@ -29,12 +29,12 @@ namespace MWGui JournalViewModelImpl() { mKeywordSearchLoaded = false; } - virtual ~JournalViewModelImpl() {} + virtual ~JournalViewModelImpl() = default; /// \todo replace this nasty BS static Utf8Span toUtf8Span(std::string_view str) { - if (str.size() == 0) + if (str.empty()) return Utf8Span(Utf8Point(nullptr), Utf8Point(nullptr)); Utf8Point point = reinterpret_cast(str.data()); @@ -56,8 +56,8 @@ namespace MWGui { MWBase::Journal* journal = MWBase::Environment::get().getJournal(); - for (MWBase::Journal::TTopicIter i = journal->topicBegin(); i != journal->topicEnd(); ++i) - mKeywordSearch.seed(i->second.getName(), intptr_t(&i->second)); + for (const auto& [_, topic] : journal->getTopics()) + mKeywordSearch.seed(topic.getName(), intptr_t(&topic)); mKeywordSearchLoaded = true; } @@ -67,25 +67,23 @@ namespace MWGui { MWBase::Journal* journal = MWBase::Environment::get().getJournal(); - return journal->begin() == journal->end(); + return journal->getEntries().empty(); } - template + template struct BaseEntry : Interface { - typedef t_iterator iterator_t; - - iterator_t mItr; + const EntryType* mEntry; JournalViewModelImpl const* mModel; - BaseEntry(JournalViewModelImpl const* model, iterator_t itr) - : mItr(itr) + BaseEntry(JournalViewModelImpl const* model, const EntryType& entry) + : mEntry(&entry) , mModel(model) , loaded(false) { } - virtual ~BaseEntry() {} + virtual ~BaseEntry() = default; mutable bool loaded; mutable std::string utf8text; @@ -193,58 +191,48 @@ namespace MWGui } }; - void visitQuestNames(bool active_only, std::function visitor) const override + void visitQuestNames(bool activeOnly, std::function visitor) const override { MWBase::Journal* journal = MWBase::Environment::get().getJournal(); - std::set> visitedQuests; + std::set visitedQuests; // Note that for purposes of the journal GUI, quests are identified by the name, not the ID, so several // different quest IDs can end up in the same quest log. A quest log should be considered finished // when any quest ID in that log is finished. - for (MWBase::Journal::TQuestIter i = journal->questBegin(); i != journal->questEnd(); ++i) + for (const auto& [_, quest] : journal->getQuests()) { - const MWDialogue::Quest& quest = i->second; - - bool isFinished = false; - for (MWBase::Journal::TQuestIter j = journal->questBegin(); j != journal->questEnd(); ++j) - { - if (quest.getName() == j->second.getName() && j->second.isFinished()) - isFinished = true; - } - - if (active_only && isFinished) - continue; - // Unfortunately Morrowind.esm has no quest names, since the quest book was added with tribunal. // Note that even with Tribunal, some quests still don't have quest names. I'm assuming those are not // supposed to appear in the quest book. - if (!quest.getName().empty()) - { - // Don't list the same quest name twice - if (visitedQuests.find(quest.getName()) != visitedQuests.end()) - continue; + if (quest.getName().empty()) + continue; + // Don't list the same quest name twice + if (!visitedQuests.insert(quest.getName()).second) + continue; - visitor(quest.getName(), isFinished); + bool isFinished = std::ranges::find_if(journal->getQuests(), [&](const auto& pair) { + return pair.second.isFinished() + && Misc::StringUtils::ciEqual(quest.getName(), pair.second.getName()); + }) != journal->getQuests().end(); - visitedQuests.emplace(quest.getName()); - } + if (activeOnly && isFinished) + continue; + + visitor(quest.getName(), isFinished); } } - template - struct JournalEntryImpl : BaseEntry + struct JournalEntryImpl : BaseEntry { - using BaseEntry::mItr; - mutable std::string timestamp_buffer; - JournalEntryImpl(JournalViewModelImpl const* model, iterator_t itr) - : BaseEntry(model, itr) + JournalEntryImpl(JournalViewModelImpl const* model, const MWDialogue::StampedJournalEntry& entry) + : BaseEntry(model, entry) { } - std::string getText() const override { return mItr->getText(); } + std::string getText() const override { return mEntry->getText(); } Utf8Span timestamp() const override { @@ -254,9 +242,9 @@ namespace MWGui std::ostringstream os; - os << mItr->mDayOfMonth << ' ' - << MWBase::Environment::get().getWorld()->getTimeManager()->getMonthName(mItr->mMonth) << " (" - << dayStr << " " << (mItr->mDay) << ')'; + os << mEntry->mDayOfMonth << ' ' + << MWBase::Environment::get().getWorld()->getTimeManager()->getMonthName(mEntry->mMonth) << " (" + << dayStr << " " << (mEntry->mDay) << ')'; timestamp_buffer = os.str(); } @@ -273,31 +261,33 @@ namespace MWGui if (!questName.empty()) { std::vector quests; - for (MWBase::Journal::TQuestIter questIt = journal->questBegin(); questIt != journal->questEnd(); - ++questIt) + for (const auto& [_, quest] : journal->getQuests()) { - if (Misc::StringUtils::ciEqual(questIt->second.getName(), questName)) - quests.push_back(&questIt->second); + if (Misc::StringUtils::ciEqual(quest.getName(), questName)) + quests.push_back(&quest); } - for (MWBase::Journal::TEntryIter i = journal->begin(); i != journal->end(); ++i) + for (const MWDialogue::StampedJournalEntry& journalEntry : journal->getEntries()) { - for (std::vector::iterator questIt = quests.begin(); - questIt != quests.end(); ++questIt) + for (const MWDialogue::Quest* quest : quests) { - MWDialogue::Quest const* quest = *questIt; - for (MWDialogue::Topic::TEntryIter j = quest->begin(); j != quest->end(); ++j) + if (quest->getTopic() != journalEntry.mTopic) + continue; + for (const MWDialogue::Entry& questEntry : *quest) { - if (i->mInfoId == j->mInfoId) - visitor(JournalEntryImpl(this, i)); + if (journalEntry.mInfoId == questEntry.mInfoId) + { + visitor(JournalEntryImpl(this, journalEntry)); + break; + } } } } } else { - for (MWBase::Journal::TEntryIter i = journal->begin(); i != journal->end(); ++i) - visitor(JournalEntryImpl(this, i)); + for (const MWDialogue::StampedJournalEntry& journalEntry : journal->getEntries()) + visitor(JournalEntryImpl(this, journalEntry)); } } @@ -312,41 +302,40 @@ namespace MWGui { MWBase::Journal* journal = MWBase::Environment::get().getJournal(); - for (MWBase::Journal::TTopicIter i = journal->topicBegin(); i != journal->topicEnd(); ++i) + for (const auto& [_, topic] : journal->getTopics()) { - Utf8Stream stream(i->second.getName()); + Utf8Stream stream(topic.getName()); Utf8Stream::UnicodeChar first = Utf8Stream::toLowerUtf8(stream.peek()); if (first != Utf8Stream::toLowerUtf8(character)) continue; - visitor(i->second.getName()); + visitor(topic.getName()); } } - struct TopicEntryImpl : BaseEntry + struct TopicEntryImpl : BaseEntry { MWDialogue::Topic const& mTopic; - TopicEntryImpl(JournalViewModelImpl const* model, MWDialogue::Topic const& topic, iterator_t itr) - : BaseEntry(model, itr) + TopicEntryImpl( + JournalViewModelImpl const* model, MWDialogue::Topic const& topic, const MWDialogue::Entry& entry) + : BaseEntry(model, entry) , mTopic(topic) { } - std::string getText() const override { return mItr->getText(); } + std::string getText() const override { return mEntry->getText(); } - Utf8Span source() const override { return toUtf8Span(mItr->mActorName); } + Utf8Span source() const override { return toUtf8Span(mEntry->mActorName); } }; void visitTopicEntries(TopicId topicId, std::function visitor) const override { - typedef MWDialogue::Topic::TEntryIter iterator_t; - MWDialogue::Topic const& topic = *reinterpret_cast(topicId); - for (iterator_t i = topic.begin(); i != topic.end(); ++i) - visitor(TopicEntryImpl(this, topic, i)); + for (const MWDialogue::Entry& entry : topic) + visitor(TopicEntryImpl(this, topic, entry)); } }; diff --git a/apps/openmw/mwgui/journalviewmodel.hpp b/apps/openmw/mwgui/journalviewmodel.hpp index 376a8e1156..9b06ae465b 100644 --- a/apps/openmw/mwgui/journalviewmodel.hpp +++ b/apps/openmw/mwgui/journalviewmodel.hpp @@ -73,7 +73,7 @@ namespace MWGui virtual bool isEmpty() const = 0; /// walks the active and optionally completed, quests providing the name and completed status - virtual void visitQuestNames(bool active_only, std::function visitor) const = 0; + virtual void visitQuestNames(bool activeOnly, std::function visitor) const = 0; /// walks over the journal entries related to all quests with the given name /// If \a questName is empty, simply visits all journal entries diff --git a/apps/openmw/mwgui/journalwindow.cpp b/apps/openmw/mwgui/journalwindow.cpp index adc05cf5b8..afab1637c5 100644 --- a/apps/openmw/mwgui/journalwindow.cpp +++ b/apps/openmw/mwgui/journalwindow.cpp @@ -435,11 +435,9 @@ namespace ESM::RefId topic = ESM::RefId::stringRefId(topicIdString); const MWBase::Journal* journal = MWBase::Environment::get().getJournal(); intptr_t topicId = 0; /// \todo get rid of intptr ids - for (MWBase::Journal::TTopicIter i = journal->topicBegin(); i != journal->topicEnd(); ++i) - { - if (i->first == topic) - topicId = intptr_t(&i->second); - } + const auto it = journal->getTopics().find(topic); + if (it != journal->getTopics().end()) + topicId = intptr_t(&it->second); notifyTopicClicked(topicId); } diff --git a/apps/openmw/mwlua/types/player.cpp b/apps/openmw/mwlua/types/player.cpp index d8dda4202d..142bb5fb2a 100644 --- a/apps/openmw/mwlua/types/player.cpp +++ b/apps/openmw/mwlua/types/player.cpp @@ -105,7 +105,7 @@ namespace const MWDialogue::Topic& getTopicDataOrThrow(const ESM::RefId& topicId, const MWBase::Journal* journal) { const auto it = journal->getTopics().find(topicId); - if (it == journal->topicEnd()) + if (it == journal->getTopics().end()) throw std::runtime_error("Topic " + topicId.toDebugString() + " could not be found in the journal"); return it->second; } @@ -151,17 +151,17 @@ namespace MWLua = [journal]( const MWLua::Topics& topicEntriesStore, std::string_view givenTopicId) -> const MWDialogue::Topic* { const auto it = journal->getTopics().find(ESM::RefId::deserializeText(givenTopicId)); - if (it == journal->topicEnd()) + if (it == journal->getTopics().end()) return nullptr; return &it->second; }; topicsBindingsClass[sol::meta_function::length] = [journal](const MWLua::Topics&) -> size_t { return journal->getTopics().size(); }; topicsBindingsClass[sol::meta_function::pairs] = [journal](const MWLua::Topics&) { - MWBase::Journal::TTopicIter iterator = journal->topicBegin(); + auto iterator = journal->getTopics().begin(); return sol::as_function( [iterator, journal]() mutable -> std::pair, const MWDialogue::Topic*> { - if (iterator != journal->topicEnd()) + if (iterator != journal->getTopics().end()) { return { iterator->first.serializeText(), &((iterator++)->second) }; } @@ -306,8 +306,8 @@ namespace MWLua }; quests[sol::meta_function::pairs] = [journal](const Quests& self) { std::vector ids; - for (auto it = journal->questBegin(); it != journal->questEnd(); ++it) - ids.push_back(it->first); + for (const auto& [id, _] : journal->getQuests()) + ids.push_back(id); size_t i = 0; return [ids = std::move(ids), i, allowChanges = self.mMutable]() mutable -> sol::optional> {