Changeset View
Standalone View
source/simulation2/components/CCmpPathfinder_Common.h
Show All 29 Lines | |||||||||||
#include "simulation2/system/Component.h" | #include "simulation2/system/Component.h" | ||||||||||
#include "ICmpPathfinder.h" | #include "ICmpPathfinder.h" | ||||||||||
#include "graphics/Overlay.h" | #include "graphics/Overlay.h" | ||||||||||
#include "graphics/Terrain.h" | #include "graphics/Terrain.h" | ||||||||||
#include "maths/MathUtil.h" | #include "maths/MathUtil.h" | ||||||||||
#include "ps/CLogger.h" | #include "ps/CLogger.h" | ||||||||||
#include "ps/ThreadFrontier.h" | |||||||||||
#include "renderer/TerrainOverlay.h" | #include "renderer/TerrainOverlay.h" | ||||||||||
#include "simulation2/components/ICmpObstructionManager.h" | #include "simulation2/components/ICmpObstructionManager.h" | ||||||||||
#include <thread> | |||||||||||
StanUnsubmitted Not Done Inline Actions
Stan: | |||||||||||
class HierarchicalPathfinder; | class HierarchicalPathfinder; | ||||||||||
class LongPathfinder; | class LongPathfinder; | ||||||||||
class VertexPathfinder; | class VertexPathfinder; | ||||||||||
class SceneCollector; | class SceneCollector; | ||||||||||
class AtlasOverlay; | class AtlasOverlay; | ||||||||||
#ifdef NDEBUG | #ifdef NDEBUG | ||||||||||
Show All 9 Lines | |||||||||||
{ | { | ||||||||||
protected: | protected: | ||||||||||
class PathfinderWorker | class PathfinderWorker | ||||||||||
{ | { | ||||||||||
friend CCmpPathfinder; | friend CCmpPathfinder; | ||||||||||
public: | public: | ||||||||||
PathfinderWorker(); | PathfinderWorker(); | ||||||||||
/** | |||||||||||
Not Done Inline ActionsDoxygen ? Stan: Doxygen ? | |||||||||||
* Implement a noexcept move constructor for std::vector that actually does nothing. | |||||||||||
*/ | |||||||||||
PathfinderWorker(PathfinderWorker&&) noexcept | |||||||||||
{ | |||||||||||
ENSURE(!m_Thread.joinable()); | |||||||||||
} | |||||||||||
~PathfinderWorker(); | |||||||||||
Not Done Inline ActionsDoxygen ? Stan: Doxygen ? | |||||||||||
// Process path requests, checking if we should stop before each new one. | /** | ||||||||||
* Create the std::thread and call InitThread | |||||||||||
*/ | |||||||||||
void Start(const CCmpPathfinder& pathfinder, size_t index); | |||||||||||
Not Done Inline ActionsDoxygen ? Stan: Doxygen ? | |||||||||||
void PrepareToKill(); | |||||||||||
/** | |||||||||||
* Will loop until a condition_variable notifies us, and call Work(). | |||||||||||
*/ | |||||||||||
void WaitForWork(const CCmpPathfinder& pathfinder); | |||||||||||
/** | |||||||||||
* Process path requests, checking if we should stop before each new one. | |||||||||||
* Should be callable both synchronously and asynchronously. | |||||||||||
*/ | |||||||||||
Not Done Inline ActionsDoxygen ? Stan: Doxygen ? | |||||||||||
void Work(const CCmpPathfinder& pathfinder); | void Work(const CCmpPathfinder& pathfinder); | ||||||||||
private: | private: | ||||||||||
// Insert requests in m_[Long/Short]Requests depending on from. | /** | ||||||||||
Not Done Inline ActionsDoxygen ? Stan: Doxygen ? | |||||||||||
// This could be removed when we may use if-constexpr in CCmpPathfinder::PushRequestsToWorkers | * Takes care of what needs to be called to initialise the thread before calling WaitForWork(). | ||||||||||
*/ | |||||||||||
void InitThread(const CCmpPathfinder& pathfinder, size_t index); | |||||||||||
/** | |||||||||||
* Insert requests in m_[Long/Short]Requests depending on from. | |||||||||||
* This could be removed when we may use if-constexpr in CCmpPathfinder::PushRequestsToWorkers | |||||||||||
*/ | |||||||||||
template<typename T> | template<typename T> | ||||||||||
void PushRequests(std::vector<T>& from, ssize_t amount); | void PushRequests(std::vector<T>& from, ssize_t amount); | ||||||||||
// Stores our results, the main thread will fetch this. | // Stores our results, the main thread will fetch this. | ||||||||||
std::vector<PathResult> m_Results; | std::vector<PathResult> m_Results; | ||||||||||
std::thread m_Thread; | |||||||||||
std::atomic<bool> m_Kill; | |||||||||||
Not Done Inline ActionsWhy do you need std::atomic if it's private? vladislavbelov: Why do you need `std::atomic` if it's private? | |||||||||||
Done Inline ActionsIt's still called from 2 threads, as the main thread calls PrepareToKill(), which sets m_Kill = true, and it's checked for in the worker thread. wraitii: It's still called from 2 threads, as the main thread calls PrepareToKill(), which sets m_Kill =… | |||||||||||
Not Done Inline Actions#include <atomic> Stan: ```
#include <atomic>
``` | |||||||||||
std::atomic<bool> m_Computing; | |||||||||||
std::vector<LongPathRequest> m_LongRequests; | std::vector<LongPathRequest> m_LongRequests; | ||||||||||
Not Done Inline ActionsProbably needs #include <vector> (likely included in its deps) Stan: Probably needs
```
#include <vector>
```
(likely included in its deps) | |||||||||||
std::vector<ShortPathRequest> m_ShortRequests; | std::vector<ShortPathRequest> m_ShortRequests; | ||||||||||
}; | }; | ||||||||||
// Allow the workers to access our private variables | // Allow the workers to access our private variables | ||||||||||
friend class PathfinderWorker; | friend class PathfinderWorker; | ||||||||||
Not Done Inline Actions#include <memory> Stan: ```
#include <memory>
``` | |||||||||||
public: | public: | ||||||||||
static void ClassInit(CComponentManager& componentManager) | static void ClassInit(CComponentManager& componentManager) | ||||||||||
{ | { | ||||||||||
componentManager.SubscribeToMessageType(MT_Deserialized); | componentManager.SubscribeToMessageType(MT_Deserialized); | ||||||||||
componentManager.SubscribeToMessageType(MT_Update); | componentManager.SubscribeToMessageType(MT_Update); | ||||||||||
componentManager.SubscribeToMessageType(MT_RenderSubmit); // for debug overlays | componentManager.SubscribeToMessageType(MT_RenderSubmit); // for debug overlays | ||||||||||
Done Inline ActionsSections should be in order: public > protected > private. vladislavbelov: Sections should be in order: public > protected > private. | |||||||||||
componentManager.SubscribeToMessageType(MT_TerrainChanged); | componentManager.SubscribeToMessageType(MT_TerrainChanged); | ||||||||||
componentManager.SubscribeToMessageType(MT_WaterChanged); | componentManager.SubscribeToMessageType(MT_WaterChanged); | ||||||||||
componentManager.SubscribeToMessageType(MT_ObstructionMapShapeChanged); | componentManager.SubscribeToMessageType(MT_ObstructionMapShapeChanged); | ||||||||||
componentManager.SubscribeToMessageType(MT_TurnStart); | componentManager.SubscribeToMessageType(MT_TurnStart); | ||||||||||
} | } | ||||||||||
~CCmpPathfinder(); | ~CCmpPathfinder(); | ||||||||||
DEFAULT_COMPONENT_ALLOCATOR(Pathfinder) | DEFAULT_COMPONENT_ALLOCATOR(Pathfinder) | ||||||||||
// Template state: | // Template state: | ||||||||||
Done Inline ActionsWhy comments states that the function uses conditional_variable but in fact it uses a simple busy waiting. vladislavbelov: Why comments states that the function uses `conditional_variable` but in fact it uses a simple… | |||||||||||
Done Inline ActionsEarlier version of the code used a condition_variable, I've changed it since. The thing is I need a m_Computing predicate anyways for the condition-variable, and possibly a mutex (I had issues which look a bit like this). wraitii: Earlier version of the code used a condition_variable, I've changed it since. The thing is I… | |||||||||||
Done Inline ActionsShould or is ? Stan: Should or is ? | |||||||||||
Done Inline ActionsShould. It also is, but it should be and remain so in the future. wraitii: Should. It also is, but it should be and remain so in the future. | |||||||||||
std::map<std::string, pass_class_t> m_PassClassMasks; | std::map<std::string, pass_class_t> m_PassClassMasks; | ||||||||||
std::vector<PathfinderPassability> m_PassClasses; | std::vector<PathfinderPassability> m_PassClasses; | ||||||||||
// Dynamic state: | // Dynamic state: | ||||||||||
std::vector<LongPathRequest> m_LongPathRequests; | std::vector<LongPathRequest> m_LongPathRequests; | ||||||||||
std::vector<ShortPathRequest> m_ShortPathRequests; | std::vector<ShortPathRequest> m_ShortPathRequests; | ||||||||||
Show All 10 Lines | public: | ||||||||||
// This should be used only in UpdateGrid(), there is no guarantee the data is properly initialized anywhere else. | // This should be used only in UpdateGrid(), there is no guarantee the data is properly initialized anywhere else. | ||||||||||
GridUpdateInformation m_DirtinessInformation; | GridUpdateInformation m_DirtinessInformation; | ||||||||||
// The data from clever updates is stored for the AI manager | // The data from clever updates is stored for the AI manager | ||||||||||
GridUpdateInformation m_AIPathfinderDirtinessInformation; | GridUpdateInformation m_AIPathfinderDirtinessInformation; | ||||||||||
bool m_TerrainDirty; | bool m_TerrainDirty; | ||||||||||
std::unique_ptr<VertexPathfinder> m_VertexPathfinder; | std::unique_ptr<VertexPathfinder> m_VertexPathfinder; | ||||||||||
std::unique_ptr<HierarchicalPathfinder> m_PathfinderHier; | std::unique_ptr<HierarchicalPathfinder> m_PathfinderHier; | ||||||||||
std::unique_ptr<LongPathfinder> m_LongPathfinder; | std::unique_ptr<LongPathfinder> m_LongPathfinder; | ||||||||||
Done Inline ActionsInitialization in Init ? Stan: Initialization in Init ? | |||||||||||
Done Inline ActionsI think it's better to init in the definition whenever possible going forward. Not sure if that's a convention we have or not? wraitii: I think it's better to init in the definition whenever possible going forward. Not sure if… | |||||||||||
// Workers process pathing requests. | // Worker process pathing requests. | ||||||||||
Not Done Inline ActionsProcess worker maybe ? Stan: Process worker maybe ? | |||||||||||
Not Done Inline Actions<atomic> Stan: <atomic> | |||||||||||
std::vector<PathfinderWorker> m_Workers; | std::vector<PathfinderWorker> m_Workers; | ||||||||||
bool m_UseThreading = false; | |||||||||||
mutable std::mutex m_PathfinderMutex; | |||||||||||
Not Done Inline Actions#include <mutex> Stan: ```
#include <mutex>
``` | |||||||||||
mutable std::condition_variable m_PathfinderConditionVariable; | |||||||||||
Not Done Inline Actions#include <condition_variable> Stan: ```
#include <condition_variable>
``` | |||||||||||
mutable ThreadFrontier m_PathfinderFrontier; | |||||||||||
// For now, we cannot push new requests while processing moves. | |||||||||||
// If this is ever useful, a double-buffered queue system might be needed to still preserve serialization capabilities. | |||||||||||
bool m_ProcessingMoves = false; | |||||||||||
Done Inline ActionsForgot to use that in this diff, but it'll be in the next. wraitii: Forgot to use that in this diff, but it'll be in the next. | |||||||||||
AtlasOverlay* m_AtlasOverlay; | AtlasOverlay* m_AtlasOverlay; | ||||||||||
static std::string GetSchema() | static std::string GetSchema() | ||||||||||
{ | { | ||||||||||
return "<a:component type='system'/><empty/>"; | return "<a:component type='system'/><empty/>"; | ||||||||||
} | } | ||||||||||
virtual void Init(const CParamNode& paramNode); | virtual void Init(const CParamNode& paramNode); | ||||||||||
virtual void Deinit(); | virtual void Deinit(); | ||||||||||
template<typename S> | template<typename S> | ||||||||||
void SerializeCommon(S& serialize); | void SerializeCommon(S& serialize); | ||||||||||
virtual void Serialize(ISerializer& serialize); | virtual void Serialize(ISerializer& serialize); | ||||||||||
Not Done Inline ActionsIt'd be good to fix names (to follow CC). Also as it'd be good to use "cache" word as the variables are used inside const methods. vladislavbelov: It'd be good to fix names (to follow CC). Also as it'd be good to use "cache" word as the… | |||||||||||
Done Inline ActionsThese were simply moved, and I think I'll refactor this in a VertexPathfinder class anyways that this calls (like the LongPathfinder) instead of what I've done here. wraitii: These were simply moved, and I think I'll refactor this in a VertexPathfinder class anyways… | |||||||||||
virtual void Deserialize(const CParamNode& paramNode, IDeserializer& deserialize); | virtual void Deserialize(const CParamNode& paramNode, IDeserializer& deserialize); | ||||||||||
virtual void HandleMessage(const CMessage& msg, bool global); | virtual void HandleMessage(const CMessage& msg, bool global); | ||||||||||
virtual pass_class_t GetPassabilityClass(const std::string& name) const; | virtual pass_class_t GetPassabilityClass(const std::string& name) const; | ||||||||||
virtual void GetPassabilityClasses(std::map<std::string, pass_class_t>& passClasses) const; | virtual void GetPassabilityClasses(std::map<std::string, pass_class_t>& passClasses) const; | ||||||||||
virtual void GetPassabilityClasses( | virtual void GetPassabilityClasses( | ||||||||||
▲ Show 20 Lines • Show All 128 Lines • Show Last 20 Lines |