Misc Bugfixes (#6530)

* Bugfix: AreaIds don't compare correctly when the priority bus doesn't have a listener.

Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>
(cherry picked from commit eb9e95fcb3884dee638b9032785b828359b0d8c0)

* Bugfix: Only deactivate a component if it's currently active.

Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>
(cherry picked from commit 7c9e4b7de49e8fe009616651d6e09f66c356db38)

* Switched shared_lock to unique_lock, since the data is being written to.

Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>

* Moved Legacy::CryCommon build dependency to correct location.
It was a public build dependency in FastNoise and GradientSignal, but really only should be a private dependency in Vegetation due to the vegetation system needing some init/shutdown events.
Fixing this up also required cleanup of a few unused legacy remnants in other files.

Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>

* InWorldBounds was incorrectly checking the X bounds against the Z bounds.

Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>

* Removed some additional unneeded legacy code that was now causing compile errors.

Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>

* Addressed PR feedback.

Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>

* Adjusted cmake dependencies again, based on what's needed in the public header files in the gems.

Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>

* Second attempt at the Deactivate() fix, this time by tracking the nested component state.

Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>

* Addressed PR feedback, made operation symmetrical.

Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>

* Added CryCommon privately to other veg modules too, since they all reference AreaSystemComponent, which uses the legacy events.

Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>
This commit is contained in:
Mike Balfour
2021-12-22 10:38:04 -06:00
committed by GitHub
parent 4aaf0defd7
commit aafb9e8110
11 changed files with 39 additions and 52 deletions
@@ -10,7 +10,6 @@
#include <AzCore/Serialization/SerializeContext.h>
#include <AzCore/RTTI/BehaviorContext.h>
#include <AzCore/Component/Entity.h>
#include <MathConversion.h>
#include <LmbrCentral/Rendering/MeshAsset.h>
#include <LmbrCentral/Animation/AttachmentComponentBus.h>
#include <LmbrCentral/Animation/SkeletalHierarchyRequestBus.h>
@@ -17,8 +17,6 @@
#include <SurfaceData/SurfaceDataSystemRequestBus.h>
#include <SurfaceData/Utility/SurfaceDataUtility.h>
#include <MathConversion.h>
namespace SurfaceData
{
void SurfaceDataMeshConfig::Reflect(AZ::ReflectContext* context)
+3 -1
View File
@@ -18,9 +18,11 @@ ly_add_target(
Include
BUILD_DEPENDENCIES
PUBLIC
Legacy::CryCommon
Gem::GradientSignal
PUBLIC
AZ::AzCore
PRIVATE
AZ::AzFramework
Gem::LmbrCentral
)
@@ -7,9 +7,6 @@
*/
#include <AzTest/AzTest.h>
#include <Mocks/ICryPakMock.h>
#include <Mocks/IConsoleMock.h>
#include <Mocks/ISystemMock.h>
#include <AzCore/Component/ComponentApplication.h>
#include <AzCore/Component/Entity.h>
@@ -101,28 +98,6 @@ public:
void SetAdvancedMode([[maybe_unused]] bool value) override {}
};
struct MockGlobalEnvironment
{
MockGlobalEnvironment()
{
m_stubEnv.pCryPak = &m_stubPak;
m_stubEnv.pConsole = &m_stubConsole;
m_stubEnv.pSystem = &m_stubSystem;
gEnv = &m_stubEnv;
}
~MockGlobalEnvironment()
{
gEnv = nullptr;
}
private:
SSystemGlobalEnvironment m_stubEnv;
testing::NiceMock<CryPakMock> m_stubPak;
testing::NiceMock<ConsoleMock> m_stubConsole;
testing::NiceMock<SystemMock> m_stubSystem;
};
TEST(FastNoiseTest, ComponentsWithComponentApplication)
{
AZ::ComponentApplication::Descriptor appDesc;
@@ -130,8 +105,6 @@ TEST(FastNoiseTest, ComponentsWithComponentApplication)
appDesc.m_recordingMode = AZ::Debug::AllocationRecords::RECORD_FULL;
appDesc.m_stackRecordLevels = 20;
MockGlobalEnvironment mocks;
AZ::ComponentApplication app;
AZ::Entity* systemEntity = app.Create(appDesc);
ASSERT_TRUE(systemEntity != nullptr);
@@ -186,7 +159,6 @@ public:
AZ::ComponentApplication m_application;
AZ::Entity* m_systemEntity;
MockGlobalEnvironment m_mocks;
};
//////////////////////////////////////////////////////////////////////////
+11 -4
View File
@@ -18,10 +18,11 @@ ly_add_target(
Include
BUILD_DEPENDENCIES
PUBLIC
Legacy::CryCommon
AZ::AzCore
AZ::AtomCore
AZ::AzFramework
Gem::SurfaceData
Gem::ImageProcessingAtom.Headers
PRIVATE
Gem::LmbrCentral
)
@@ -37,9 +38,11 @@ ly_add_target(
Include
BUILD_DEPENDENCIES
PRIVATE
Gem::GradientSignal.Static
Gem::LmbrCentral
PUBLIC
AZ::AzCore
AZ::AtomCore
Gem::GradientSignal.Static
Gem::ImageProcessingAtom.Headers # Atom/ImageProcessing/PixelFormats.h is part of a header in Includes
RUNTIME_DEPENDENCIES
Gem::LmbrCentral
@@ -69,7 +72,9 @@ if(PAL_TRAIT_BUILD_HOST_TOOLS)
Gem::LmbrCentral.Editor
PUBLIC
3rdParty::Qt::Widgets
Legacy::CryCommon
AZ::AzCore
AZ::AtomCore
AZ::AzFramework
AZ::AzToolsFramework
AZ::AssetBuilderSDK
Gem::GradientSignal.Static
@@ -92,6 +97,8 @@ if(PAL_TRAIT_BUILD_HOST_TOOLS)
PRIVATE
Gem::GradientSignal.Editor.Static
Gem::LmbrCentral.Editor
PUBLIC
AZ::AtomCore
RUNTIME_DEPENDENCIES
Gem::LmbrCentral.Editor
)
@@ -14,11 +14,6 @@
#include <AzCore/Math/Transform.h>
#include <LmbrCentral/Shape/ShapeComponentBus.h>
namespace LmbrCentral
{
class MeshAsset;
}
namespace GradientSignal
{
enum class WrappingType : AZ::u8
@@ -56,6 +56,7 @@ namespace LmbrCentral
TComponent m_component;
TConfiguration m_configuration;
bool m_visible = true;
bool m_runtimeComponentActive = false;
};
} // namespace LmbrCentral
@@ -177,6 +177,7 @@ namespace LmbrCentral
void EditorWrappedComponentBase<TComponent, TConfiguration>::Init()
{
AzToolsFramework::Components::EditorComponentBase::Init();
m_runtimeComponentActive = false;
m_component.ReadInConfig(&m_configuration);
m_component.Init();
}
@@ -196,6 +197,7 @@ namespace LmbrCentral
if (m_visible)
{
m_component.Activate();
m_runtimeComponentActive = true;
}
}
@@ -205,8 +207,10 @@ namespace LmbrCentral
AzToolsFramework::EditorVisibilityNotificationBus::Handler::BusDisconnect();
AzToolsFramework::Components::EditorComponentBase::Deactivate();
m_runtimeComponentActive = false;
m_component.Deactivate();
m_component.SetEntity(nullptr); // remove the entity association, in case the parent component is being removed, otherwise the component will be reactivated
// remove the entity association, in case the parent component is being removed, otherwise the component will be reactivated
m_component.SetEntity(nullptr);
}
template <typename TComponent, typename TConfiguration>
@@ -222,12 +226,18 @@ namespace LmbrCentral
template <typename TComponent, typename TConfiguration>
AZ::u32 EditorWrappedComponentBase<TComponent, TConfiguration>::ConfigurationChanged()
{
m_component.Deactivate();
if (m_runtimeComponentActive)
{
m_runtimeComponentActive = false;
m_component.Deactivate();
}
m_component.ReadInConfig(&m_configuration);
if (m_visible && m_component.GetEntity())
if (m_visible && !m_runtimeComponentActive)
{
m_component.Activate();
m_runtimeComponentActive = true;
}
return AZ::Edit::PropertyRefreshLevels::None;
@@ -9,8 +9,6 @@
#include <AzCore/Asset/AssetCommon.h>
#include <smartptr.h>
namespace LmbrCentral
{
class MeshAsset
@@ -25,10 +25,12 @@ bool TerrainLayerPriorityComparator::operator()(const AZ::EntityId& layer1id, co
{
// Comparator for insertion/keylookup.
// Sorts into layer/priority order, highest priority first.
AZ::u32 priority1, layer1;
AZ::u32 priority1 = 0;
AZ::u32 layer1 = 0;
Terrain::TerrainSpawnerRequestBus::Event(layer1id, &Terrain::TerrainSpawnerRequestBus::Events::GetPriority, layer1, priority1);
AZ::u32 priority2, layer2;
AZ::u32 priority2 = 0;
AZ::u32 layer2 = 0;
Terrain::TerrainSpawnerRequestBus::Event(layer2id, &Terrain::TerrainSpawnerRequestBus::Events::GetPriority, layer2, priority2);
if (layer1 < layer2)
@@ -80,7 +82,7 @@ void TerrainSystem::Activate()
m_requestedSettings.m_systemActive = true;
{
AZStd::shared_lock<AZStd::shared_mutex> lock(m_areaMutex);
AZStd::unique_lock<AZStd::shared_mutex> lock(m_areaMutex);
m_registeredAreas.clear();
}
@@ -109,7 +111,7 @@ void TerrainSystem::Deactivate()
AzFramework::Terrain::TerrainDataRequestBus::Handler::BusDisconnect();
{
AZStd::shared_lock<AZStd::shared_mutex> lock(m_areaMutex);
AZStd::unique_lock<AZStd::shared_mutex> lock(m_areaMutex);
m_registeredAreas.clear();
}
@@ -163,8 +165,7 @@ void TerrainSystem::ClampPosition(float x, float y, AZ::Vector2& outPosition, AZ
bool TerrainSystem::InWorldBounds(float x, float y) const
{
const float zTestValue = m_currentSettings.m_worldBounds.GetMin().GetX() +
((m_currentSettings.m_worldBounds.GetMax().GetX() - m_currentSettings.m_worldBounds.GetMin().GetX()) / 2.0f);
const float zTestValue = m_currentSettings.m_worldBounds.GetMin().GetZ();
const AZ::Vector3 testValue{ x, y, zTestValue };
if (m_currentSettings.m_worldBounds.Contains(testValue))
{
+4
View File
@@ -24,6 +24,7 @@ ly_add_target(
PRIVATE
Gem::LmbrCentral
Gem::SurfaceData
Legacy::CryCommon
PUBLIC
Gem::AtomLyIntegration_CommonFeatures.Static
RUNTIME_DEPENDENCIES
@@ -43,6 +44,7 @@ ly_add_target(
BUILD_DEPENDENCIES
PRIVATE
Gem::Vegetation.Static
Legacy::CryCommon
RUNTIME_DEPENDENCIES
Gem::LmbrCentral
Gem::GradientSignal
@@ -72,6 +74,7 @@ if(PAL_TRAIT_BUILD_HOST_TOOLS)
PRIVATE
Gem::Vegetation.Static
AZ::AzToolsFramework
Legacy::CryCommon
RUNTIME_DEPENDENCIES
Gem::LmbrCentral.Editor
Gem::GradientSignal.Editor
@@ -104,6 +107,7 @@ if(PAL_TRAIT_BUILD_TESTS_SUPPORTED)
AZ::AzFrameworkTestShared
Gem::Vegetation.Static
Gem::LmbrCentral.Mocks
Legacy::CryCommon
)
ly_add_googletest(
NAME Gem::Vegetation.Tests