From eac613b7bf7025e273d93ed0a324b85b767c96b3 Mon Sep 17 00:00:00 2001 From: wraitii Date: Fri, 26 Mar 2021 16:47:07 +0000 Subject: [PATCH] Enforce tight coupling of CCmpUnitMotion/CCmpUnitMotionManager bae258f9a1 implemented CCmpUnitMotionManager, which handles motion for CCmpUnitMotion (as distinct from ICmpUnitMotion, the interface). The tight coupling between these two components was awkward at the interface level, leaking underlying implementation details. This diff makes CmpUnitMotionManager explicitly manage CmpUnitMotion, instead of any implementation of ICmpUnitMotion, and moves files around as requir ed. This: - Makes it impossible to accidentally try to have the wrong IID_UnitMotion managed by the UnitMotionManager. - Allows devirtualising the calls from the manager to UnitMotion itself (and inlining, as they are compiled in the same TU). - Cleans up the manager interface - MotionState is now part of CCmpUnitMotionManager. - Cleans up ICmpUnitMotion interface - no need to provide a private interface for the manager. Differential Revision: https://code.wildfiregames.com/D3732 This was SVN commit r25125. --- .../{CCmpUnitMotion.cpp => CCmpUnitMotion.h} | 32 +++--- ...ionManager.cpp => CCmpUnitMotionManager.h} | 108 +++++------------- .../components/CCmpUnitMotion_System.cpp | 101 ++++++++++++++++ .../simulation2/components/ICmpUnitMotion.cpp | 8 -- .../simulation2/components/ICmpUnitMotion.h | 15 --- .../components/ICmpUnitMotionManager.h | 42 +++---- 6 files changed, 163 insertions(+), 143 deletions(-) rename source/simulation2/components/{CCmpUnitMotion.cpp => CCmpUnitMotion.h} (98%) rename source/simulation2/components/{CCmpUnitMotionManager.cpp => CCmpUnitMotionManager.h} (57%) create mode 100644 source/simulation2/components/CCmpUnitMotion_System.cpp diff --git a/source/simulation2/components/CCmpUnitMotion.cpp b/source/simulation2/components/CCmpUnitMotion.h similarity index 98% rename from source/simulation2/components/CCmpUnitMotion.cpp rename to source/simulation2/components/CCmpUnitMotion.h index 8c5ce40663..70d7a9e8cb 100644 --- a/source/simulation2/components/CCmpUnitMotion.cpp +++ b/source/simulation2/components/CCmpUnitMotion.h @@ -15,11 +15,13 @@ * along with 0 A.D. If not, see . */ -#include "precompiled.h" +#ifndef INCLUDED_CCMPUNITMOTION +#define INCLUDED_CCMPUNITMOTION #include "simulation2/system/Component.h" #include "ICmpUnitMotion.h" +#include "simulation2/components/CCmpUnitMotionManager.h" #include "simulation2/components/ICmpObstruction.h" #include "simulation2/components/ICmpObstructionManager.h" #include "simulation2/components/ICmpOwnership.h" @@ -41,6 +43,9 @@ #include "ps/Profile.h" #include "renderer/Scene.h" +// NB: this implementation of ICmpUnitMotion is very tightly coupled with UnitMotionManager. +// As such, both are compiled in the same TU. + // For debugging; units will start going straight to the target // instead of calling the pathfinder #define DISABLE_PATHFINDER 0 @@ -113,8 +118,9 @@ static const u8 VERY_OBSTRUCTED_THRESHOLD = 10; static const CColor OVERLAY_COLOR_LONG_PATH(1, 1, 1, 1); static const CColor OVERLAY_COLOR_SHORT_PATH(1, 0, 0, 1); -class CCmpUnitMotion : public ICmpUnitMotion +class CCmpUnitMotion final : public ICmpUnitMotion { + friend class CCmpUnitMotionManager; public: static void ClassInit(CComponentManager& componentManager) { @@ -326,7 +332,7 @@ public: case MT_Create: { if (!ENTITY_IS_LOCAL(GetEntityId())) - CmpPtr(GetSystemEntity())->Register(GetEntityId(), m_FormationController); + CmpPtr(GetSystemEntity())->Register(this, GetEntityId(), m_FormationController); break; } case MT_Destroy: @@ -351,7 +357,7 @@ public: { OnValueModification(); if (!ENTITY_IS_LOCAL(GetEntityId())) - CmpPtr(GetSystemEntity())->Register(GetEntityId(), m_FormationController); + CmpPtr(GetSystemEntity())->Register(this, GetEntityId(), m_FormationController); break; } } @@ -640,13 +646,11 @@ private: * and ensure that distance comparisons are done while units are not being moved * (otherwise they won't be commutative). */ - virtual void OnTurnStart(); + void OnTurnStart(); - virtual void PreMove(ICmpUnitMotionManager::MotionState& state); - - virtual void Move(ICmpUnitMotionManager::MotionState& state, fixed dt); - - virtual void PostMove(ICmpUnitMotionManager::MotionState& state, fixed dt); + void PreMove(CCmpUnitMotionManager::MotionState& state); + void Move(CCmpUnitMotionManager::MotionState& state, fixed dt); + void PostMove(CCmpUnitMotionManager::MotionState& state, fixed dt); /** * Returns true if we are possibly at our destination. @@ -911,14 +915,14 @@ void CCmpUnitMotion::OnTurnStart() } } -void CCmpUnitMotion::PreMove(ICmpUnitMotionManager::MotionState& state) +void CCmpUnitMotion::PreMove(CCmpUnitMotionManager::MotionState& state) { // If we were idle and will still be, no need for an update. state.needUpdate = state.cmpPosition->IsInWorld() && (m_CurSpeed != fixed::Zero() || m_MoveRequest.m_Type != MoveRequest::NONE); } -void CCmpUnitMotion::Move(ICmpUnitMotionManager::MotionState& state, fixed dt) +void CCmpUnitMotion::Move(CCmpUnitMotionManager::MotionState& state, fixed dt) { PROFILE("Move"); @@ -930,7 +934,7 @@ void CCmpUnitMotion::Move(ICmpUnitMotionManager::MotionState& state, fixed dt) state.wasObstructed = PerformMove(dt, state.cmpPosition->GetTurnRate(), m_ShortPath, m_LongPath, state.pos, state.angle); } -void CCmpUnitMotion::PostMove(ICmpUnitMotionManager::MotionState& state, fixed dt) +void CCmpUnitMotion::PostMove(CCmpUnitMotionManager::MotionState& state, fixed dt) { // Update our speed over this turn so that the visual actor shows the correct animation. if (state.pos == state.initialPos) @@ -1718,3 +1722,5 @@ void CCmpUnitMotion::RenderSubmit(SceneCollector& collector) for (size_t i = 0; i < m_DebugOverlayShortPathLines.size(); ++i) collector.Submit(&m_DebugOverlayShortPathLines[i]); } + +#endif // INCLUDED_CCMPUNITMOTION diff --git a/source/simulation2/components/CCmpUnitMotionManager.cpp b/source/simulation2/components/CCmpUnitMotionManager.h similarity index 57% rename from source/simulation2/components/CCmpUnitMotionManager.cpp rename to source/simulation2/components/CCmpUnitMotionManager.h index bf7fa96330..4078091981 100644 --- a/source/simulation2/components/CCmpUnitMotionManager.cpp +++ b/source/simulation2/components/CCmpUnitMotionManager.h @@ -15,17 +15,16 @@ * along with 0 A.D. If not, see . */ -#include "precompiled.h" +#ifndef INCLUDED_CCMPUNITMOTIONMANAGER +#define INCLUDED_CCMPUNITMOTIONMANAGER #include "simulation2/system/Component.h" #include "ICmpUnitMotionManager.h" #include "simulation2/MessageTypes.h" -#include "simulation2/components/ICmpUnitMotion.h" #include "simulation2/system/EntityMap.h" -#include "ps/CLogger.h" -#include "ps/Profile.h" +class CCmpUnitMotion; class CCmpUnitMotionManager : public ICmpUnitMotionManager { @@ -40,6 +39,30 @@ public: DEFAULT_COMPONENT_ALLOCATOR(UnitMotionManager) + // Persisted state for each unit. + struct MotionState + { + // Component references - these must be kept alive for the duration of motion. + // NB: this is generally not something one should do, but because of the tight coupling here it's doable. + CmpPtr cmpPosition; + CCmpUnitMotion* cmpUnitMotion; + + // Position before units start moving + CFixedVector2D initialPos; + // Transient position during the movement. + CFixedVector2D pos; + + fixed initialAngle; + fixed angle; + + // If true, the entity needs to be handled during movement. + bool needUpdate; + + // 'Leak' from UnitMotion. + bool wentStraight; + bool wasObstructed; + }; + EntityMap m_Units; EntityMap m_FormationControllers; @@ -99,7 +122,7 @@ public: } } - virtual void Register(entity_id_t ent, bool formationController); + virtual void Register(CCmpUnitMotion* component, entity_id_t ent, bool formationController); virtual void Unregister(entity_id_t ent); virtual bool ComputingMotion() const @@ -114,77 +137,6 @@ public: void Move(EntityMap& ents, fixed dt); }; -void CCmpUnitMotionManager::Register(entity_id_t ent, bool formationController) -{ - MotionState state = { - CmpPtr(GetSimContext(), ent), - CmpPtr(GetSimContext(), ent), - CFixedVector2D(), - CFixedVector2D(), - fixed::Zero(), - fixed::Zero(), - false, - false - }; - if (!formationController) - m_Units.insert(ent, state); - else - m_FormationControllers.insert(ent, state); -} - -void CCmpUnitMotionManager::Unregister(entity_id_t ent) -{ - EntityMap::iterator it = m_Units.find(ent); - if (it != m_Units.end()) - { - m_Units.erase(it); - return; - } - it = m_FormationControllers.find(ent); - if (it != m_FormationControllers.end()) - m_FormationControllers.erase(it); -} - -void CCmpUnitMotionManager::OnTurnStart() -{ - for (EntityMap::value_type& data : m_FormationControllers) - data.second.cmpUnitMotion->OnTurnStart(); - - for (EntityMap::value_type& data : m_Units) - data.second.cmpUnitMotion->OnTurnStart(); -} - -void CCmpUnitMotionManager::MoveUnits(fixed dt) -{ - Move(m_Units, dt); -} - -void CCmpUnitMotionManager::MoveFormations(fixed dt) -{ - Move(m_FormationControllers, dt); -} - -void CCmpUnitMotionManager::Move(EntityMap& ents, fixed dt) -{ - m_MovingUnits.clear(); - for (EntityMap::iterator it = ents.begin(); it != ents.end(); ++it) - { - it->second.cmpUnitMotion->PreMove(it->second); - if (!it->second.needUpdate) - continue; - m_MovingUnits.push_back(it); - it->second.initialPos = it->second.cmpPosition->GetPosition2D(); - it->second.initialAngle = it->second.cmpPosition->GetRotation().Y; - it->second.pos = it->second.initialPos; - it->second.angle = it->second.initialAngle; - } - - for (EntityMap::iterator& it : m_MovingUnits) - it->second.cmpUnitMotion->Move(it->second, dt); - - for (EntityMap::iterator& it : m_MovingUnits) - it->second.cmpUnitMotion->PostMove(it->second, dt); -} - - REGISTER_COMPONENT_TYPE(UnitMotionManager) + +#endif // INCLUDED_CCMPUNITMOTIONMANAGER diff --git a/source/simulation2/components/CCmpUnitMotion_System.cpp b/source/simulation2/components/CCmpUnitMotion_System.cpp new file mode 100644 index 0000000000..19c564e518 --- /dev/null +++ b/source/simulation2/components/CCmpUnitMotion_System.cpp @@ -0,0 +1,101 @@ +/* Copyright (C) 2021 Wildfire Games. + * This file is part of 0 A.D. + * + * 0 A.D. is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * 0 A.D. is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with 0 A.D. If not, see . + */ + +#include "precompiled.h" + +#include "CCmpUnitMotion.h" +#include "CCmpUnitMotionManager.h" + +#include "ps/CLogger.h" +#include "ps/Profile.h" + +// NB: this TU contains the CCmpUnitMotion/CCmpUnitMotionManager couple. +// In practice, UnitMotionManager functions need access to the full implementation of UnitMotion, +// but UnitMotion needs access to MotionState (defined in UnitMotionManager). +// To avoid inclusion issues, implementation of UnitMotionManager that uses UnitMotion is here. + +void CCmpUnitMotionManager::Register(CCmpUnitMotion* component, entity_id_t ent, bool formationController) +{ + MotionState state = { + CmpPtr(GetSimContext(), ent), + component, + CFixedVector2D(), + CFixedVector2D(), + fixed::Zero(), + fixed::Zero(), + false, + false + }; + if (!formationController) + m_Units.insert(ent, state); + else + m_FormationControllers.insert(ent, state); +} + +void CCmpUnitMotionManager::Unregister(entity_id_t ent) +{ + EntityMap::iterator it = m_Units.find(ent); + if (it != m_Units.end()) + { + m_Units.erase(it); + return; + } + it = m_FormationControllers.find(ent); + if (it != m_FormationControllers.end()) + m_FormationControllers.erase(it); +} + +void CCmpUnitMotionManager::OnTurnStart() +{ + for (EntityMap::value_type& data : m_FormationControllers) + data.second.cmpUnitMotion->OnTurnStart(); + + for (EntityMap::value_type& data : m_Units) + data.second.cmpUnitMotion->OnTurnStart(); +} + +void CCmpUnitMotionManager::MoveUnits(fixed dt) +{ + Move(m_Units, dt); +} + +void CCmpUnitMotionManager::MoveFormations(fixed dt) +{ + Move(m_FormationControllers, dt); +} + +void CCmpUnitMotionManager::Move(EntityMap& ents, fixed dt) +{ + m_MovingUnits.clear(); + for (EntityMap::iterator it = ents.begin(); it != ents.end(); ++it) + { + it->second.cmpUnitMotion->PreMove(it->second); + if (!it->second.needUpdate) + continue; + m_MovingUnits.push_back(it); + it->second.initialPos = it->second.cmpPosition->GetPosition2D(); + it->second.initialAngle = it->second.cmpPosition->GetRotation().Y; + it->second.pos = it->second.initialPos; + it->second.angle = it->second.initialAngle; + } + + for (EntityMap::iterator& it : m_MovingUnits) + it->second.cmpUnitMotion->Move(it->second, dt); + + for (EntityMap::iterator& it : m_MovingUnits) + it->second.cmpUnitMotion->PostMove(it->second, dt); +} diff --git a/source/simulation2/components/ICmpUnitMotion.cpp b/source/simulation2/components/ICmpUnitMotion.cpp index e6ccb7d893..196a88d319 100644 --- a/source/simulation2/components/ICmpUnitMotion.cpp +++ b/source/simulation2/components/ICmpUnitMotion.cpp @@ -48,14 +48,6 @@ class CCmpUnitMotionScripted : public ICmpUnitMotion public: DEFAULT_SCRIPT_WRAPPER(UnitMotionScripted) -private: - virtual void OnTurnStart() {}; - virtual void PreMove(ICmpUnitMotionManager::MotionState&) {}; - virtual void Move(ICmpUnitMotionManager::MotionState&, fixed) {}; - virtual void PostMove(ICmpUnitMotionManager::MotionState&, fixed) {}; - -public: - virtual bool MoveToPointRange(entity_pos_t x, entity_pos_t z, entity_pos_t minRange, entity_pos_t maxRange) { return m_Script.Call("MoveToPointRange", x, z, minRange, maxRange); diff --git a/source/simulation2/components/ICmpUnitMotion.h b/source/simulation2/components/ICmpUnitMotion.h index 49f442eb7a..caf8c5a78a 100644 --- a/source/simulation2/components/ICmpUnitMotion.h +++ b/source/simulation2/components/ICmpUnitMotion.h @@ -22,9 +22,6 @@ #include "simulation2/components/ICmpPathfinder.h" // for pass_class_t #include "simulation2/components/ICmpPosition.h" // for entity_pos_t -#include "simulation2/components/ICmpUnitMotionManager.h" - -class CCmpUnitMotionManager; /** * Motion interface for entities with complex movement capabilities. @@ -36,18 +33,6 @@ class CCmpUnitMotionManager; */ class ICmpUnitMotion : public IComponent { -protected: - friend class CCmpUnitMotionManager; - - /** - * This external interface is used by the Unit Motion Manager. - * Components that do not register there do not need to implement these. - */ - virtual void OnTurnStart() = 0; - virtual void PreMove(ICmpUnitMotionManager::MotionState& state) = 0; - virtual void Move(ICmpUnitMotionManager::MotionState& state, fixed dt) = 0; - virtual void PostMove(ICmpUnitMotionManager::MotionState& state, fixed dt) = 0; - public: /** diff --git a/source/simulation2/components/ICmpUnitMotionManager.h b/source/simulation2/components/ICmpUnitMotionManager.h index 5f91f9c20c..26794abdad 100644 --- a/source/simulation2/components/ICmpUnitMotionManager.h +++ b/source/simulation2/components/ICmpUnitMotionManager.h @@ -20,40 +20,25 @@ #include "simulation2/system/Interface.h" -#include "maths/Fixed.h" -#include "maths/FixedVector2D.h" -#include "simulation2/system/CmpPtr.h" - -class ICmpPosition; -class ICmpUnitMotion; +class CCmpUnitMotion; +/** + * UnitMotionManager - handles motion for CCmpUnitMotion. + * This allows units to push past each other instead of requiring pathfinder computations, + * making movement much smoother overall. + */ class ICmpUnitMotionManager : public IComponent { public: - // Persisted state for each unit. - struct MotionState - { - // Component references - these must be kept alive for the duration of motion. - CmpPtr cmpPosition; - CmpPtr cmpUnitMotion; + DECLARE_INTERFACE_TYPE(UnitMotionManager) - // Position before units start moving - CFixedVector2D initialPos; - // Transient position during the movement. - CFixedVector2D pos; +private: + /** + * This class makes no sense outside of CCmpUnitMotion. This enforces that tight coupling. + */ + friend class CCmpUnitMotion; - fixed initialAngle; - fixed angle; - - // If true, the entity needs to be handled during movement. - bool needUpdate; - - // 'Leak' from UnitMotion. - bool wentStraight; - bool wasObstructed; - }; - - virtual void Register(entity_id_t ent, bool formationController) = 0; + virtual void Register(CCmpUnitMotion* component, entity_id_t ent, bool formationController) = 0; virtual void Unregister(entity_id_t ent) = 0; /** @@ -61,7 +46,6 @@ public: */ virtual bool ComputingMotion() const = 0; - DECLARE_INTERFACE_TYPE(UnitMotionManager) }; #endif // INCLUDED_ICMPUNITMOTIONMANAGER