Merge remote-tracking branch 'scrawl/master'

CMakeLists.txt

@@ -61,7 +61,7 @@ option(BUILD_MWINIIMPORTER "build MWiniImporter" ON)
 option(BUILD_OPENCS "build OpenMW Construction Set" ON)
 option(BUILD_WIZARD "build Installation Wizard" ON)
 option(BUILD_WITH_CODE_COVERAGE "Enable code coverage with gconv" OFF)
-option(BUILD_UNITTESTS "Enable Unittests with Google C++ Unittest and GMock frameworks" OFF)
+option(BUILD_UNITTESTS "Enable Unittests with Google C++ Unittest" OFF)
 option(BUILD_NIFTEST "build nif file tester" OFF)
 option(BUILD_MYGUI_PLUGIN "build MyGUI plugin for OpenMW resources, to use with MyGUI tools" ON)
 

apps/openmw/mwdialogue/hypertextparser.cpp

@@ -56,13 +56,17 @@ namespace MWDialogue
             keywordList.sort(Misc::StringUtils::ciLess);
 
             KeywordSearch<std::string, int /*unused*/> keywordSearch;
-            KeywordSearch<std::string, int /*unused*/>::Match match;
 
             for (std::list<std::string>::const_iterator it = keywordList.begin(); it != keywordList.end(); ++it)
                 keywordSearch.seed(*it, 0 /*unused*/);
 
-            for (std::string::const_iterator it = text.begin(); it != text.end() && keywordSearch.search(it, text.end(), match, text.begin()); it = match.mEnd)
+            std::vector<KeywordSearch<std::string, int /*unused*/>::Match> matches;
-                tokens.push_back(Token(std::string(match.mBeg, match.mEnd), Token::ImplicitKeyword));
+            keywordSearch.highlightKeywords(text.begin(), text.end(), matches);
+
+            for (std::vector<KeywordSearch<std::string, int /*unused*/>::Match>::const_iterator it = matches.begin(); it != matches.end(); ++it)
+            {
+                tokens.push_back(Token(std::string(it->mBeg, it->mEnd), Token::ImplicitKeyword));
+            }
         }
 
         size_t removePseudoAsterisks(std::string & phrase)

apps/openmw/mwdialogue/keywordsearch.hpp

@@ -66,19 +66,26 @@ public:
         return false;
     }
 
-    bool search (Point beg, Point end, Match & match, Point start)
+    static bool sortMatches(const Match& left, const Match& right)
     {
+        return left.mBeg < right.mBeg;
+    }
+
+    void highlightKeywords (Point beg, Point end, std::vector<Match>& out)
+    {
+        std::vector<Match> matches;
         for (Point i = beg; i != end; ++i)
         {
             // check if previous character marked start of new word
-            if (i != start)
+            if (i != beg)
             {
                 Point prev = i;
-                --prev; 
+                --prev;
                 if(isalpha(*prev))
                     continue;
             }
 
+
             // check first character
             typename Entry::childen_t::iterator candidate = mRoot.mChildren.find (std::tolower (*i, mLocale));
 
@@ -137,16 +144,57 @@ public:
                 if (t != candidate->second.mKeyword.end ())
                     continue;
 
-                // we did it, report the good news
+                // found a keyword, but there might still be longer keywords that start somewhere _within_ this keyword
+                // we will resolve these overlapping keywords later, choosing the longest one in case of conflict
+                Match match;
                 match.mValue = candidate->second.mValue;
                 match.mBeg = i;
                 match.mEnd = k;
-                return true;
+                matches.push_back(match);
+                break;
             }
         }
 
-        // no match in range, report the bad news
+        // resolve overlapping keywords
-        return false;
+        while (matches.size())
+        {
+            int longestKeywordSize = 0;
+            typename std::vector<Match>::iterator longestKeyword;
+            for (typename std::vector<Match>::iterator it = matches.begin(); it != matches.end(); ++it)
+            {
+                int size = it->mEnd - it->mBeg;
+                if (size > longestKeywordSize)
+                {
+                    longestKeywordSize = size;
+                    longestKeyword = it;
+                }
+
+                typename std::vector<Match>::iterator next = it;
+                ++next;
+
+                if (next == matches.end())
+                    break;
+
+                if (it->mEnd <= next->mBeg)
+                {
+                    break; // no overlap
+                }
+            }
+
+            Match keyword = *longestKeyword;
+            matches.erase(longestKeyword);
+            out.push_back(keyword);
+            // erase anything that overlaps with the keyword we just added to the output
+            for (typename std::vector<Match>::iterator it = matches.begin(); it != matches.end();)
+            {
+                if (it->mBeg < keyword.mEnd && it->mEnd > keyword.mBeg)
+                    it = matches.erase(it);
+                else
+                    ++it;
+            }
+        }
+
+        std::sort(out.begin(), out.end(), sortMatches);
     }
 
 private:

apps/openmw/mwgui/dialogue.cpp

@@ -177,11 +177,13 @@ namespace MWGui
         }
         else
         {
-            std::string::const_iterator i = text.begin ();
+            std::vector<KeywordSearchT::Match> matches;
-            KeywordSearchT::Match match;
+            keywordSearch->highlightKeywords(text.begin(), text.end(), matches);
 
-            while (i != text.end () && keywordSearch->search (i, text.end (), match, text.begin ()))
+            std::string::const_iterator i = text.begin ();
+            for (std::vector<KeywordSearchT::Match>::iterator it = matches.begin(); it != matches.end(); ++it)
             {
+                KeywordSearchT::Match match = *it;
                 if (i != match.mBeg)
                     addTopicLink (typesetter, 0, i - text.begin (), match.mBeg - text.begin ());
 
@@ -189,7 +191,6 @@ namespace MWGui
 
                 i = match.mEnd;
             }
-
             if (i != text.end ())
                 addTopicLink (typesetter, 0, i - text.begin (), text.size ());
         }

apps/openmw/mwgui/formatting.cpp

@@ -340,7 +340,7 @@ namespace MWGui
         {
             if (attr.find("color") != attr.end())
             {
-                int color;
+                unsigned int color;
                 std::stringstream ss;
                 ss << attr.at("color");
                 ss >> std::hex >> color;

apps/openmw/mwgui/journalviewmodel.cpp

@@ -174,12 +174,14 @@ struct JournalViewModelImpl : JournalViewModel
             }
             else
             {
+                std::vector<KeywordSearchT::Match> matches;
+                mModel->mKeywordSearch.highlightKeywords(utf8text.begin(), utf8text.end(), matches);
+
                 std::string::const_iterator i = utf8text.begin ();
-
+                for (std::vector<KeywordSearchT::Match>::const_iterator it = matches.begin(); it != matches.end(); ++it)
-                KeywordSearchT::Match match;
-
-                while (i != utf8text.end () && mModel->mKeywordSearch.search (i, utf8text.end (), match, utf8text.begin()))
                 {
+                    const KeywordSearchT::Match& match = *it;
+
                     if (i != match.mBeg)
                         visitor (0, i - utf8text.begin (), match.mBeg - utf8text.begin ());
 

apps/openmw/mwgui/windowmanagerimp.cpp

@@ -1455,6 +1455,8 @@ namespace MWGui
     void WindowManager::showSoulgemDialog(MWWorld::Ptr item)
     {
         mSoulgemDialog->show(item);
+        MWBase::Environment::get().getInputManager()->changeInputMode(isGuiMode());
+        updateVisible();
     }
 
     void WindowManager::frameStarted (float dt)

apps/openmw/mwinput/inputmanagerimp.cpp

@@ -409,8 +409,7 @@ namespace MWInput
 
                 if (mControlSwitch["playerviewswitch"]) {
 
-                    // work around preview mode toggle when pressing Alt+Tab
+                    if (actionIsActive(A_TogglePOV)) {
-                    if (actionIsActive(A_TogglePOV) && !mInputManager->isModifierHeld(SDL_Keymod(KMOD_ALT))) {
                         if (mPreviewPOVDelay <= 0.5 &&
                             (mPreviewPOVDelay += dt) > 0.5)
                         {

apps/openmw/mwrender/animation.cpp

@@ -336,7 +336,7 @@ void Animation::addExtraLight(Ogre::SceneManager *sceneMgr, NifOgre::ObjectScene
 {
     const MWWorld::Fallback *fallback = MWBase::Environment::get().getWorld()->getFallback();
 
-    const int clr = light->mData.mColor;
+    const unsigned int clr = light->mData.mColor;
     Ogre::ColourValue color(((clr >> 0) & 0xFF) / 255.0f,
                             ((clr >> 8) & 0xFF) / 255.0f,
                             ((clr >> 16) & 0xFF) / 255.0f);

apps/openmw/mwworld/esmstore.cpp

@@ -192,7 +192,15 @@ void ESMStore::setUp()
             case ESM::REC_LEVI:
             case ESM::REC_LEVC:
 
-                mStores[type]->read (reader);
+                {
+                    std::string id = reader.getHNString ("NAME");
+                    mStores[type]->read (reader, id);
+
+                    // FIXME: there might be stale dynamic IDs in mIds from an earlier savegame
+                    // that really should be cleared instead of just overwritten
+
+                    mIds[id] = type;
+                }
 
                 if (type==ESM::REC_NPC_)
                 {

apps/openmw/mwworld/esmstore.hpp

@@ -99,9 +99,6 @@ namespace MWWorld
         ESMStore()
           : mDynamicCount(0)
         {
-            // Cell store needs access to this for tracking moved references
-            mCells.mEsmStore = this;
-
             mStores[ESM::REC_ACTI] = &mActivators;
             mStores[ESM::REC_ALCH] = &mPotions;
             mStores[ESM::REC_APPA] = &mAppas;

apps/openmw/mwworld/store.cpp

@@ -11,8 +11,7 @@ void Store<ESM::Cell>::handleMovedCellRefs(ESM::ESMReader& esm, ESM::Cell* cell)
         ESM::MovedCellRef cMRef;
         cell->getNextMVRF(esm, cMRef);
 
-        MWWorld::Store<ESM::Cell> &cStore = const_cast<MWWorld::Store<ESM::Cell>&>(mEsmStore->get<ESM::Cell>());
+        ESM::Cell *cellAlt = const_cast<ESM::Cell*>(searchOrCreate(cMRef.mTarget[0], cMRef.mTarget[1]));
-        ESM::Cell *cellAlt = const_cast<ESM::Cell*>(cStore.searchOrCreate(cMRef.mTarget[0], cMRef.mTarget[1]));
 
         // Get regular moved reference data. Adapted from CellStore::loadRefs. Maybe we can optimize the following
         //  implementation when the oher implementation works as well.

apps/openmw/mwworld/store.hpp

@@ -30,7 +30,7 @@ namespace MWWorld
 
         virtual void write (ESM::ESMWriter& writer) const {}
 
-        virtual void read (ESM::ESMReader& reader) {}
+        virtual void read (ESM::ESMReader& reader, const std::string& id) {}
         ///< Read into dynamic storage
     };
 
@@ -140,6 +140,7 @@ namespace MWWorld
         virtual void clearDynamic()
         {
             // remove the dynamic part of mShared
+            assert(mShared.size() >= mStatic.size());
             mShared.erase(mShared.begin() + mStatic.size(), mShared.end());
             mDynamic.clear();
         }
@@ -216,8 +217,6 @@ namespace MWWorld
         }
 
         void setUp() {
-            // remove the dynamic part of mShared
-            mShared.erase(mShared.begin() + mStatic.size(), mShared.end());
         }
 
         iterator begin() const {
@@ -305,6 +304,7 @@ namespace MWWorld
             mDynamic.erase(it);
 
             // have to reinit the whole shared part
+            assert(mShared.size() >= mStatic.size());
             mShared.erase(mShared.begin() + mStatic.size(), mShared.end());
             for (it = mDynamic.begin(); it != mDynamic.end(); ++it) {
                 mShared.push_back(&it->second);
@@ -329,29 +329,15 @@ namespace MWWorld
             }
         }
 
-        void read (ESM::ESMReader& reader)
+        void read (ESM::ESMReader& reader, const std::string& id)
         {
             T record;
-            record.mId = reader.getHNString ("NAME");
+            record.mId = id;
             record.load (reader);
             insert (record);
         }
     };
 
-    template <>
-    inline void Store<ESM::NPC>::clearDynamic()
-    {
-        std::map<std::string, ESM::NPC>::iterator iter = mDynamic.begin();
-
-        while (iter!=mDynamic.end())
-            if (iter->first=="player")
-                ++iter;
-            else
-                mDynamic.erase (iter++);
-
-        mShared.clear();
-    }
-
     template <>
     inline void Store<ESM::Dialogue>::load(ESM::ESMReader &esm, const std::string &id) {
         std::string idLower = Misc::StringUtils::lowerCase(id);
@@ -593,14 +579,8 @@ namespace MWWorld
         void handleMovedCellRefs(ESM::ESMReader& esm, ESM::Cell* cell);
 
     public:
-        ESMStore *mEsmStore;
-
         typedef SharedIterator<ESM::Cell> iterator;
 
-        Store<ESM::Cell>()
-        : mEsmStore(NULL)
-        {}
-
         const ESM::Cell *search(const std::string &id) const {
             ESM::Cell cell;
             cell.mName = Misc::StringUtils::lowerCase(id);

apps/openmw_test_suite/CMakeLists.txt

@@ -1,24 +1,18 @@
-# TODO: This should not be needed, check how it was done in FindGTEST
-set(GMOCK_ROOT "/usr/include")
-set(GMOCK_BUILD "/usr/lib")
-
 find_package(GTest REQUIRED)
-find_package(GMock REQUIRED)
-
-if (GTEST_FOUND AND GMOCK_FOUND)
 
+if (GTEST_FOUND)
     include_directories(${GTEST_INCLUDE_DIRS})
-    include_directories(${GMOCK_INCLUDE_DIRS})
 
     file(GLOB UNITTEST_SRC_FILES
         components/misc/test_*.cpp
+        mwdialogue/test_*.cpp
     )
 
     source_group(apps\\openmw_test_suite FILES openmw_test_suite.cpp ${UNITTEST_SRC_FILES})
 
     add_executable(openmw_test_suite openmw_test_suite.cpp ${UNITTEST_SRC_FILES})
 
-    target_link_libraries(openmw_test_suite ${GMOCK_BOTH_LIBRARIES} ${GTEST_BOTH_LIBRARIES} components)
+    target_link_libraries(openmw_test_suite ${GTEST_BOTH_LIBRARIES} components)
     # Fix for not visible pthreads functions for linker with glibc 2.15
     if (UNIX AND NOT APPLE)
         target_link_libraries(openmw_test_suite ${CMAKE_THREAD_LIBS_INIT})

apps/openmw_test_suite/mwdialogue/test_keywordsearch.cpp

@@ -0,0 +1,67 @@
+#include <gtest/gtest.h>
+#include "apps/openmw/mwdialogue/keywordsearch.hpp"
+
+struct KeywordSearchTest : public ::testing::Test
+{
+  protected:
+    virtual void SetUp()
+    {
+    }
+
+    virtual void TearDown()
+    {
+    }
+};
+
+TEST_F(KeywordSearchTest, keyword_test_conflict_resolution)
+{
+    // test to make sure the longest keyword in a chain of conflicting keywords gets chosen
+    MWDialogue::KeywordSearch<std::string, int> search;
+    search.seed("foo bar", 0);
+    search.seed("bar lock", 0);
+    search.seed("lock switch", 0);
+
+    std::string text = "foo bar lock switch";
+
+    std::vector<MWDialogue::KeywordSearch<std::string, int>::Match> matches;
+    search.highlightKeywords(text.begin(), text.end(), matches);
+
+    // Should contain: "foo bar", "lock switch"
+    ASSERT_TRUE (matches.size() == 2);
+    ASSERT_TRUE (std::string(matches.front().mBeg, matches.front().mEnd) == "foo bar");
+    ASSERT_TRUE (std::string(matches.rbegin()->mBeg, matches.rbegin()->mEnd) == "lock switch");
+}
+
+TEST_F(KeywordSearchTest, keyword_test_conflict_resolution2)
+{
+    MWDialogue::KeywordSearch<std::string, int> search;
+    search.seed("the dwemer", 0);
+    search.seed("dwemer language", 0);
+
+    std::string text = "the dwemer language";
+
+    std::vector<MWDialogue::KeywordSearch<std::string, int>::Match> matches;
+    search.highlightKeywords(text.begin(), text.end(), matches);
+
+    ASSERT_TRUE (matches.size() == 1);
+    ASSERT_TRUE (std::string(matches.front().mBeg, matches.front().mEnd) == "dwemer language");
+}
+
+
+TEST_F(KeywordSearchTest, keyword_test_conflict_resolution3)
+{
+    // testing that the longest keyword is chosen, rather than maximizing the
+    // amount of highlighted characters by highlighting the first and last keyword
+    MWDialogue::KeywordSearch<std::string, int> search;
+    search.seed("foo bar", 0);
+    search.seed("bar lock", 0);
+    search.seed("lock so", 0);
+
+    std::string text = "foo bar lock so";
+
+    std::vector<MWDialogue::KeywordSearch<std::string, int>::Match> matches;
+    search.highlightKeywords(text.begin(), text.end(), matches);
+
+    ASSERT_TRUE (matches.size() == 1);
+    ASSERT_TRUE (std::string(matches.front().mBeg, matches.front().mEnd) == "bar lock");
+}

apps/openmw_test_suite/openmw_test_suite.cpp

@@ -1,12 +1,6 @@
-#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
-
+GTEST_API_ int main(int argc, char **argv) {
-int main(int argc, char** argv) {
+    testing::InitGoogleTest(&argc, argv);
-  // The following line causes Google Mock to throw an exception on failure,
+    return RUN_ALL_TESTS();
-  // which will be interpreted by your testing framework as a test failure.
-  ::testing::GTEST_FLAG(throw_on_failure) = false;
-  ::testing::InitGoogleMock(&argc, argv);
-
-  return RUN_ALL_TESTS();
 }

cmake/FindGMock.cmake

@@ -1,91 +0,0 @@
-# Locate the Google C++ Mocking Framework.
-#
-# Defines the following variables:
-#
-#   GMOCK_FOUND - Found the Google Mocking framework
-#   GMOCK_INCLUDE_DIRS - Include directories
-#
-# Also defines the library variables below as normal
-# variables.  These contain debug/optimized keywords when
-# a debugging library is found.
-#
-#   GMOCK_BOTH_LIBRARIES - Both libgmock & libgmock-main
-#   GMOCK_LIBRARIES - libgmock
-#   GMOCK_MAIN_LIBRARIES - libgmock-main
-#
-# Accepts the following variables as input:
-#
-#   GMOCK_ROOT - (as CMake or env. variable)
-#                The root directory of the gmock install prefix
-#
-#-----------------------
-# Example Usage:
-#
-#    enable_testing(true)
-#    find_package(GMock REQUIRED)
-#    include_directories(${GMOCK_INCLUDE_DIRS})
-#
-#    add_executable(foo foo.cc)
-#    target_link_libraries(foo ${GMOCK_BOTH_LIBRARIES})
-#
-#    add_test(AllTestsInFoo foo)
-#
-
-#set (GMOCK_FOUND FALSE)
-
-
-#set (GMOCK_ROOT $ENV{GMOCK_ROOT} CACHE PATH "Path to the gmock root directory.")
-if (NOT EXISTS ${GMOCK_ROOT})
-    message (FATAL_ERROR "GMOCK_ROOT does not exist.")
-endif ()
-
-#set (GMOCK_BUILD ${GMOCK_ROOT}/build CACHE PATH "Path to the gmock build directory.")
-if (NOT EXISTS ${GMOCK_BUILD})
-    message (FATAL_ERROR "GMOCK_BUILD does not exist.")
-endif ()
-
-# Find the include directory
-find_path(GMOCK_INCLUDE_DIRS gmock/gmock.h
-    HINTS
-        $ENV{GMOCK_ROOT}/include
-        ${GMOCK_ROOT}/include
-)
-mark_as_advanced(GMOCK_INCLUDE_DIRS)
-
-function(_gmock_find_library _name)
-    find_library(${_name}
-        NAMES ${ARGN}
-        HINTS
-            $ENV{GMOCK_BUILD}
-            ${GMOCK_BUILD}
-    )
-    mark_as_advanced(${_name})
-endfunction()
-
-# Find the gmock libraries
-if (MSVC)
-    _gmock_find_library (GMOCK_LIBRARIES_DEBUG   gmock ${GMOCK_BUILD}/Debug)
-    _gmock_find_library (GMOCK_LIBRARIES_RELEASE gmock ${GMOCK_BUILD}/Release)
-    _gmock_find_library (GMOCK_MAIN_LIBRARIES_DEBUG   gmock_main ${GMOCK_BUILD}/Debug)
-    _gmock_find_library (GMOCK_MAIN_LIBRARIES_RELEASE gmock_main ${GMOCK_BUILD}/Release)
-    set (GMOCK_LIBRARIES
-        debug ${GMOCK_LIBRARIES_DEBUG}
-        optimized ${GMOCK_LIBRARIES_RELEASE}
-    )
-    set (GMOCK_MAIN_LIBRARIES
-        debug ${GMOCK_MAIN_LIBRARIES_DEBUG}
-        optimized ${GMOCK_MAIN_LIBRARIES_RELEASE}
-    )
-else ()
-    _gmock_find_library (GMOCK_LIBRARIES      gmock      ${GMOCK_BUILD})
-    _gmock_find_library (GMOCK_MAIN_LIBRARIES gmock_main ${GMOCK_BUILD} ${GMOCK_BUILD}/Debug)
-endif ()
-
-FIND_PACKAGE_HANDLE_STANDARD_ARGS(GMock DEFAULT_MSG GMOCK_LIBRARIES GMOCK_INCLUDE_DIRS GMOCK_MAIN_LIBRARIES)
-
-if(GMOCK_FOUND)
-    set(GMOCK_INCLUDE_DIRS ${GMOCK_INCLUDE_DIR})
-    set(GMOCK_BOTH_LIBRARIES ${GMOCK_LIBRARIES} ${GMOCK_MAIN_LIBRARIES})
-endif()
-
-

components/esm/loadligh.hpp

@@ -37,7 +37,7 @@ struct Light
         int mValue;
         int mTime; // Duration
         int mRadius;
-        int mColor; // 4-byte rgba value
+        unsigned int mColor; // 4-byte rgba value
         int mFlags;
     }; // Size = 24 bytes
 

components/misc/stringops.cpp

@@ -1,14 +1,5 @@
 #include "stringops.hpp"
 
-#include <cctype>
-#include <algorithm>
-#include <iterator>
-
-#include <string.h>
-#include <libs/platform/strings.h>
-
-
-
 namespace Misc
 {