Updates to EMFX cameras and better support for character follow behavior (#7676)

* initial wip change to help resolve EMFX camera viewport issue

Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com>

* some updates to add/remove camera input API

Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com>

* add additional comments and remove some optimize off calls

Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com>

* improvements to camera behavior in EMFX editor

Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com>

* add some tests for add/remove logic for cameras

Signed-off-by: Tom Hulton-Harrop <82228511+hultonha@users.noreply.github.com>

Code/Editor/EditorModularViewportCameraComposer.cpp

@@ -287,16 +287,16 @@ namespace SandboxEditor
             return SandboxEditor::CameraBoostMultiplier();
         };
 
-        m_orbitDollyScrollCamera = AZStd::make_shared<AzFramework::OrbitDollyScrollCameraInput>();
+        m_orbitScrollDollyCamera = AZStd::make_shared<AzFramework::OrbitScrollDollyCameraInput>();
 
-        m_orbitDollyScrollCamera->m_scrollSpeedFn = []
+        m_orbitScrollDollyCamera->m_scrollSpeedFn = []
         {
             return SandboxEditor::CameraScrollSpeed();
         };
 
-        m_orbitDollyMoveCamera = AZStd::make_shared<AzFramework::OrbitDollyMotionCameraInput>(SandboxEditor::CameraOrbitDollyChannelId());
+        m_orbitMotionDollyCamera = AZStd::make_shared<AzFramework::OrbitMotionDollyCameraInput>(SandboxEditor::CameraOrbitDollyChannelId());
 
-        m_orbitDollyMoveCamera->m_motionSpeedFn = []
+        m_orbitMotionDollyCamera->m_motionSpeedFn = []
         {
             return SandboxEditor::CameraDollyMotionSpeed();
         };
@@ -326,8 +326,8 @@ namespace SandboxEditor
 
         m_orbitCamera->m_orbitCameras.AddCamera(m_orbitRotateCamera);
         m_orbitCamera->m_orbitCameras.AddCamera(m_orbitTranslateCamera);
-        m_orbitCamera->m_orbitCameras.AddCamera(m_orbitDollyScrollCamera);
-        m_orbitCamera->m_orbitCameras.AddCamera(m_orbitDollyMoveCamera);
+        m_orbitCamera->m_orbitCameras.AddCamera(m_orbitScrollDollyCamera);
+        m_orbitCamera->m_orbitCameras.AddCamera(m_orbitMotionDollyCamera);
         m_orbitCamera->m_orbitCameras.AddCamera(m_orbitPanCamera);
         m_orbitCamera->m_orbitCameras.AddCamera(m_orbitFocusCamera);
     }
@@ -344,7 +344,7 @@ namespace SandboxEditor
         m_orbitTranslateCamera->SetTranslateCameraInputChannelIds(translateCameraInputChannelIds);
         m_orbitPanCamera->SetPanInputChannelId(SandboxEditor::CameraOrbitPanChannelId());
         m_orbitRotateCamera->SetRotateInputChannelId(SandboxEditor::CameraOrbitLookChannelId());
-        m_orbitDollyMoveCamera->SetDollyInputChannelId(SandboxEditor::CameraOrbitDollyChannelId());
+        m_orbitMotionDollyCamera->SetDollyInputChannelId(SandboxEditor::CameraOrbitDollyChannelId());
         m_orbitFocusCamera->SetFocusInputChannelId(SandboxEditor::CameraFocusChannelId());
     }
 

Code/Editor/EditorModularViewportCameraComposer.h

@@ -56,8 +56,8 @@ namespace SandboxEditor
         AZStd::shared_ptr<AzFramework::OrbitCameraInput> m_orbitCamera;
         AZStd::shared_ptr<AzFramework::RotateCameraInput> m_orbitRotateCamera;
         AZStd::shared_ptr<AzFramework::TranslateCameraInput> m_orbitTranslateCamera;
-        AZStd::shared_ptr<AzFramework::OrbitDollyScrollCameraInput> m_orbitDollyScrollCamera;
-        AZStd::shared_ptr<AzFramework::OrbitDollyMotionCameraInput> m_orbitDollyMoveCamera;
+        AZStd::shared_ptr<AzFramework::OrbitScrollDollyCameraInput> m_orbitScrollDollyCamera;
+        AZStd::shared_ptr<AzFramework::OrbitMotionDollyCameraInput> m_orbitMotionDollyCamera;
         AZStd::shared_ptr<AzFramework::PanCameraInput> m_orbitPanCamera;
         AZStd::shared_ptr<AzFramework::FocusCameraInput> m_orbitFocusCamera;
 

Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.cpp

@@ -190,9 +190,67 @@ namespace AzFramework
         return nextCamera;
     }
 
-    void Cameras::AddCamera(AZStd::shared_ptr<CameraInput> cameraInput)
+    bool Cameras::AddCamera(AZStd::shared_ptr<CameraInput> cameraInput)
     {
-        m_idleCameraInputs.push_back(AZStd::move(cameraInput));
+        const auto idleCameraIt = AZStd::find(m_idleCameraInputs.begin(), m_idleCameraInputs.end(), cameraInput);
+        const auto activeCameraIt = AZStd::find(m_activeCameraInputs.begin(), m_activeCameraInputs.end(), cameraInput);
+
+        if (idleCameraIt == m_idleCameraInputs.end() && activeCameraIt == m_activeCameraInputs.end())
+        {
+            m_idleCameraInputs.push_back(AZStd::move(cameraInput));
+            return true;
+        }
+
+        return false;
+    }
+
+    bool Cameras::AddCameras(const AZStd::vector<AZStd::shared_ptr<AzFramework::CameraInput>>& cameraInputs)
+    {
+        bool allAdded = true;
+        for (auto cameraInput : cameraInputs)
+        {
+            allAdded = AddCamera(AZStd::move(cameraInput)) && allAdded;
+        }
+        return allAdded;
+    }
+
+    bool Cameras::RemoveCamera(const AZStd::shared_ptr<CameraInput>& cameraInput)
+    {
+        if (const auto idleCameraIt = AZStd::find(m_idleCameraInputs.begin(), m_idleCameraInputs.end(), cameraInput);
+            idleCameraIt != m_idleCameraInputs.end())
+        {
+            const auto idleIndex = idleCameraIt - m_idleCameraInputs.begin();
+            using AZStd::swap;
+            swap(m_idleCameraInputs[idleIndex], m_idleCameraInputs[m_idleCameraInputs.size() - 1]);
+            m_idleCameraInputs.pop_back();
+
+            return true;
+        }
+
+        if (const auto activeCameraIt = AZStd::find(m_activeCameraInputs.begin(), m_activeCameraInputs.end(), cameraInput);
+            activeCameraIt != m_activeCameraInputs.end())
+        {
+            (*activeCameraIt)->Reset();
+
+            const auto activeIndex = activeCameraIt - m_idleCameraInputs.begin();
+            using AZStd::swap;
+            swap(m_activeCameraInputs[activeIndex], m_activeCameraInputs[m_activeCameraInputs.size() - 1]);
+            m_activeCameraInputs.pop_back();
+
+            return true;
+        }
+
+        return false;
+    }
+
+    bool Cameras::RemoveCameras(const AZStd::vector<AZStd::shared_ptr<AzFramework::CameraInput>>& cameraInputs)
+    {
+        bool allRemoved = true;
+        for (const auto& cameraInput : cameraInputs)
+        {
+            allRemoved = RemoveCamera(cameraInput) && allRemoved;
+        }
+        return allRemoved;
     }
 
     bool Cameras::HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, const float scrollDelta)
@@ -645,7 +703,7 @@ namespace AzFramework
         m_orbitChannelId = orbitChanneId;
     }
 
-    OrbitDollyScrollCameraInput::OrbitDollyScrollCameraInput()
+    OrbitScrollDollyCameraInput::OrbitScrollDollyCameraInput()
     {
         m_scrollSpeedFn = []() constexpr
         {
@@ -653,7 +711,7 @@ namespace AzFramework
         };
     }
 
-    bool OrbitDollyScrollCameraInput::HandleEvents(
+    bool OrbitScrollDollyCameraInput::HandleEvents(
         const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] const float scrollDelta)
     {
         if (const auto* scroll = AZStd::get_if<ScrollEvent>(&event))
@@ -691,7 +749,7 @@ namespace AzFramework
         return nextCamera;
     }
 
-    Camera OrbitDollyScrollCameraInput::StepCamera(
+    Camera OrbitScrollDollyCameraInput::StepCamera(
         const Camera& targetCamera,
         [[maybe_unused]] const ScreenVector& cursorDelta,
         const float scrollDelta,
@@ -702,7 +760,7 @@ namespace AzFramework
         return nextCamera;
     }
 
-    OrbitDollyMotionCameraInput::OrbitDollyMotionCameraInput(const InputChannelId& dollyChannelId)
+    OrbitMotionDollyCameraInput::OrbitMotionDollyCameraInput(const InputChannelId& dollyChannelId)
         : m_dollyChannelId(dollyChannelId)
     {
         m_motionSpeedFn = []() constexpr
@@ -711,14 +769,14 @@ namespace AzFramework
         };
     }
 
-    bool OrbitDollyMotionCameraInput::HandleEvents(
+    bool OrbitMotionDollyCameraInput::HandleEvents(
         const InputEvent& event, [[maybe_unused]] const ScreenVector& cursorDelta, [[maybe_unused]] const float scrollDelta)
     {
         HandleActivationEvents(event, m_dollyChannelId, cursorDelta, m_clickDetector, *this);
         return CameraInputUpdatingAfterMotion(*this);
     }
 
-    Camera OrbitDollyMotionCameraInput::StepCamera(
+    Camera OrbitMotionDollyCameraInput::StepCamera(
         const Camera& targetCamera,
         const ScreenVector& cursorDelta,
         [[maybe_unused]] const float scrollDelta,
@@ -727,7 +785,7 @@ namespace AzFramework
         return OrbitDolly(targetCamera, aznumeric_cast<float>(cursorDelta.m_y) * m_motionSpeedFn());
     }
 
-    void OrbitDollyMotionCameraInput::SetDollyInputChannelId(const InputChannelId& dollyChannelId)
+    void OrbitMotionDollyCameraInput::SetDollyInputChannelId(const InputChannelId& dollyChannelId)
     {
         m_dollyChannelId = dollyChannelId;
     }

Code/Framework/AzFramework/AzFramework/Viewport/CameraInput.h

@@ -289,7 +289,17 @@ namespace AzFramework
 
         //! Add a camera input (behavior) to run in this set of camera inputs.
         //! The camera inputs added here will determine the overall behavior of the camera.
-        void AddCamera(AZStd::shared_ptr<CameraInput> cameraInput);
+        //! @return Returns if the camera was successfully added (if the camera already exists it is not added and AddCamera returns false).
+        bool AddCamera(AZStd::shared_ptr<CameraInput> cameraInput);
+        //! Add a collection of camera inputs (behaviors) to run in this set of camera inputs.
+        //! @return Returns if all cameras were added successfully.
+        bool AddCameras(const AZStd::vector<AZStd::shared_ptr<AzFramework::CameraInput>>& cameraInputs);
+        //! Remove a camera input (behavior) to stop it running in the set of camera inputs.
+        //! @return Returns if the camera was removed successfully (if the could not be found RemoveCamera returns false).
+        bool RemoveCamera(const AZStd::shared_ptr<CameraInput>& cameraInput);
+        //! Remove a collection of camera inputs (behaviors) to stop them running in the set of camera inputs.
+        //! @return Returns if all cameras were removed successfully.
+        bool RemoveCameras(const AZStd::vector<AZStd::shared_ptr<AzFramework::CameraInput>>& cameraInputs);
         //! Reset the state of all cameras.
         void Reset();
         //! Remove all cameras that were added.
@@ -299,9 +309,10 @@ namespace AzFramework
         bool Exclusive() const;
 
     private:
-        AZStd::vector<AZStd::shared_ptr<CameraInput>> m_activeCameraInputs; //!< Active camera inputs updating the camera (empty initially).
-        AZStd::vector<AZStd::shared_ptr<CameraInput>>
-            m_idleCameraInputs; //!< Idle camera inputs not contributing to the update (filled initially).
+        //! Active camera inputs updating the camera (empty initially).
+        AZStd::vector<AZStd::shared_ptr<CameraInput>> m_activeCameraInputs;
+        //! Idle camera inputs not contributing to the update (filled initially).
+        AZStd::vector<AZStd::shared_ptr<CameraInput>> m_idleCameraInputs;
     };
 
     //! Responsible for updating a series of cameras given various inputs.
@@ -586,10 +597,10 @@ namespace AzFramework
     };
 
     //! A camera input to handle discrete scroll events that can modify the camera offset.
-    class OrbitDollyScrollCameraInput : public CameraInput
+    class OrbitScrollDollyCameraInput : public CameraInput
     {
     public:
-        OrbitDollyScrollCameraInput();
+        OrbitScrollDollyCameraInput();
 
         // CameraInput overrides ...
         bool HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;
@@ -599,10 +610,10 @@ namespace AzFramework
     };
 
     //! A camera input to handle motion deltas that can modify the camera offset.
-    class OrbitDollyMotionCameraInput : public CameraInput
+    class OrbitMotionDollyCameraInput : public CameraInput
     {
     public:
-        explicit OrbitDollyMotionCameraInput(const InputChannelId& dollyChannelId);
+        explicit OrbitMotionDollyCameraInput(const InputChannelId& dollyChannelId);
 
         // CameraInput overrides ...
         bool HandleEvents(const InputEvent& event, const ScreenVector& cursorDelta, float scrollDelta) override;

Code/Framework/AzFramework/Tests/CameraInputTests.cpp

@@ -490,4 +490,40 @@ namespace UnitTest
         EXPECT_THAT(m_orbitCamera->Ending(), IsFalse());
         EXPECT_THAT(m_orbitCamera->Idle(), IsTrue());
     }
+
+    TEST_F(CameraInputFixture, NewCameraInputCanBeAddedToCameraSystem)
+    {
+        auto firstPersonPanCamera = AZStd::make_shared<AzFramework::PanCameraInput>(
+            AzFramework::InputDeviceMouse::Button::Middle, AzFramework::LookPan, AzFramework::TranslatePivotLook);
+        const bool added =
+            m_cameraSystem->m_cameras.AddCameras(AZStd::vector<AZStd::shared_ptr<AzFramework::CameraInput>>{ firstPersonPanCamera });
+
+        EXPECT_THAT(added, ::testing::IsTrue());
+    }
+
+    TEST_F(CameraInputFixture, ExistingCameraInputCannotBeAddedToCameraSystem)
+    {
+        const bool added =
+            m_cameraSystem->m_cameras.AddCameras(AZStd::vector<AZStd::shared_ptr<AzFramework::CameraInput>>{ m_firstPersonRotateCamera });
+
+        EXPECT_THAT(added, ::testing::IsFalse());
+    }
+
+    TEST_F(CameraInputFixture, ExistingCameraInputCanBeRemovedFromCameraSystem)
+    {
+        const bool removed = m_cameraSystem->m_cameras.RemoveCameras(
+            AZStd::vector<AZStd::shared_ptr<AzFramework::CameraInput>>{ m_firstPersonRotateCamera });
+
+        EXPECT_THAT(removed, ::testing::IsTrue());
+    }
+
+    TEST_F(CameraInputFixture, NonExistentCameraInputCannotBeRemovedFromCameraSystem)
+    {
+        auto firstPersonPanCamera = AZStd::make_shared<AzFramework::PanCameraInput>(
+            AzFramework::InputDeviceMouse::Button::Middle, AzFramework::LookPan, AzFramework::TranslatePivotLook);
+        const bool removed = m_cameraSystem->m_cameras.RemoveCameras(
+            AZStd::vector<AZStd::shared_ptr<AzFramework::CameraInput>>{ firstPersonPanCamera });
+
+        EXPECT_THAT(removed, ::testing::IsFalse());
+    }
 } // namespace UnitTest