From e11fbc10b1c5b63b7b94901ff7e7a4390c741dd5 Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 16 Jul 2022 12:36:10 +0200 Subject: [PATCH 1/4] Remove unused member functions from MWMechanics::DerivedClassStorage --- apps/openmw/mwmechanics/aistate.hpp | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/apps/openmw/mwmechanics/aistate.hpp b/apps/openmw/mwmechanics/aistate.hpp index 976e21c65c..692f731be3 100644 --- a/apps/openmw/mwmechanics/aistate.hpp +++ b/apps/openmw/mwmechanics/aistate.hpp @@ -1,8 +1,6 @@ #ifndef AISTATE_H #define AISTATE_H -#include - namespace MWMechanics { @@ -63,17 +61,7 @@ namespace MWMechanics delete mStorage; mStorage = p; } - - bool empty() const - { - return mStorage == nullptr; - } - - const std::type_info& getType() const - { - return typeid(mStorage); - } - + DerivedClassStorage():mStorage(nullptr){} ~DerivedClassStorage() { From f5c2e09df9bf92c88bef035466716d85c2fa91d4 Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 16 Jul 2022 13:00:03 +0200 Subject: [PATCH 2/4] Move AiTemporaryBase to a separate header --- apps/openmw/mwmechanics/aicombat.hpp | 1 + apps/openmw/mwmechanics/aifollow.hpp | 1 + apps/openmw/mwmechanics/aistate.hpp | 15 ++------------- apps/openmw/mwmechanics/aitemporarybase.hpp | 19 +++++++++++++++++++ apps/openmw/mwmechanics/aiwander.hpp | 2 +- 5 files changed, 24 insertions(+), 14 deletions(-) create mode 100644 apps/openmw/mwmechanics/aitemporarybase.hpp diff --git a/apps/openmw/mwmechanics/aicombat.hpp b/apps/openmw/mwmechanics/aicombat.hpp index a51393647c..1d8b8a38b3 100644 --- a/apps/openmw/mwmechanics/aicombat.hpp +++ b/apps/openmw/mwmechanics/aicombat.hpp @@ -2,6 +2,7 @@ #define GAME_MWMECHANICS_AICOMBAT_H #include "typedaipackage.hpp" +#include "aitemporarybase.hpp" #include "../mwworld/cellstore.hpp" // for Doors diff --git a/apps/openmw/mwmechanics/aifollow.hpp b/apps/openmw/mwmechanics/aifollow.hpp index c57d6da483..c1969a7e27 100644 --- a/apps/openmw/mwmechanics/aifollow.hpp +++ b/apps/openmw/mwmechanics/aifollow.hpp @@ -2,6 +2,7 @@ #define GAME_MWMECHANICS_AIFOLLOW_H #include "typedaipackage.hpp" +#include "aitemporarybase.hpp" #include #include diff --git a/apps/openmw/mwmechanics/aistate.hpp b/apps/openmw/mwmechanics/aistate.hpp index 692f731be3..73a054c59e 100644 --- a/apps/openmw/mwmechanics/aistate.hpp +++ b/apps/openmw/mwmechanics/aistate.hpp @@ -1,6 +1,8 @@ #ifndef AISTATE_H #define AISTATE_H +#include "aitemporarybase.hpp" + namespace MWMechanics { @@ -70,19 +72,6 @@ namespace MWMechanics } }; - - /// \brief base class for the temporary storage of AiPackages. - /** - * Each AI package with temporary values needs a AiPackageStorage class - * which is derived from AiTemporaryBase. The Actor holds a container - * AiState where one of these storages can be stored at a time. - * The execute(...) member function takes this container as an argument. - * */ - struct AiTemporaryBase - { - virtual ~AiTemporaryBase(){} - }; - /// \brief Container for AI package status. typedef DerivedClassStorage AiState; } diff --git a/apps/openmw/mwmechanics/aitemporarybase.hpp b/apps/openmw/mwmechanics/aitemporarybase.hpp new file mode 100644 index 0000000000..8ac8e4b71b --- /dev/null +++ b/apps/openmw/mwmechanics/aitemporarybase.hpp @@ -0,0 +1,19 @@ +#ifndef OPENMW_MWMECHANICS_AISTATE_H +#define OPENMW_MWMECHANICS_AISTATE_H + +namespace MWMechanics +{ + /// \brief base class for the temporary storage of AiPackages. + /** + * Each AI package with temporary values needs a AiPackageStorage class + * which is derived from AiTemporaryBase. The Actor holds a container + * AiState where one of these storages can be stored at a time. + * The execute(...) member function takes this container as an argument. + * */ + struct AiTemporaryBase + { + virtual ~AiTemporaryBase() = default; + }; +} + +#endif diff --git a/apps/openmw/mwmechanics/aiwander.hpp b/apps/openmw/mwmechanics/aiwander.hpp index 02a1054363..1adb4ed84e 100644 --- a/apps/openmw/mwmechanics/aiwander.hpp +++ b/apps/openmw/mwmechanics/aiwander.hpp @@ -7,7 +7,7 @@ #include "pathfinding.hpp" #include "obstacle.hpp" -#include "aistate.hpp" +#include "aitemporarybase.hpp" #include "aitimer.hpp" namespace ESM From d2b7253c7f56c170816b9a7df188daf0cc29d38f Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 16 Jul 2022 12:49:01 +0200 Subject: [PATCH 3/4] Use forward declarations instead of including aistate.hpp --- apps/openmw/mwmechanics/aipackage.hpp | 5 +++-- apps/openmw/mwmechanics/aisequence.hpp | 4 ---- apps/openmw/mwmechanics/aistate.hpp | 4 +--- apps/openmw/mwmechanics/aistatefwd.hpp | 15 +++++++++++++++ 4 files changed, 19 insertions(+), 9 deletions(-) create mode 100644 apps/openmw/mwmechanics/aistatefwd.hpp diff --git a/apps/openmw/mwmechanics/aipackage.hpp b/apps/openmw/mwmechanics/aipackage.hpp index 4e5dfcab6b..684ff3338d 100644 --- a/apps/openmw/mwmechanics/aipackage.hpp +++ b/apps/openmw/mwmechanics/aipackage.hpp @@ -4,12 +4,13 @@ #include #include - + #include "pathfinding.hpp" #include "obstacle.hpp" -#include "aistate.hpp" #include "aipackagetypeid.hpp" #include "aitimer.hpp" +#include "aistatefwd.hpp" + #include "../mwworld/ptr.hpp" namespace ESM diff --git a/apps/openmw/mwmechanics/aisequence.hpp b/apps/openmw/mwmechanics/aisequence.hpp index 5c6a43b9b8..e6f5a108c2 100644 --- a/apps/openmw/mwmechanics/aisequence.hpp +++ b/apps/openmw/mwmechanics/aisequence.hpp @@ -27,10 +27,6 @@ namespace MWMechanics { class AiPackage; class CharacterController; - - template< class Base > class DerivedClassStorage; - struct AiTemporaryBase; - typedef DerivedClassStorage AiState; using AiPackages = std::vector>; diff --git a/apps/openmw/mwmechanics/aistate.hpp b/apps/openmw/mwmechanics/aistate.hpp index 73a054c59e..ca90029e47 100644 --- a/apps/openmw/mwmechanics/aistate.hpp +++ b/apps/openmw/mwmechanics/aistate.hpp @@ -1,6 +1,7 @@ #ifndef AISTATE_H #define AISTATE_H +#include "aistatefwd.hpp" #include "aitemporarybase.hpp" namespace MWMechanics @@ -71,9 +72,6 @@ namespace MWMechanics delete mStorage; } }; - - /// \brief Container for AI package status. - typedef DerivedClassStorage AiState; } #endif // AISTATE_H diff --git a/apps/openmw/mwmechanics/aistatefwd.hpp b/apps/openmw/mwmechanics/aistatefwd.hpp new file mode 100644 index 0000000000..83216d1d27 --- /dev/null +++ b/apps/openmw/mwmechanics/aistatefwd.hpp @@ -0,0 +1,15 @@ +#ifndef OPENMW_MWMECHANICS_AISTATEFWD_H +#define OPENMW_MWMECHANICS_AISTATEFWD_H + +namespace MWMechanics +{ + template + class DerivedClassStorage; + + struct AiTemporaryBase; + + /// \brief Container for AI package status. + using AiState = DerivedClassStorage; +} + +#endif From b8937a493ab057c31e2f44fe0105ba4dc4c69076 Mon Sep 17 00:00:00 2001 From: elsid Date: Sat, 16 Jul 2022 12:41:06 +0200 Subject: [PATCH 4/4] Avoid manual memory management for MWMechanics::DerivedClassStorage --- apps/openmw/mwmechanics/aisequence.cpp | 6 +-- apps/openmw/mwmechanics/aistate.hpp | 51 ++++++++++---------------- apps/openmw/mwmechanics/aiwander.cpp | 2 +- 3 files changed, 23 insertions(+), 36 deletions(-) diff --git a/apps/openmw/mwmechanics/aisequence.cpp b/apps/openmw/mwmechanics/aisequence.cpp index fc91ce0c42..a9de4a5152 100644 --- a/apps/openmw/mwmechanics/aisequence.cpp +++ b/apps/openmw/mwmechanics/aisequence.cpp @@ -419,9 +419,9 @@ void AiSequence::stack (const AiPackage& package, const MWWorld::Ptr& actor, boo // Make sure that temporary storage is empty if (cancelOther) { - mAiState.moveIn(new AiCombatStorage()); - mAiState.moveIn(new AiFollowStorage()); - mAiState.moveIn(new AiWanderStorage()); + mAiState.moveIn(std::make_unique()); + mAiState.moveIn(std::make_unique()); + mAiState.moveIn(std::make_unique()); } } diff --git a/apps/openmw/mwmechanics/aistate.hpp b/apps/openmw/mwmechanics/aistate.hpp index ca90029e47..ca05c44b36 100644 --- a/apps/openmw/mwmechanics/aistate.hpp +++ b/apps/openmw/mwmechanics/aistate.hpp @@ -4,6 +4,8 @@ #include "aistatefwd.hpp" #include "aitemporarybase.hpp" +#include + namespace MWMechanics { @@ -14,28 +16,24 @@ namespace MWMechanics */ template< class Base > class DerivedClassStorage - { + { private: - Base* mStorage; - - //if needed you have to provide a clone member function - DerivedClassStorage( const DerivedClassStorage& other ); - DerivedClassStorage& operator=( const DerivedClassStorage& ); - + std::unique_ptr mStorage; + public: /// \brief returns reference to stored object or deletes it and creates a fitting template< class Derived > Derived& get() { - Derived* result = dynamic_cast(mStorage); - - if(!result) + Derived* result = dynamic_cast(mStorage.get()); + + if (result == nullptr) { - if(mStorage) - delete mStorage; - mStorage = result = new Derived(); + auto storage = std::make_unique(); + result = storage.get(); + mStorage = std::move(storage); } - + //return a reference to the (new allocated) object return *result; } @@ -43,33 +41,22 @@ namespace MWMechanics template< class Derived > void copy(DerivedClassStorage& destination) const { - Derived* result = dynamic_cast(mStorage); + Derived* result = dynamic_cast(mStorage.get()); if (result != nullptr) destination.store(*result); } - + template< class Derived > void store( const Derived& payload ) { - if(mStorage) - delete mStorage; - mStorage = new Derived(payload); - } - - /// \brief takes ownership of the passed object - template< class Derived > - void moveIn( Derived* p ) - { - if(mStorage) - delete mStorage; - mStorage = p; + mStorage = std::make_unique(payload); } - DerivedClassStorage():mStorage(nullptr){} - ~DerivedClassStorage() + /// \brief takes ownership of the passed object + template + void moveIn(std::unique_ptr&& storage) { - if(mStorage) - delete mStorage; + mStorage = std::move(storage); } }; } diff --git a/apps/openmw/mwmechanics/aiwander.cpp b/apps/openmw/mwmechanics/aiwander.cpp index f6c46945b1..777e86955d 100644 --- a/apps/openmw/mwmechanics/aiwander.cpp +++ b/apps/openmw/mwmechanics/aiwander.cpp @@ -767,7 +767,7 @@ namespace MWMechanics ToWorldCoordinates(dest, actor.getCell()->getCell()); - state.moveIn(new AiWanderStorage()); + state.moveIn(std::make_unique()); osg::Vec3f pos(static_cast(dest.mX), static_cast(dest.mY), static_cast(dest.mZ)); MWBase::Environment::get().getWorld()->moveObject(actor, pos);