Merge pull request #2805 from elsid/detournavigator_update_callback

Use callback to handle changed tiles

apps/openmw_test_suite/detournavigator/tilecachedrecastmeshmanager.cpp

@@ -20,6 +20,7 @@ namespace
     struct DetourNavigatorTileCachedRecastMeshManagerTest : Test
     {
         Settings mSettings;
+        std::vector<TilePosition> mChangedTiles;
 
         DetourNavigatorTileCachedRecastMeshManagerTest()
         {
@@ -29,6 +30,11 @@ namespace
             mSettings.mTileSize = 64;
             mSettings.mTrianglesPerChunk = 256;
         }
+
+        void onChangedTile(const TilePosition& tilePosition)
+        {
+            mChangedTiles.push_back(tilePosition);
+        }
     };
 
     TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, get_mesh_for_empty_should_return_nullptr)
@@ -78,8 +84,10 @@ namespace
         const btBoxShape boxShape(btVector3(20, 20, 100));
         const btTransform transform(btMatrix3x3::getIdentity(), btVector3(getTileSize(mSettings) / mSettings.mRecastScaleFactor, 0, 0));
         manager.addObject(ObjectId(&boxShape), boxShape, transform, AreaType::AreaType_ground);
+        EXPECT_TRUE(manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground,
+                                         [&] (const auto& v) { onChangedTile(v); }));
         EXPECT_THAT(
-            manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground),
+            mChangedTiles,
             ElementsAre(TilePosition(-1, -1), TilePosition(-1, 0), TilePosition(0, -1), TilePosition(0, 0),
                         TilePosition(1, -1), TilePosition(1, 0))
         );
@@ -90,10 +98,9 @@ namespace
         TileCachedRecastMeshManager manager(mSettings);
         const btBoxShape boxShape(btVector3(20, 20, 100));
         manager.addObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground);
-        EXPECT_EQ(
+        EXPECT_FALSE(manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground,
-            manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground),
+                                          [&] (const auto& v) { onChangedTile(v); }));
-            std::vector<TilePosition>()
+        EXPECT_EQ(mChangedTiles, std::vector<TilePosition>());
-        );
     }
 
     TEST_F(DetourNavigatorTileCachedRecastMeshManagerTest, get_mesh_after_add_object_should_return_recast_mesh_for_each_used_tile)
@@ -127,7 +134,7 @@ namespace
         EXPECT_NE(manager.getMesh(TilePosition(1, 0)), nullptr);
         EXPECT_NE(manager.getMesh(TilePosition(1, -1)), nullptr);
 
-        manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground);
+        manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground, [] (auto) {});
         EXPECT_NE(manager.getMesh(TilePosition(-1, -1)), nullptr);
         EXPECT_NE(manager.getMesh(TilePosition(-1, 0)), nullptr);
         EXPECT_NE(manager.getMesh(TilePosition(0, -1)), nullptr);
@@ -144,7 +151,7 @@ namespace
         EXPECT_EQ(manager.getMesh(TilePosition(-1, -1)), nullptr);
         EXPECT_EQ(manager.getMesh(TilePosition(-1, 0)), nullptr);
 
-        manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground);
+        manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground, [] (auto) {});
         EXPECT_EQ(manager.getMesh(TilePosition(1, 0)), nullptr);
         EXPECT_EQ(manager.getMesh(TilePosition(1, -1)), nullptr);
     }
@@ -172,7 +179,7 @@ namespace
         EXPECT_NE(manager.getMesh(TilePosition(0, -1)), nullptr);
         EXPECT_NE(manager.getMesh(TilePosition(0, 0)), nullptr);
 
-        manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground);
+        manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground, [] (auto) {});
         EXPECT_NE(manager.getMesh(TilePosition(-1, -1)), nullptr);
         EXPECT_NE(manager.getMesh(TilePosition(-1, 0)), nullptr);
         EXPECT_NE(manager.getMesh(TilePosition(0, -1)), nullptr);
@@ -205,7 +212,7 @@ namespace
         const btTransform transform(btMatrix3x3::getIdentity(), btVector3(getTileSize(mSettings) / mSettings.mRecastScaleFactor, 0, 0));
         manager.addObject(ObjectId(&boxShape), boxShape, transform, AreaType::AreaType_ground);
         const auto beforeUpdateRevision = manager.getRevision();
-        manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground);
+        manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground, [] (auto) {});
         EXPECT_EQ(manager.getRevision(), beforeUpdateRevision + 1);
     }
 
@@ -215,7 +222,7 @@ namespace
         const btBoxShape boxShape(btVector3(20, 20, 100));
         manager.addObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground);
         const auto beforeUpdateRevision = manager.getRevision();
-        manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground);
+        manager.updateObject(ObjectId(&boxShape), boxShape, btTransform::getIdentity(), AreaType::AreaType_ground, [] (auto) {});
         EXPECT_EQ(manager.getRevision(), beforeUpdateRevision);
     }
 

components/detournavigator/navmeshmanager.cpp

@@ -56,12 +56,8 @@ namespace DetourNavigator
     bool NavMeshManager::updateObject(const ObjectId id, const btCollisionShape& shape, const btTransform& transform,
                                       const AreaType areaType)
     {
-        const auto changedTiles = mRecastMeshManager.updateObject(id, shape, transform, areaType);
+        return mRecastMeshManager.updateObject(id, shape, transform, areaType,
-        if (changedTiles.empty())
+            [&] (const auto& tile) { addChangedTile(tile, ChangeType::update); });
-            return false;
-        for (const auto& tile : changedTiles)
-            addChangedTile(tile, ChangeType::update);
-        return true;
     }
 
     bool NavMeshManager::removeObject(const ObjectId id)

components/detournavigator/tilecachedrecastmeshmanager.cpp

@@ -31,43 +31,6 @@ namespace DetourNavigator
         return result;
     }
 
-    std::vector<TilePosition> TileCachedRecastMeshManager::updateObject(const ObjectId id, const btCollisionShape& shape,
-        const btTransform& transform, const AreaType areaType)
-    {
-        const auto object = mObjectsTilesPositions.find(id);
-        if (object == mObjectsTilesPositions.end())
-            return std::vector<TilePosition>();
-        auto& currentTiles = object->second;
-        const auto border = getBorderSize(mSettings);
-        std::vector<TilePosition> changedTiles;
-        std::set<TilePosition> newTiles;
-        {
-            auto tiles = mTiles.lock();
-            const auto onTilePosition = [&] (const TilePosition& tilePosition)
-            {
-                if (currentTiles.count(tilePosition))
-                {
-                    newTiles.insert(tilePosition);
-                    if (updateTile(id, transform, areaType, tilePosition, tiles.get()))
-                        changedTiles.push_back(tilePosition);
-                }
-                else if (addTile(id, shape, transform, areaType, tilePosition, border, tiles.get()))
-                {
-                    newTiles.insert(tilePosition);
-                    changedTiles.push_back(tilePosition);
-                }
-            };
-            getTilesPositions(shape, transform, mSettings, onTilePosition);
-            for (const auto& tile : currentTiles)
-                if (!newTiles.count(tile) && removeTile(id, tile, tiles.get()))
-                    changedTiles.push_back(tile);
-        }
-        std::swap(currentTiles, newTiles);
-        if (!changedTiles.empty())
-            ++mRevision;
-        return changedTiles;
-    }
-
     boost::optional<RemovedRecastMeshObject> TileCachedRecastMeshManager::removeObject(const ObjectId id)
     {
         const auto object = mObjectsTilesPositions.find(id);

components/detournavigator/tilecachedrecastmeshmanager.hpp

@@ -3,6 +3,8 @@
 
 #include "cachedrecastmeshmanager.hpp"
 #include "tileposition.hpp"
+#include "settingsutils.hpp"
+#include "gettilespositions.hpp"
 
 #include <components/misc/guarded.hpp>
 
@@ -20,8 +22,52 @@ namespace DetourNavigator
         bool addObject(const ObjectId id, const btCollisionShape& shape, const btTransform& transform,
                        const AreaType areaType);
 
-        std::vector<TilePosition> updateObject(const ObjectId id, const btCollisionShape& shape,
+        template <class OnChangedTile>
-                                               const btTransform& transform, const AreaType areaType);
+        bool updateObject(const ObjectId id, const btCollisionShape& shape, const btTransform& transform,
+            const AreaType areaType, OnChangedTile&& onChangedTile)
+        {
+            const auto object = mObjectsTilesPositions.find(id);
+            if (object == mObjectsTilesPositions.end())
+                return false;
+            auto& currentTiles = object->second;
+            const auto border = getBorderSize(mSettings);
+            bool changed = false;
+            std::set<TilePosition> newTiles;
+            {
+                auto tiles = mTiles.lock();
+                const auto onTilePosition = [&] (const TilePosition& tilePosition)
+                {
+                    if (currentTiles.count(tilePosition))
+                    {
+                        newTiles.insert(tilePosition);
+                        if (updateTile(id, transform, areaType, tilePosition, tiles.get()))
+                        {
+                            onChangedTile(tilePosition);
+                            changed = true;
+                        }
+                    }
+                    else if (addTile(id, shape, transform, areaType, tilePosition, border, tiles.get()))
+                    {
+                        newTiles.insert(tilePosition);
+                        onChangedTile(tilePosition);
+                        changed = true;
+                    }
+                };
+                getTilesPositions(shape, transform, mSettings, onTilePosition);
+                for (const auto& tile : currentTiles)
+                {
+                    if (!newTiles.count(tile) && removeTile(id, tile, tiles.get()))
+                    {
+                        onChangedTile(tile);
+                        changed = true;
+                    }
+                }
+            }
+            std::swap(currentTiles, newTiles);
+            if (changed)
+                ++mRevision;
+            return changed;
+        }
 
         boost::optional<RemovedRecastMeshObject> removeObject(const ObjectId id);