From eb70453574e87eeae2e57b648830711e3ea1e647 Mon Sep 17 00:00:00 2001 From: Gawaboumga Date: Thu, 20 Apr 2017 12:30:43 +0200 Subject: [PATCH 1/3] State machine as a stack (#123) * State machine as a stack * Fix comments linked to code review * I will think to compile, next time --- SDK/include/NDK/StateMachine.hpp | 14 +++- SDK/include/NDK/StateMachine.inl | 137 +++++++++++++++++++++++++------ tests/SDK/NDK/StateMachine.cpp | 64 ++++++++++++++- 3 files changed, 186 insertions(+), 29 deletions(-) diff --git a/SDK/include/NDK/StateMachine.hpp b/SDK/include/NDK/StateMachine.hpp index dae81ecd0..e9e665c44 100644 --- a/SDK/include/NDK/StateMachine.hpp +++ b/SDK/include/NDK/StateMachine.hpp @@ -1,4 +1,4 @@ -// Copyright (C) 2017 Jérôme Leclercq +// Copyright (C) 2017 Jérôme Leclercq // This file is part of the "Nazara Development Kit" // For conditions of distribution and use, see copyright notice in Prerequesites.hpp @@ -10,6 +10,7 @@ #include #include #include +#include namespace Ndk { @@ -25,14 +26,21 @@ namespace Ndk inline const std::shared_ptr& GetCurrentState() const; + inline bool IsTopState(const State* state) const; + + inline std::shared_ptr PopState(); + inline bool PopStatesUntil(std::shared_ptr state); + inline void PushState(std::shared_ptr state); + + inline void SetState(std::shared_ptr state); + inline bool Update(float elapsedTime); inline StateMachine& operator=(StateMachine&& fsm) = default; StateMachine& operator=(const StateMachine&) = delete; private: - std::shared_ptr m_currentState; - std::shared_ptr m_nextState; + std::vector> m_states; }; } diff --git a/SDK/include/NDK/StateMachine.inl b/SDK/include/NDK/StateMachine.inl index d30146a97..1086df54d 100644 --- a/SDK/include/NDK/StateMachine.inl +++ b/SDK/include/NDK/StateMachine.inl @@ -3,7 +3,6 @@ // For conditions of distribution and use, see copyright notice in Prerequesites.hpp #include -#include #include namespace Ndk @@ -11,7 +10,7 @@ namespace Ndk /*! * \ingroup NDK * \class Ndk::StateMachine - * \brief NDK class that represents a state machine, to represent the multiple states of your program + * \brief NDK class that represents a state machine, to represent the multiple states of your program as a stack */ /*! @@ -23,61 +22,153 @@ namespace Ndk * \remark Produces a NazaraAssert if nullptr is given */ - inline StateMachine::StateMachine(std::shared_ptr originalState) : - m_currentState(std::move(originalState)) + inline StateMachine::StateMachine(std::shared_ptr originalState) { - NazaraAssert(m_currentState, "StateMachine must have a state to begin with"); - m_currentState->Enter(*this); + NazaraAssert(originalState, "StateMachine must have a state to begin with"); + PushState(std::move(originalState)); } /*! * \brief Destructs the object * - * \remark Calls "Leave" on the state + * \remark Calls "Leave" on all the states */ inline StateMachine::~StateMachine() { - m_currentState->Leave(*this); + for (std::shared_ptr& state : m_states) + state->Leave(*this); } /*! - * \brief Changes the current state of the machine + * \brief Replaces the current state on the top of the machine * - * \param state Next state to represent + * \param state State to replace the top one if it is nullptr, no action is performed */ inline void StateMachine::ChangeState(std::shared_ptr state) { - m_nextState = std::move(state); + if (state) + { + PopState(); + PushState(std::move(state)); + } } /*! - * \brief Gets the current state of the machine + * \brief Gets the current state on the top of the machine * \return A constant reference to the state + * + * \remark The stack is supposed to be non empty, otherwise it is undefined behaviour + * + * \see PopStatesUntil */ inline const std::shared_ptr& StateMachine::GetCurrentState() const { - return m_currentState; + return m_states.back(); } /*! - * \brief Updates the state - * \return True if update is successful + * \brief Checks whether the state is on the top of the machine + * \return true If it is the case + * + * \param state State to compare the top with + */ + + inline bool StateMachine::IsTopState(const State* state) const + { + if (m_states.empty()) + return false; + + return m_states.back().get() == state; + } + + /*! + * \brief Pops the state on the top of the machine + * \return Old state on the top, nullptr if stack was empty + * + * \remark This method can completely empty the stack + */ + + inline std::shared_ptr StateMachine::PopState() + { + if (m_states.empty()) + return nullptr; + + m_states.back()->Leave(*this); + std::shared_ptr oldTopState = std::move(m_states.back()); + m_states.pop_back(); + return oldTopState; + } + + /*! + * \brief Pops all the states of the machine until a specific one is reached + * \return true If that specific state is on top, false if stack is empty + * + * \param state State to find on the stack if it is nullptr, no action is performed + * + * \remark This method can completely empty the stack + */ + + inline bool StateMachine::PopStatesUntil(std::shared_ptr state) + { + if (!state) + return false; + + while (!m_states.empty() && !IsTopState(state.get())) + PopState(); + + return !m_states.empty(); + } + + /*! + * \brief Pushes a new state on the top of the machine + * + * \param state Next state to represent if it is nullptr, it performs no action + * + * \remark Produces a NazaraAssert if the same state is pushed two times on the stack + */ + + inline void StateMachine::PushState(std::shared_ptr state) + { + if (state) + { + NazaraAssert(std::find(m_states.begin(), m_states.end(), state) == m_states.end(), "The same state was pushed two times"); + + m_states.push_back(std::move(state)); + m_states.back()->Enter(*this); + } + } + + /*! + * \brief Pops every states of the machine to put a new one + * + * \param state State to reset the stack with if it is nullptr, no action is performed + */ + + inline void StateMachine::SetState(std::shared_ptr state) + { + if (state) + { + while (!m_states.empty()) + PopState(); + + PushState(std::move(state)); + } + } + + /*! + * \brief Updates all the states + * \return true If update is successful for everyone of them * * \param elapsedTime Delta time used for the update */ inline bool StateMachine::Update(float elapsedTime) { - if (m_nextState) - { - m_currentState->Leave(*this); - m_currentState = std::move(m_nextState); - m_currentState->Enter(*this); - } - - return m_currentState->Update(*this, elapsedTime); + return std::all_of(m_states.begin(), m_states.end(), [=](std::shared_ptr& state) { + return state->Update(*this, elapsedTime); + }); } } diff --git a/tests/SDK/NDK/StateMachine.cpp b/tests/SDK/NDK/StateMachine.cpp index 1c14b1e76..066a150d7 100644 --- a/tests/SDK/NDK/StateMachine.cpp +++ b/tests/SDK/NDK/StateMachine.cpp @@ -28,21 +28,79 @@ class TestState : public Ndk::State bool m_isUpdated; }; +class SecondTestState : public Ndk::State +{ + public: + void Enter(Ndk::StateMachine& /*fsm*/) override + { + m_isUpdated = false; + } + + bool IsUpdated() const + { + return m_isUpdated; + } + + void Leave(Ndk::StateMachine& /*fsm*/) override + { + } + + bool Update(Ndk::StateMachine& fsm, float /*elapsedTime*/) override + { + if (fsm.IsTopState(this)) + m_isUpdated = true; + return true; + } + + private: + bool m_isUpdated; +}; + SCENARIO("State & StateMachine", "[NDK][STATE]") { - GIVEN("A statemachine with our TestState") + GIVEN("A statemachine with our test states") { std::shared_ptr testState = std::make_shared(); - Ndk::StateMachine stateMachine(testState); + std::shared_ptr secondTestState = std::make_shared(); + Ndk::StateMachine stateMachine(secondTestState); + stateMachine.PushState(testState); REQUIRE(!testState->IsUpdated()); + REQUIRE(!secondTestState->IsUpdated()); WHEN("We update our machine") { stateMachine.Update(1.f); - THEN("Our state has been updated") + THEN("Our state on the top has been updated but not the bottom one") { + REQUIRE(stateMachine.IsTopState(testState.get())); + REQUIRE(!stateMachine.IsTopState(secondTestState.get())); + REQUIRE(testState->IsUpdated()); + REQUIRE(!secondTestState->IsUpdated()); + } + } + + WHEN("We exchange the states' positions while emptying the stack") + { + REQUIRE(stateMachine.PopStatesUntil(secondTestState)); + REQUIRE(stateMachine.IsTopState(secondTestState.get())); + + std::shared_ptr oldState = stateMachine.PopState(); + REQUIRE(stateMachine.PopState() == nullptr); + + stateMachine.SetState(testState); + stateMachine.PushState(oldState); + + stateMachine.Update(1.f); + + THEN("Both states should be updated") + { + REQUIRE(!stateMachine.IsTopState(testState.get())); + REQUIRE(stateMachine.IsTopState(secondTestState.get())); + + REQUIRE(testState->IsUpdated()); + REQUIRE(secondTestState->IsUpdated()); } } } From 33b10989e2da7f7c39ac68d618d513a6f9dc28ec Mon Sep 17 00:00:00 2001 From: Lynix Date: Fri, 21 Apr 2017 21:39:37 +0200 Subject: [PATCH 2/3] Physics2D/PhysWorld2D: Initialize callbacks to nullptr to prevent misuse --- include/Nazara/Physics2D/PhysWorld2D.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/Nazara/Physics2D/PhysWorld2D.hpp b/include/Nazara/Physics2D/PhysWorld2D.hpp index 4873e5642..30a3b1141 100644 --- a/include/Nazara/Physics2D/PhysWorld2D.hpp +++ b/include/Nazara/Physics2D/PhysWorld2D.hpp @@ -64,10 +64,10 @@ namespace Nz struct Callback { - ContactEndCallback endCallback; - ContactPreSolveCallback preSolveCallback; - ContactPostSolveCallback postSolveCallback; - ContactStartCallback startCallback; + ContactEndCallback endCallback = nullptr; + ContactPreSolveCallback preSolveCallback = nullptr; + ContactPostSolveCallback postSolveCallback = nullptr; + ContactStartCallback startCallback = nullptr; void* userdata; }; From 140e52203d25c0a5df6c6bf3efdd39dcd428a271 Mon Sep 17 00:00:00 2001 From: Lynix Date: Fri, 21 Apr 2017 21:48:05 +0200 Subject: [PATCH 3/3] Graphics/ForwardRenderQueue: Cleanup a bit --- include/Nazara/Graphics/ForwardRenderQueue.hpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/include/Nazara/Graphics/ForwardRenderQueue.hpp b/include/Nazara/Graphics/ForwardRenderQueue.hpp index f0ac6edda..218fd312d 100644 --- a/include/Nazara/Graphics/ForwardRenderQueue.hpp +++ b/include/Nazara/Graphics/ForwardRenderQueue.hpp @@ -73,7 +73,7 @@ namespace Nz std::vector billboards; }; - typedef std::map BatchedBillboardContainer; + using BatchedBillboardContainer = std::map; struct BatchedBillboardPipelineEntry { @@ -81,7 +81,7 @@ namespace Nz bool enabled = false; }; - typedef std::map BillboardPipelineBatches; + using BillboardPipelineBatches = std::map; /// Sprites struct SpriteChain_XYZ_Color_UV @@ -97,7 +97,7 @@ namespace Nz std::vector spriteChains; }; - typedef std::map SpriteOverlayBatches; + using SpriteOverlayBatches = std::map; struct BatchedBasicSpriteEntry { @@ -107,7 +107,7 @@ namespace Nz bool enabled = false; }; - typedef std::map SpriteMaterialBatches; + using SpriteMaterialBatches = std::map; struct BatchedSpritePipelineEntry { @@ -115,7 +115,7 @@ namespace Nz bool enabled = false; }; - typedef std::map SpritePipelineBatches; + using SpritePipelineBatches = std::map; /// Meshes struct MeshDataComparator @@ -132,7 +132,7 @@ namespace Nz Spheref squaredBoundingSphere; }; - typedef std::map MeshInstanceContainer; + using MeshInstanceContainer = std::map; struct BatchedModelEntry { @@ -142,7 +142,7 @@ namespace Nz bool enabled = false; }; - typedef std::map MeshMaterialBatches; + using MeshMaterialBatches = std::map; struct BatchedMaterialEntry { @@ -150,7 +150,7 @@ namespace Nz MeshMaterialBatches materialMap; }; - typedef std::map MeshPipelineBatches; + using MeshPipelineBatches = std::map; struct TransparentModelData { @@ -160,7 +160,7 @@ namespace Nz const Material* material; }; - typedef std::vector TransparentModelContainer; + using TransparentModelContainer = std::vector; struct Layer {