From 91eb7476eb5f70d30fd28e61550625fefdabaa87 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Mon, 30 Aug 2021 14:47:35 -0700 Subject: [PATCH 01/19] Remove superfluous AutoPacket params Signed-off-by: puvvadar --- .../Code/Source/AutoGen/Multiplayer.AutoPackets.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/AutoGen/Multiplayer.AutoPackets.xml b/Gems/Multiplayer/Code/Source/AutoGen/Multiplayer.AutoPackets.xml index 203750d761..c553bc5351 100644 --- a/Gems/Multiplayer/Code/Source/AutoGen/Multiplayer.AutoPackets.xml +++ b/Gems/Multiplayer/Code/Source/AutoGen/Multiplayer.AutoPackets.xml @@ -32,11 +32,11 @@ - + - + From de512db010c65d667b3c485c20db7aa073c635b3 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Wed, 1 Sep 2021 16:26:56 -0700 Subject: [PATCH 02/19] Determine blended timestamp on client plus minor API cleanup Signed-off-by: puvvadar --- .../Code/Include/Multiplayer/IMultiplayer.h | 6 ++---- .../Include/Multiplayer/NetworkTime/INetworkTime.h | 7 ++----- .../Components/LocalPredictionPlayerInputComponent.cpp | 10 +++++----- .../Code/Source/NetworkTime/NetworkTime.cpp | 8 ++------ Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.h | 3 +-- 5 files changed, 12 insertions(+), 22 deletions(-) diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h b/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h index 4f714068db..e32b6188eb 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/IMultiplayer.h @@ -193,15 +193,13 @@ namespace Multiplayer m_previousHostFrameId = time->GetHostFrameId(); m_previousHostTimeMs = time->GetHostTimeMs(); m_previousRewindConnectionId = time->GetRewindingConnectionId(); - time->AlterTime(frameId, timeMs, connectionId); m_previousBlendFactor = time->GetHostBlendFactor(); - time->AlterBlendFactor(blendFactor); + time->AlterTime(frameId, timeMs, blendFactor, connectionId); } inline ~ScopedAlterTime() { INetworkTime* time = GetNetworkTime(); - time->AlterTime(m_previousHostFrameId, m_previousHostTimeMs, m_previousRewindConnectionId); - time->AlterBlendFactor(m_previousBlendFactor); + time->AlterTime(m_previousHostFrameId, m_previousHostTimeMs, m_previousBlendFactor, m_previousRewindConnectionId); } private: HostFrameId m_previousHostFrameId = InvalidHostFrameId; diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/INetworkTime.h b/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/INetworkTime.h index ae47aa8373..bfddcbdebb 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/INetworkTime.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/INetworkTime.h @@ -66,12 +66,9 @@ namespace Multiplayer //! Alters the current HostFrameId and binds that alteration to the provided ConnectionId. //! @param frameId the new HostFrameId to use //! @param timeMs the new HostTimeMs to use + //! @param blendFactor the factor used to blend between values at the current and previous HostFrameId //! @param rewindConnectionId the rewinding ConnectionId - virtual void AlterTime(HostFrameId frameId, AZ::TimeMs timeMs, AzNetworking::ConnectionId rewindConnectionId) = 0; - - //! Alters the current Host blend factor. Used to drive interpolation in rewound states. - //! @param blendFactor the blend factor to use - virtual void AlterBlendFactor(float blendFactor) = 0; + virtual void AlterTime(HostFrameId frameId, AZ::TimeMs timeMs, float blendFactor, AzNetworking::ConnectionId rewindConnectionId) = 0; //! Syncs all entities contained within a volume to the current rewind state. //! @param rewindVolume the volume to rewind entities within (needed for physics entities) diff --git a/Gems/Multiplayer/Code/Source/Components/LocalPredictionPlayerInputComponent.cpp b/Gems/Multiplayer/Code/Source/Components/LocalPredictionPlayerInputComponent.cpp index 452cb31a7a..f33c7f4ef3 100644 --- a/Gems/Multiplayer/Code/Source/Components/LocalPredictionPlayerInputComponent.cpp +++ b/Gems/Multiplayer/Code/Source/Components/LocalPredictionPlayerInputComponent.cpp @@ -179,12 +179,9 @@ namespace Multiplayer // Discard move input events, client may be speed hacking if (m_clientBankedTime < sv_MaxBankTimeWindowSec) { - // Client blends from previous frame to target so here we subtract blend factor to get to that state - const float blendFactor = AZStd::min(AZStd::max(0.f, input.GetHostBlendFactor()), 1.0f); - const AZ::TimeMs blendMs = AZ::TimeMs(static_cast(static_cast(cl_InputRateMs)) * (1.0f - blendFactor)); m_clientBankedTime = AZStd::min(m_clientBankedTime + clientInputRateSec, (double)sv_MaxBankTimeWindowSec); // clamp to boundary { - ScopedAlterTime scopedTime(input.GetHostFrameId(), input.GetHostTimeMs() - blendMs, input.GetHostBlendFactor(), invokingConnection->GetConnectionId()); + ScopedAlterTime scopedTime(input.GetHostFrameId(), input.GetHostTimeMs(), input.GetHostBlendFactor(), invokingConnection->GetConnectionId()); GetNetBindComponent()->ProcessInput(input, static_cast(clientInputRateSec)); } @@ -430,10 +427,13 @@ namespace Multiplayer NetworkInputArray inputArray(GetEntityHandle()); NetworkInput& input = inputArray[0]; + const float blendFactor = AZStd::min(AZStd::max(0.f, multiplayer->GetCurrentBlendFactor()), 1.0f); + const AZ::TimeMs blendMs = AZ::TimeMs(static_cast(static_cast(cl_InputRateMs)) * (1.0f - blendFactor)); input.SetClientInputId(m_clientInputId); input.SetHostFrameId(networkTime->GetHostFrameId()); - input.SetHostTimeMs(multiplayer->GetCurrentHostTimeMs()); + // Account for the client blending from previous frame to current + input.SetHostTimeMs(multiplayer->GetCurrentHostTimeMs() - blendMs); input.SetHostBlendFactor(multiplayer->GetCurrentBlendFactor()); // Allow components to form the input for this frame diff --git a/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.cpp b/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.cpp index db8d6bd2f7..69c97d9c3f 100644 --- a/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.cpp +++ b/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.cpp @@ -79,16 +79,12 @@ namespace Multiplayer m_rewindingConnectionId = AzNetworking::InvalidConnectionId; } - void NetworkTime::AlterTime(HostFrameId frameId, AZ::TimeMs timeMs, AzNetworking::ConnectionId rewindConnectionId) + void NetworkTime::AlterTime(HostFrameId frameId, AZ::TimeMs timeMs, float blendFactor, AzNetworking::ConnectionId rewindConnectionId) { m_hostFrameId = frameId; m_hostTimeMs = timeMs; - m_rewindingConnectionId = rewindConnectionId; - } - - void NetworkTime::AlterBlendFactor(float blendFactor) - { m_hostBlendFactor = blendFactor; + m_rewindingConnectionId = rewindConnectionId; } void NetworkTime::SyncEntitiesToRewindState(const AZ::Aabb& rewindVolume) diff --git a/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.h b/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.h index 0278ddd2b0..7c845db8b6 100644 --- a/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.h +++ b/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.h @@ -34,8 +34,7 @@ namespace Multiplayer AzNetworking::ConnectionId GetRewindingConnectionId() const override; HostFrameId GetHostFrameIdForRewindingConnection(AzNetworking::ConnectionId rewindConnectionId) const override; void ForceSetTime(HostFrameId frameId, AZ::TimeMs timeMs) override; - void AlterTime(HostFrameId frameId, AZ::TimeMs timeMs, AzNetworking::ConnectionId rewindConnectionId) override; - void AlterBlendFactor(float blendFactor) override; + void AlterTime(HostFrameId frameId, AZ::TimeMs timeMs, float blendFactor, AzNetworking::ConnectionId rewindConnectionId) override; void SyncEntitiesToRewindState(const AZ::Aabb& rewindVolume) override; void ClearRewoundEntities() override; //! @} From c5d8c194dfced4ef22bc74a4a32402b8dd14d3a6 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Thu, 2 Sep 2021 14:24:46 -0700 Subject: [PATCH 03/19] Cleaning up NetworkTime and reworking how we approach network interpolation Signed-off-by: puvvadar --- .../Multiplayer/Components/NetBindComponent.h | 4 +- .../Components/NetworkTransformComponent.h | 17 +--- .../Source/AutoGen/AutoComponent_Header.jinja | 15 ++-- .../Source/AutoGen/AutoComponent_Source.jinja | 19 ++-- ...etworkTransformComponent.AutoComponent.xml | 4 +- .../Source/Components/NetBindComponent.cpp | 4 +- .../Components/NetworkTransformComponent.cpp | 90 ++++++------------- .../Source/MultiplayerSystemComponent.cpp | 4 +- .../Code/Source/NetworkTime/NetworkTime.cpp | 11 ++- 9 files changed, 66 insertions(+), 102 deletions(-) diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetBindComponent.h b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetBindComponent.h index 65bac09726..d09de1ad0d 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetBindComponent.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetBindComponent.h @@ -35,7 +35,7 @@ namespace Multiplayer using EntityMigrationStartEvent = AZ::Event; using EntityMigrationEndEvent = AZ::Event<>; using EntityServerMigrationEvent = AZ::Event; - using EntityPreRenderEvent = AZ::Event; + using EntityPreRenderEvent = AZ::Event; using EntityCorrectionEvent = AZ::Event<>; //! @class NetBindComponent @@ -118,7 +118,7 @@ namespace Multiplayer void NotifyMigrationStart(ClientInputId migratedInputId); void NotifyMigrationEnd(); void NotifyServerMigration(HostId hostId, AzNetworking::ConnectionId connectionId); - void NotifyPreRender(float deltaTime, float blendFactor); + void NotifyPreRender(float deltaTime); void NotifyCorrection(); void AddEntityStopEventHandler(EntityStopEvent::Handler& eventHandler); diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkTransformComponent.h b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkTransformComponent.h index 914aeaadd3..a018279a79 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkTransformComponent.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/Components/NetworkTransformComponent.h @@ -29,24 +29,9 @@ namespace Multiplayer void OnDeactivate(Multiplayer::EntityIsMigrating entityIsMigrating) override; private: - void OnPreRender(float deltaTime, float blendFactor); + void OnPreRender(float deltaTime); void OnCorrection(); - void OnRotationChangedEvent(const AZ::Quaternion& rotation); - void OnTranslationChangedEvent(const AZ::Vector3& translation); - void OnScaleChangedEvent(float scale); - void OnResetCountChangedEvent(); - - void UpdateTargetHostFrameId(); - - AZ::Transform m_previousTransform = AZ::Transform::CreateIdentity(); - AZ::Transform m_targetTransform = AZ::Transform::CreateIdentity(); - - AZ::Event::Handler m_rotationEventHandler; - AZ::Event::Handler m_translationEventHandler; - AZ::Event::Handler m_scaleEventHandler; - AZ::Event::Handler m_resetCountEventHandler; - EntityPreRenderEvent::Handler m_entityPreRenderEventHandler; EntityCorrectionEvent::Handler m_entityCorrectionEventHandler; diff --git a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja index 8cde9613b7..815f7d0348 100644 --- a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja +++ b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Header.jinja @@ -7,21 +7,21 @@ {% macro DeclareNetworkPropertyGetter(Property) %} {% set PropertyName = UpperFirst(Property.attrib['Name']) %} {% if Property.attrib['Container'] == 'Array' %} -{% if Property.attrib['IsRewindable']|booleanTrue %} +{% if Property.attrib['IsRewindable']|booleanTrue %} const RewindableArray<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] }}>& Get{{ PropertyName }}Array() const; -{% else %} +{% else %} const AZStd::array<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] }}>& Get{{ PropertyName }}Array() const; -{% endif %} +{% endif %} const {{ Property.attrib['Type'] }}& Get{{ PropertyName }}(int32_t index) const; {% if Property.attrib['GenerateEventBindings']|booleanTrue %} void {{ PropertyName }}AddEvent(AZ::Event::Handler& handler); {% endif %} {% elif Property.attrib['Container'] == 'Vector' %} -{% if Property.attrib['IsRewindable']|booleanTrue %} +{% if Property.attrib['IsRewindable']|booleanTrue %} const RewindableFixedVector<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] }}>& Get{{ PropertyName }}Vector() const; -{% else %} +{% else %} const AZStd::fixed_vector<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] }}>& Get{{ PropertyName }}Vector() const; -{% endif %} +{% endif %} const {{ Property.attrib['Type'] }}& Get{{ PropertyName }}(int32_t index) const; const {{ Property.attrib['Type'] }}& {{ PropertyName }}GetBack() const; uint32_t {{ PropertyName }}GetSize() const; @@ -31,6 +31,9 @@ void {{ PropertyName }}SizeChangedAddEvent(AZ::Event::Handler& handler {% endif %} {% else %} const {{ Property.attrib['Type'] }}& Get{{ PropertyName }}() const; +{% if Property.attrib['IsRewindable']|booleanTrue %} +const {{ Property.attrib['Type'] }}& Get{{ PropertyName }}Previous() const; +{% endif %} {% if Property.attrib['GenerateEventBindings']|booleanTrue %} void {{ PropertyName }}AddEvent(AZ::Event<{{ Property.attrib['Type'] }}>::Handler& handler); {% endif %} diff --git a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Source.jinja b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Source.jinja index 079cac329b..a02235cc0f 100644 --- a/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Source.jinja +++ b/Gems/Multiplayer/Code/Source/AutoGen/AutoComponent_Source.jinja @@ -3,11 +3,11 @@ {% macro LowerFirst(text) %}{{ text[0] | lower}}{{ text[1:] }}{% endmacro %} {% macro DefineNetworkPropertyGet(ClassName, Property, Prefix = '') %} {% if Property.attrib['Container'] == 'Array' %} -{% if Property.attrib['IsRewindable']|booleanTrue %} +{% if Property.attrib['IsRewindable']|booleanTrue %} const RewindableArray<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] }}>& {{ ClassName }}::Get{{ UpperFirst(Property.attrib['Name']) }}Array() const -{% else %} +{% else %} const AZStd::array<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] }}>& {{ ClassName }}::Get{{ UpperFirst(Property.attrib['Name']) }}Array() const -{% endif %} +{% endif %} { return {{ Prefix }}m_{{ LowerFirst(Property.attrib['Name']) }}; } @@ -25,11 +25,11 @@ void {{ ClassName }}::{{ UpperFirst(Property.attrib['Name']) }}AddEvent(AZ::Even {% endif %} {% elif Property.attrib['Container'] == 'Vector' %} -{% if Property.attrib['IsRewindable']|booleanTrue %} +{% if Property.attrib['IsRewindable']|booleanTrue %} const RewindableFixedVector<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] }}>& {{ ClassName }}::Get{{ UpperFirst(Property.attrib['Name']) }}Vector() const -{% else %} +{% else %} const AZStd::fixed_vector<{{ Property.attrib['Type'] }}, {{ Property.attrib['Count'] }}>& {{ ClassName }}::Get{{ UpperFirst(Property.attrib['Name']) }}Vector() const -{% endif %} +{% endif %} { return {{ Prefix }}m_{{ LowerFirst(Property.attrib['Name']) }}; } @@ -68,7 +68,12 @@ const {{ Property.attrib['Type'] }}& {{ ClassName }}::Get{{ UpperFirst(Property. { return {{ Prefix }}m_{{ LowerFirst(Property.attrib['Name']) }}; } - +{% if Property.attrib['IsRewindable']|booleanTrue %} +const {{ Property.attrib['Type'] }}& {{ ClassName }}::Get{{ UpperFirst(Property.attrib['Name']) }}Previous() const +{ + return {{ Prefix }}m_{{ LowerFirst(Property.attrib['Name']) }}.GetPrevious(); +} +{% endif %} {% if Property.attrib['GenerateEventBindings']|booleanTrue %} void {{ ClassName }}::{{ UpperFirst(Property.attrib['Name']) }}AddEvent(AZ::Event<{{ Property.attrib['Type'] }}>::Handler& handler) { diff --git a/Gems/Multiplayer/Code/Source/AutoGen/NetworkTransformComponent.AutoComponent.xml b/Gems/Multiplayer/Code/Source/AutoGen/NetworkTransformComponent.AutoComponent.xml index cec005cc26..8ab2e61e5e 100644 --- a/Gems/Multiplayer/Code/Source/AutoGen/NetworkTransformComponent.AutoComponent.xml +++ b/Gems/Multiplayer/Code/Source/AutoGen/NetworkTransformComponent.AutoComponent.xml @@ -12,9 +12,9 @@ - + - + diff --git a/Gems/Multiplayer/Code/Source/Components/NetBindComponent.cpp b/Gems/Multiplayer/Code/Source/Components/NetBindComponent.cpp index d8e8a765ce..9c7d48e67a 100644 --- a/Gems/Multiplayer/Code/Source/Components/NetBindComponent.cpp +++ b/Gems/Multiplayer/Code/Source/Components/NetBindComponent.cpp @@ -403,9 +403,9 @@ namespace Multiplayer m_entityServerMigrationEvent.Signal(m_netEntityHandle, hostId, connectionId); } - void NetBindComponent::NotifyPreRender(float deltaTime, float blendFactor) + void NetBindComponent::NotifyPreRender(float deltaTime) { - m_entityPreRenderEvent.Signal(deltaTime, blendFactor); + m_entityPreRenderEvent.Signal(deltaTime); } void NetBindComponent::NotifyCorrection() diff --git a/Gems/Multiplayer/Code/Source/Components/NetworkTransformComponent.cpp b/Gems/Multiplayer/Code/Source/Components/NetworkTransformComponent.cpp index fa08794c4d..671228f89d 100644 --- a/Gems/Multiplayer/Code/Source/Components/NetworkTransformComponent.cpp +++ b/Gems/Multiplayer/Code/Source/Components/NetworkTransformComponent.cpp @@ -26,11 +26,7 @@ namespace Multiplayer } NetworkTransformComponent::NetworkTransformComponent() - : m_rotationEventHandler([this](const AZ::Quaternion& rotation) { OnRotationChangedEvent(rotation); }) - , m_translationEventHandler([this](const AZ::Vector3& translation) { OnTranslationChangedEvent(translation); }) - , m_scaleEventHandler([this](float scale) { OnScaleChangedEvent(scale); }) - , m_resetCountEventHandler([this](const uint8_t&) { OnResetCountChangedEvent(); }) - , m_entityPreRenderEventHandler([this](float deltaTime, float blendFactor) { OnPreRender(deltaTime, blendFactor); }) + : m_entityPreRenderEventHandler([this](float deltaTime) { OnPreRender(deltaTime); }) , m_entityCorrectionEventHandler([this]() { OnCorrection(); }) { ; @@ -43,15 +39,8 @@ namespace Multiplayer void NetworkTransformComponent::OnActivate([[maybe_unused]] Multiplayer::EntityIsMigrating entityIsMigrating) { - RotationAddEvent(m_rotationEventHandler); - TranslationAddEvent(m_translationEventHandler); - ScaleAddEvent(m_scaleEventHandler); - ResetCountAddEvent(m_resetCountEventHandler); GetNetBindComponent()->AddEntityPreRenderEventHandler(m_entityPreRenderEventHandler); GetNetBindComponent()->AddEntityCorrectionEventHandler(m_entityCorrectionEventHandler); - - // When coming into relevance, reset all blending factors so we don't interpolate to our start position - OnResetCountChangedEvent(); } void NetworkTransformComponent::OnDeactivate([[maybe_unused]] Multiplayer::EntityIsMigrating entityIsMigrating) @@ -59,59 +48,31 @@ namespace Multiplayer ; } - void NetworkTransformComponent::OnRotationChangedEvent(const AZ::Quaternion& rotation) - { - m_previousTransform.SetRotation(m_targetTransform.GetRotation()); - m_targetTransform.SetRotation(rotation); - UpdateTargetHostFrameId(); - } - - void NetworkTransformComponent::OnTranslationChangedEvent(const AZ::Vector3& translation) - { - m_previousTransform.SetTranslation(m_targetTransform.GetTranslation()); - m_targetTransform.SetTranslation(translation); - UpdateTargetHostFrameId(); - } - - void NetworkTransformComponent::OnScaleChangedEvent(float scale) - { - m_previousTransform.SetUniformScale(m_targetTransform.GetUniformScale()); - m_targetTransform.SetUniformScale(scale); - UpdateTargetHostFrameId(); - } - - void NetworkTransformComponent::OnResetCountChangedEvent() - { - m_targetTransform.SetRotation(GetRotation()); - m_targetTransform.SetTranslation(GetTranslation()); - m_targetTransform.SetUniformScale(GetScale()); - m_previousTransform = m_targetTransform; - } - - void NetworkTransformComponent::UpdateTargetHostFrameId() - { - HostFrameId currentHostFrameId = Multiplayer::GetNetworkTime()->GetHostFrameId(); - if (currentHostFrameId > m_targetHostFrameId) - { - m_targetHostFrameId = currentHostFrameId; - } - } - - void NetworkTransformComponent::OnPreRender([[maybe_unused]] float deltaTime, float blendFactor) + void NetworkTransformComponent::OnPreRender([[maybe_unused]] float deltaTime) { if (!HasController()) { AZ::Transform blendTransform; - if (Multiplayer::GetNetworkTime() && Multiplayer::GetNetworkTime()->GetHostFrameId() > m_targetHostFrameId) + blendTransform.SetRotation(GetRotation()); + blendTransform.SetTranslation(GetTranslation()); + blendTransform.SetUniformScale(GetScale()); + + const float blendFactor = GetNetworkTime()->GetHostBlendFactor(); + if (!!AZ::IsClose(blendFactor, 1.0f)) { - m_previousTransform = m_targetTransform; - blendTransform = m_targetTransform; - } - else - { - blendTransform.SetRotation(m_previousTransform.GetRotation().Slerp(m_targetTransform.GetRotation(), blendFactor)); - blendTransform.SetTranslation(m_previousTransform.GetTranslation().Lerp(m_targetTransform.GetTranslation(), blendFactor)); - blendTransform.SetUniformScale(AZ::Lerp(m_previousTransform.GetUniformScale(), m_targetTransform.GetUniformScale(), blendFactor)); + AZ::Transform blendTransformPrevious; + blendTransformPrevious.SetRotation(GetRotationPrevious()); + blendTransformPrevious.SetTranslation(GetTranslationPrevious()); + blendTransformPrevious.SetUniformScale(GetScalePrevious()); + + if (!blendTransform.IsClose(blendTransformPrevious)) + { + blendTransform.SetRotation(blendTransformPrevious.GetRotation().Slerp(blendTransform.GetRotation(), blendFactor)); + blendTransform.SetTranslation( + blendTransformPrevious.GetTranslation().Lerp(blendTransform.GetTranslation(), blendFactor)); + blendTransform.SetUniformScale( + AZ::Lerp(blendTransformPrevious.GetUniformScale(), blendTransform.GetUniformScale(), blendFactor)); + } } if (!GetTransformComponent()->GetWorldTM().IsClose(blendTransform)) @@ -124,12 +85,15 @@ namespace Multiplayer void NetworkTransformComponent::OnCorrection() { // Snap to latest - OnResetCountChangedEvent(); + AZ::Transform targetTransform; + targetTransform.SetRotation(GetRotation()); + targetTransform.SetTranslation(GetTranslation()); + targetTransform.SetUniformScale(GetScale()); // Hard set the entities transform - if (!GetTransformComponent()->GetWorldTM().IsClose(m_targetTransform)) + if (!GetTransformComponent()->GetWorldTM().IsClose(targetTransform)) { - GetTransformComponent()->SetWorldTM(m_targetTransform); + GetTransformComponent()->SetWorldTM(targetTransform); } } diff --git a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp index 2d17b072b3..da0c464061 100644 --- a/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp +++ b/Gems/Multiplayer/Code/Source/MultiplayerSystemComponent.cpp @@ -922,7 +922,7 @@ namespace Multiplayer for (NetBindComponent* netBindComponent : gatheredEntities) { - netBindComponent->NotifyPreRender(deltaTime, m_renderBlendFactor); + netBindComponent->NotifyPreRender(deltaTime); } } else @@ -934,7 +934,7 @@ namespace Multiplayer NetBindComponent* netBindComponent = entity->FindComponent(); if (netBindComponent != nullptr) { - netBindComponent->NotifyPreRender(deltaTime, m_renderBlendFactor); + netBindComponent->NotifyPreRender(deltaTime); } } } diff --git a/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.cpp b/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.cpp index 69c97d9c3f..8edf2a8ce4 100644 --- a/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.cpp +++ b/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.cpp @@ -110,8 +110,15 @@ namespace Multiplayer if (networkTransform != nullptr) { - // We're not presently factoring in interpolated position here - const AZ::Vector3 rewindCenter = networkTransform->GetTranslation(); // Get the rewound position + // Get the rewound position for target host frame ID plus the one preceding it for potential lerp + AZ::Vector3 rewindCenter = networkTransform->GetTranslation(); + const AZ::Vector3 rewindCenterPrevious = networkTransform->GetTranslationPrevious(); + const float blendFactor = GetNetworkTime()->GetHostBlendFactor(); + if (!AZ::IsClose(blendFactor, 1.0f) && !rewindCenter.IsClose(rewindCenterPrevious)) + { + // If we have a blend factor, lerp the translation for accuracy + rewindCenter = rewindCenterPrevious.Lerp(rewindCenter, blendFactor); + } const AZ::Vector3 rewindOffset = rewindCenter - currentCenter; // Compute offset between rewound and current positions const AZ::Aabb rewoundAabb = currentBounds.GetTranslated(rewindOffset); // Apply offset to the entity aabb From f2841f2eba67a37f73e2df29cca1f2d47b698770 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Fri, 17 Sep 2021 14:11:29 -0700 Subject: [PATCH 04/19] Fix interpolation logic errors in NetworkTransform Signed-off-by: puvvadar --- .../Code/Source/Components/NetworkTransformComponent.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gems/Multiplayer/Code/Source/Components/NetworkTransformComponent.cpp b/Gems/Multiplayer/Code/Source/Components/NetworkTransformComponent.cpp index 671228f89d..1d9fee16e0 100644 --- a/Gems/Multiplayer/Code/Source/Components/NetworkTransformComponent.cpp +++ b/Gems/Multiplayer/Code/Source/Components/NetworkTransformComponent.cpp @@ -57,8 +57,8 @@ namespace Multiplayer blendTransform.SetTranslation(GetTranslation()); blendTransform.SetUniformScale(GetScale()); - const float blendFactor = GetNetworkTime()->GetHostBlendFactor(); - if (!!AZ::IsClose(blendFactor, 1.0f)) + const float blendFactor = GetMultiplayer()->GetCurrentBlendFactor(); + if (!AZ::IsClose(blendFactor, 1.0f)) { AZ::Transform blendTransformPrevious; blendTransformPrevious.SetRotation(GetRotationPrevious()); From 67e2498a12c658ee39c5900d40918bf854105119 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Fri, 17 Sep 2021 14:43:14 -0700 Subject: [PATCH 05/19] Update GetPrevious to behave similarly to Get when rewound on owning connection Signed-off-by: puvvadar --- .../Multiplayer/NetworkTime/INetworkTime.h | 6 ------ .../NetworkTime/RewindableObject.h | 8 ++++++-- .../NetworkTime/RewindableObject.inl | 19 +++++++++++++++++-- .../Code/Source/NetworkTime/NetworkTime.cpp | 5 ----- .../Code/Source/NetworkTime/NetworkTime.h | 1 - 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/INetworkTime.h b/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/INetworkTime.h index bfddcbdebb..c12cbb660b 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/INetworkTime.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/INetworkTime.h @@ -52,12 +52,6 @@ namespace Multiplayer //! @return the ConnectionId of the connection requesting the rewind operation virtual AzNetworking::ConnectionId GetRewindingConnectionId() const = 0; - //! Get the controlling connection that may be currently altering global game time. - //! Note this abstraction is required at a relatively high level to allow for 'don't rewind the shooter' semantics - //! @param rewindConnectionId if this parameter matches the current rewindConnectionId, it will return the unaltered hostFrameId - //! @return the HostFrameId taking into account the provided rewinding connectionId - virtual HostFrameId GetHostFrameIdForRewindingConnection(AzNetworking::ConnectionId rewindConnectionId) const = 0; - //! Forcibly sets the current network time to the provided frameId and game time in milliseconds. //! @param frameId the new HostFrameId to use //! @param timeMs the new HostTimeMs to use diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/RewindableObject.h b/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/RewindableObject.h index a8af564c15..152f6f47a7 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/RewindableObject.h +++ b/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/RewindableObject.h @@ -60,7 +60,7 @@ namespace Multiplayer //! @return value in const base type form const BASE_TYPE& Get() const; - //! Const base type retriever for one host frame behind Get(). Only intended for use in SyncRewind contexts. + //! Const base type retriever for one host frame behind Get() when contextually appropriate, otherwise identical to Get(). //! @return value in const base type form const BASE_TYPE& GetPrevious() const; @@ -86,9 +86,13 @@ namespace Multiplayer private: //! Returns what the appropriate current time is for this rewindable property. - //! @return the appropriate current time is for this rewindable property + //! @return the appropriate current time for this rewindable property HostFrameId GetCurrentTimeForProperty() const; + //! Returns what the appropriate previous time is for this rewindable property. + //! @return the appropriate previous time for this rewindable property + HostFrameId GetPreviousTimeForProperty() const; + //! Updates the latest value for this object instance, if frameTime represents a current or future time. //! Any attempts to set old values on the object will fail //! @param value the new value to set in the object history diff --git a/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/RewindableObject.inl b/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/RewindableObject.inl index b0c9bc0c46..9183a1e9da 100644 --- a/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/RewindableObject.inl +++ b/Gems/Multiplayer/Code/Include/Multiplayer/NetworkTime/RewindableObject.inl @@ -69,7 +69,7 @@ namespace Multiplayer template inline const BASE_TYPE& RewindableObject::GetPrevious() const { - return GetValueForTime(GetCurrentTimeForProperty() - HostFrameId(1)); + return GetValueForTime(GetPreviousTimeForProperty()); } template @@ -118,7 +118,22 @@ namespace Multiplayer inline HostFrameId RewindableObject::GetCurrentTimeForProperty() const { INetworkTime* networkTime = Multiplayer::GetNetworkTime(); - return networkTime->GetHostFrameIdForRewindingConnection(m_owningConnectionId); + if (networkTime->IsTimeRewound() && (m_owningConnectionId == networkTime->GetRewindingConnectionId())) + { + return networkTime->GetUnalteredHostFrameId(); + } + return networkTime->GetHostFrameId(); + } + + template + inline HostFrameId RewindableObject::GetPreviousTimeForProperty() const + { + INetworkTime* networkTime = Multiplayer::GetNetworkTime(); + if (networkTime->IsTimeRewound() && (m_owningConnectionId == networkTime->GetRewindingConnectionId())) + { + return networkTime->GetUnalteredHostFrameId(); + } + return networkTime->GetHostFrameId() - HostFrameId(1); } template diff --git a/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.cpp b/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.cpp index 8edf2a8ce4..0838a1ef3d 100644 --- a/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.cpp +++ b/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.cpp @@ -65,11 +65,6 @@ namespace Multiplayer return m_rewindingConnectionId; } - HostFrameId NetworkTime::GetHostFrameIdForRewindingConnection(AzNetworking::ConnectionId rewindConnectionId) const - { - return (IsTimeRewound() && (rewindConnectionId == m_rewindingConnectionId)) ? m_unalteredFrameId : m_hostFrameId; - } - void NetworkTime::ForceSetTime(HostFrameId frameId, AZ::TimeMs timeMs) { AZ_Assert(!IsTimeRewound(), "Forcibly setting network time is unsupported under a rewound time scope"); diff --git a/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.h b/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.h index 7c845db8b6..2bcf019623 100644 --- a/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.h +++ b/Gems/Multiplayer/Code/Source/NetworkTime/NetworkTime.h @@ -32,7 +32,6 @@ namespace Multiplayer AZ::TimeMs GetHostTimeMs() const override; float GetHostBlendFactor() const override; AzNetworking::ConnectionId GetRewindingConnectionId() const override; - HostFrameId GetHostFrameIdForRewindingConnection(AzNetworking::ConnectionId rewindConnectionId) const override; void ForceSetTime(HostFrameId frameId, AZ::TimeMs timeMs) override; void AlterTime(HostFrameId frameId, AZ::TimeMs timeMs, float blendFactor, AzNetworking::ConnectionId rewindConnectionId) override; void SyncEntitiesToRewindState(const AZ::Aabb& rewindVolume) override; From 3e8fe7df395f3d1d9e356d96693acd269ef29cd0 Mon Sep 17 00:00:00 2001 From: puvvadar Date: Fri, 17 Sep 2021 15:41:54 -0700 Subject: [PATCH 06/19] Add unit tests for RewindableObject Get and GetPrevious Signed-off-by: puvvadar --- .../Code/Tests/RewindableObjectTests.cpp | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/Gems/Multiplayer/Code/Tests/RewindableObjectTests.cpp b/Gems/Multiplayer/Code/Tests/RewindableObjectTests.cpp index b8d60a94e8..04de971f0d 100644 --- a/Gems/Multiplayer/Code/Tests/RewindableObjectTests.cpp +++ b/Gems/Multiplayer/Code/Tests/RewindableObjectTests.cpp @@ -57,6 +57,35 @@ namespace UnitTest } } + TEST_F(RewindableObjectTests, CurrentPreviousTests) + { + Multiplayer::RewindableObject test(0); + + for (uint32_t i = 0; i < RewindableBufferFrames; ++i) + { + test = i; + EXPECT_EQ(i, test); + Multiplayer::GetNetworkTime()->IncrementHostFrameId(); + } + + { + // Test that Get/GetPrevious return different value when not on the owning connection + Multiplayer::ScopedAlterTime time(static_cast(RewindableBufferFrames - 1), AZ::TimeMs{ 0 }, 1.f, AzNetworking::InvalidConnectionId); + EXPECT_EQ(RewindableBufferFrames - 1, test.Get()); + EXPECT_EQ(RewindableBufferFrames - 2, test.GetPrevious()); + } + + // Test that Get/GetPrevious return the unaltered frame on the owning conection + Multiplayer::GetNetworkTime()->AlterTime(static_cast(RewindableBufferFrames - 1), AZ::TimeMs{ 0 }, 1.f, AzNetworking::ConnectionId(0)); + { + Multiplayer::ScopedAlterTime time(static_cast(RewindableBufferFrames - 1), AZ::TimeMs{ 0 }, 1.f, AzNetworking::ConnectionId(0)); + test.SetOwningConnectionId(AzNetworking::ConnectionId(0)); + EXPECT_EQ(RewindableBufferFrames - 1, test.Get()); + EXPECT_EQ(RewindableBufferFrames - 1, test.GetPrevious()); + } + Multiplayer::GetNetworkTime()->AlterTime(static_cast(RewindableBufferFrames), AZ::TimeMs(0), 1.f, AzNetworking::InvalidConnectionId); + } + TEST_F(RewindableObjectTests, OverflowTests) { Multiplayer::RewindableObject test(0); From bd0032a6d111ba249f3b8b36a3f6ad8fa4664162 Mon Sep 17 00:00:00 2001 From: santorac <55155825+santorac@users.noreply.github.com> Date: Fri, 17 Sep 2021 18:03:33 -0700 Subject: [PATCH 07/19] Fixed a recently introduced Material initialization bug. An early-return in Material::SetPropertyValue was breaking initialization because Init() was called multiple times, and on subsequent initializations the property values weren't getting reset. Added a unit test to ensure this kind of thing doesn't happen again. Signed-off-by: santorac <55155825+santorac@users.noreply.github.com> --- .../Atom/RPI.Reflect/Material/MaterialAsset.h | 6 ++ .../Source/RPI.Public/Material/Material.cpp | 8 ++- .../RPI/Code/Tests/Material/MaterialTests.cpp | 72 +++++++++++++++++++ 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h index 52af56ba0e..6ca3bca652 100644 --- a/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h +++ b/Gems/Atom/RPI/Code/Include/Atom/RPI.Reflect/Material/MaterialAsset.h @@ -19,6 +19,11 @@ #include +namespace UnitTest +{ + class MaterialTests; +} + namespace AZ { class ReflectContext; @@ -40,6 +45,7 @@ namespace AZ friend class MaterialAssetCreator; friend class MaterialAssetHandler; friend class MaterialAssetCreatorCommon; + friend class UnitTest::MaterialTests; public: AZ_RTTI(MaterialAsset, "{522C7BE0-501D-463E-92C6-15184A2B7AD8}", AZ::Data::AssetData); diff --git a/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp b/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp index 2dcaa70deb..d44ed40c7b 100644 --- a/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp +++ b/Gems/Atom/RPI/Code/Source/RPI.Public/Material/Material.cpp @@ -100,9 +100,15 @@ namespace AZ ShaderReloadNotificationBus::MultiHandler::BusConnect(shaderItem.GetShaderAsset().GetId()); } + // If this Init() is actually a re-initialize, we need to re-apply any overridden property values + // after loading the property values from the asset, so we save that data here. MaterialPropertyFlags prevOverrideFlags = m_propertyOverrideFlags; AZStd::vector prevPropertyValues = m_propertyValues; + // The property values are cleared to their default state to ensure that SetPropertyValue() does not early-return + // when called below. This is important when Init() is actually a re-initialize. + m_propertyValues.clear(); + // Initialize the shader runtime data like shader constant buffers and shader variants by applying the // material's property values. This will feed through the normal runtime material value-change data flow, which may // include custom property change handlers provided by the material type. @@ -504,7 +510,7 @@ namespace AZ MaterialPropertyValue& savedPropertyValue = m_propertyValues[index.GetIndex()]; - // If the property value didn't actually change, don't waste time running functors and compiling the changes + // If the property value didn't actually change, don't waste time running functors and compiling the changes. if (savedPropertyValue == value) { return false; diff --git a/Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp b/Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp index d82a17e1f3..f202747bf6 100644 --- a/Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp +++ b/Gems/Atom/RPI/Code/Tests/Material/MaterialTests.cpp @@ -182,6 +182,13 @@ namespace UnitTest EXPECT_EQ(srgData.GetImageView(srgData.FindShaderInputImageIndex(Name{ "m_image" }), 0), m_testImage->GetImageView()); EXPECT_EQ(srgData.GetConstant(srgData.FindShaderInputConstantIndex(Name{ "m_enum" })), 2u); } + + //! Provides write access to private material asset property values, primarily for simulating + //! MaterialAsset hot reload. + MaterialPropertyValue& AccessMaterialAssetPropertyValue(Data::Asset materialAsset, Name propertyName) + { + return materialAsset->m_propertyValues[materialAsset->GetMaterialPropertiesLayout()->FindPropertyIndex(propertyName).GetIndex()]; + } }; TEST_F(MaterialTests, TestCreateVsFindOrCreate) @@ -313,6 +320,30 @@ namespace UnitTest EXPECT_EQ(srgData.GetConstant(srgData.FindShaderInputConstantIndex(Name{ "m_uint" })), 42u); } + TEST_F(MaterialTests, TestSetPropertyValueWhenValueIsUnchanged) + { + Data::Instance material = Material::FindOrCreate(m_testMaterialAsset); + + EXPECT_TRUE(material->SetPropertyValue(material->FindPropertyIndex(Name{ "MyFloat" }), 2.5f)); + + ProcessQueuedSrgCompilations(m_testMaterialShaderAsset, m_testMaterialSrgLayout->GetName()); + EXPECT_TRUE(material->Compile()); + + // Taint the SRG so we can check whether it was set by the SetPropertyValue() calls below. + const RHI::ShaderResourceGroup* srg = material->GetRHIShaderResourceGroup(); + const RHI::ShaderResourceGroupData& srgData = srg->GetData(); + const_cast(&srgData)->SetConstant(m_testMaterialSrgLayout->FindShaderInputConstantIndex(Name{"m_float"}), 0.0f); + + // Set the properties to the same values as before + EXPECT_FALSE(material->SetPropertyValue(material->FindPropertyIndex(Name{ "MyFloat" }), 2.5f)); + + ProcessQueuedSrgCompilations(m_testMaterialShaderAsset, m_testMaterialSrgLayout->GetName()); + EXPECT_FALSE(material->Compile()); + + // Make sure the SRG is still tainted, because the SetPropertyValue() functions weren't processed + EXPECT_EQ(srgData.GetConstant(srgData.FindShaderInputConstantIndex(Name{ "m_float" })), 0.0f); + } + TEST_F(MaterialTests, TestImageNotProvided) { Data::Asset materialAssetWithEmptyImage; @@ -785,4 +816,45 @@ namespace UnitTest EXPECT_EQ((float)inputColor.GetElement(i), (float)colorFromMaterial.GetElement(i)); } } + + TEST_F(MaterialTests, TestReinitializeForHotReload) + { + Data::Instance material = Material::FindOrCreate(m_testMaterialAsset); + const RHI::ShaderResourceGroupData* srgData = &material->GetRHIShaderResourceGroup()->GetData(); + ProcessQueuedSrgCompilations(m_testMaterialShaderAsset, m_testMaterialSrgLayout->GetName()); + + // Check the default property value + EXPECT_EQ(material->GetPropertyValue(material->FindPropertyIndex(Name{ "MyFloat" })), 1.5f); + EXPECT_EQ(srgData->GetConstant(srgData->FindShaderInputConstantIndex(Name{ "m_float" })), 1.5f); + EXPECT_EQ(material->GetPropertyValue(material->FindPropertyIndex(Name{ "MyInt" })), -2); + EXPECT_EQ(srgData->GetConstant(srgData->FindShaderInputConstantIndex(Name{ "m_int" })), -2); + + // Override a property value + EXPECT_TRUE(material->SetPropertyValue(material->FindPropertyIndex(Name{ "MyFloat" }), 5.5f)); + + // Apply the changes + EXPECT_TRUE(material->Compile()); + ProcessQueuedSrgCompilations(m_testMaterialShaderAsset, m_testMaterialSrgLayout->GetName()); + + // Check the updated values with one overridden + EXPECT_EQ(material->GetPropertyValue(material->FindPropertyIndex(Name{ "MyFloat" })), 5.5f); + EXPECT_EQ(srgData->GetConstant(srgData->FindShaderInputConstantIndex(Name{ "m_float" })), 5.5f); + EXPECT_EQ(material->GetPropertyValue(material->FindPropertyIndex(Name{ "MyInt" })), -2); + EXPECT_EQ(srgData->GetConstant(srgData->FindShaderInputConstantIndex(Name{ "m_int" })), -2); + + // Pretend there was a hot-reload with new default values + AccessMaterialAssetPropertyValue(m_testMaterialAsset, Name{"MyFloat"}) = 0.5f; + AccessMaterialAssetPropertyValue(m_testMaterialAsset, Name{"MyInt"}) = -7; + AZ::Data::AssetBus::Event(m_testMaterialAsset.GetId(), &AZ::Data::AssetBus::Handler::OnAssetReloaded, m_testMaterialAsset); + srgData = &material->GetRHIShaderResourceGroup()->GetData(); + ProcessQueuedSrgCompilations(m_testMaterialShaderAsset, m_testMaterialSrgLayout->GetName()); + + // Make sure the override values are still there + EXPECT_EQ(srgData->GetConstant(srgData->FindShaderInputConstantIndex(Name{ "m_float" })), 5.5f); + EXPECT_EQ(material->GetPropertyValue(material->FindPropertyIndex(Name{ "MyFloat" })), 5.5f); + + // Make sure the new default value is applied where it was not overridden + EXPECT_EQ(material->GetPropertyValue(material->FindPropertyIndex(Name{ "MyInt" })), -7); + EXPECT_EQ(srgData->GetConstant(srgData->FindShaderInputConstantIndex(Name{ "m_int" })), -7); + } } From 22e43c9122fdb4debe5edd48b745ac64c7c8bcb2 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Fri, 17 Sep 2021 21:16:04 -0700 Subject: [PATCH 08/19] Nearly all build jobs fail with a missing function in Archive.cpp (#4195) Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- Code/Editor/Include/IEditorClassFactory.h | 6 ++++-- Code/Framework/AzFramework/AzFramework/Archive/Archive.cpp | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Code/Editor/Include/IEditorClassFactory.h b/Code/Editor/Include/IEditorClassFactory.h index 0827dc96dc..dd47f803e2 100644 --- a/Code/Editor/Include/IEditorClassFactory.h +++ b/Code/Editor/Include/IEditorClassFactory.h @@ -14,8 +14,10 @@ #define CRYINCLUDE_EDITOR_INCLUDE_IEDITORCLASSFACTORY_H #pragma once +#include #include #include +#include #define DEFINE_UUID(l, w1, w2, b1, b2, b3, b4, b5, b6, b7, b8) \ static const GUID uuid() { return { l, w1, w2, { b1, b2, b3, b4, b5, b6, b7, b8 } }; } @@ -34,7 +36,7 @@ struct IUnknown #endif #define __uuidof(T) T::uuid() -#if defined(AZ_PLATFORM_LINUX) +#if defined(AZ_PLATFORM_LINUX) || defined(AZ_PLATFORM_MAC) # ifndef _REFGUID_DEFINED # define _REFGUID_DEFINED @@ -65,7 +67,7 @@ enum }; #endif -#endif // defined(AZ_PLATFORM_LINUX) +#endif // defined(AZ_PLATFORM_LINUX) || defined(AZ_PLATFORM_MAC) #include "SandboxAPI.h" diff --git a/Code/Framework/AzFramework/AzFramework/Archive/Archive.cpp b/Code/Framework/AzFramework/AzFramework/Archive/Archive.cpp index c94d588a90..d2a81102dc 100644 --- a/Code/Framework/AzFramework/AzFramework/Archive/Archive.cpp +++ b/Code/Framework/AzFramework/AzFramework/Archive/Archive.cpp @@ -181,7 +181,7 @@ namespace AZ::IO::ArchiveInternal return 0; } - nTotal = (AZStd::min)(nTotal, GetFileSize() - m_nCurSeek); + nTotal = AZStd::min(nTotal, GetFileSize() - m_nCurSeek); int64_t nReadBytes = GetFile()->ReadData(pDest, m_nCurSeek, nTotal); if (nReadBytes == -1) From de9a27597be342516c8bb02036b214f72bec3aa5 Mon Sep 17 00:00:00 2001 From: hultonha <82228511+hultonha@users.noreply.github.com> Date: Mon, 20 Sep 2021 09:49:01 +0100 Subject: [PATCH 09/19] Additional test support for Manipulator Ditto commands (#4191) * additional test support for manipulator ditto Signed-off-by: hultonha * tidy-up to changes Signed-off-by: hultonha * fix for non-unity build compile error Signed-off-by: hultonha --- Code/Editor/EditorToolsApplication.cpp | 16 ++- Code/Editor/EditorToolsApplication.h | 10 ++ Code/Editor/EditorViewportWidget.cpp | 19 ++-- Code/Editor/EditorViewportWidget.h | 4 - .../AzFramework/Viewport/ClickDetector.cpp | 27 ++++- .../AzFramework/Viewport/ClickDetector.h | 14 ++- .../ImmediateModeActionDispatcher.h | 16 +++ .../Source/ImmediateModeActionDispatcher.cpp | 2 + .../Viewport/ViewportMessages.h | 18 ++++ .../EditorTransformComponentSelection.cpp | 19 +++- ...EditorTransformComponentSelectionTests.cpp | 102 +++++++++++++++++- 11 files changed, 220 insertions(+), 27 deletions(-) diff --git a/Code/Editor/EditorToolsApplication.cpp b/Code/Editor/EditorToolsApplication.cpp index 1e5d747e4a..26e608f657 100644 --- a/Code/Editor/EditorToolsApplication.cpp +++ b/Code/Editor/EditorToolsApplication.cpp @@ -34,10 +34,14 @@ namespace EditorInternal : ToolsApplication(argc, argv) { EditorToolsApplicationRequests::Bus::Handler::BusConnect(); + AzToolsFramework::ViewportInteraction::EditorModifierKeyRequestBus::Handler::BusConnect(); + AzToolsFramework::ViewportInteraction::EditorViewportInputTimeNowRequestBus::Handler::BusConnect(); } EditorToolsApplication::~EditorToolsApplication() { + AzToolsFramework::ViewportInteraction::EditorViewportInputTimeNowRequestBus::Handler::BusDisconnect(); + AzToolsFramework::ViewportInteraction::EditorModifierKeyRequestBus::Handler::BusDisconnect(); EditorToolsApplicationRequests::Bus::Handler::BusDisconnect(); Stop(); } @@ -48,7 +52,6 @@ namespace EditorInternal return m_StartupAborted; } - void EditorToolsApplication::RegisterCoreComponents() { AzToolsFramework::ToolsApplication::RegisterCoreComponents(); @@ -274,5 +277,14 @@ namespace EditorInternal Exit(); } -} + AzToolsFramework::ViewportInteraction::KeyboardModifiers EditorToolsApplication::QueryKeyboardModifiers() + { + return AzToolsFramework::ViewportInteraction::BuildKeyboardModifiers(QGuiApplication::queryKeyboardModifiers()); + } + AZStd::chrono::milliseconds EditorToolsApplication::EditorViewportInputTimeNow() + { + const auto now = AZStd::chrono::high_resolution_clock::now(); + return AZStd::chrono::time_point_cast(now).time_since_epoch(); + } +} // namespace EditorInternal diff --git a/Code/Editor/EditorToolsApplication.h b/Code/Editor/EditorToolsApplication.h index 422772cbc4..d4e6223445 100644 --- a/Code/Editor/EditorToolsApplication.h +++ b/Code/Editor/EditorToolsApplication.h @@ -7,7 +7,9 @@ */ #pragma once + #include +#include #include "Core/EditorMetricsPlainTextNameRegistration.h" #include "EditorToolsApplicationAPI.h" @@ -19,6 +21,8 @@ namespace EditorInternal class EditorToolsApplication : public AzToolsFramework::ToolsApplication , public EditorToolsApplicationRequests::Bus::Handler + , public AzToolsFramework::ViewportInteraction::EditorModifierKeyRequestBus::Handler + , public AzToolsFramework::ViewportInteraction::EditorViewportInputTimeNowRequestBus::Handler { public: EditorToolsApplication(int* argc, char*** argv); @@ -44,6 +48,12 @@ namespace EditorInternal void CreateReflectionManager() override; void Reflect(AZ::ReflectContext* context) override; + // EditorModifierKeyRequestBus overrides ... + AzToolsFramework::ViewportInteraction::KeyboardModifiers QueryKeyboardModifiers() override; + + // EditorViewportInputTimeNowRequestBus overrides ... + AZStd::chrono::milliseconds EditorViewportInputTimeNow() override; + protected: // From EditorToolsApplicationRequests bool OpenLevel(AZStd::string_view levelName) override; diff --git a/Code/Editor/EditorViewportWidget.cpp b/Code/Editor/EditorViewportWidget.cpp index 1ed901e514..5b5b52d28f 100644 --- a/Code/Editor/EditorViewportWidget.cpp +++ b/Code/Editor/EditorViewportWidget.cpp @@ -744,11 +744,15 @@ void EditorViewportWidget::RenderAll() { namespace AztfVi = AzToolsFramework::ViewportInteraction; + AztfVi::KeyboardModifiers keyboardModifiers; + AztfVi::EditorModifierKeyRequestBus::BroadcastResult( + keyboardModifiers, &AztfVi::EditorModifierKeyRequestBus::Events::QueryKeyboardModifiers); + m_debugDisplay->DepthTestOff(); m_manipulatorManager->DrawManipulators( *m_debugDisplay, GetCameraState(), BuildMouseInteractionInternal( - AztfVi::MouseButtons(AztfVi::TranslateMouseButtons(QGuiApplication::mouseButtons())), QueryKeyboardModifiers(), + AztfVi::MouseButtons(AztfVi::TranslateMouseButtons(QGuiApplication::mouseButtons())), keyboardModifiers, BuildMousePick(WidgetToViewport(mapFromGlobal(QCursor::pos()))))); m_debugDisplay->DepthTestOn(); } @@ -959,12 +963,13 @@ QWidget* EditorViewportWidget::GetWidgetForViewportContextMenu() bool EditorViewportWidget::ShowingWorldSpace() { - return QueryKeyboardModifiers().Shift(); -} + namespace AztfVi = AzToolsFramework::ViewportInteraction; -AzToolsFramework::ViewportInteraction::KeyboardModifiers EditorViewportWidget::QueryKeyboardModifiers() -{ - return AzToolsFramework::ViewportInteraction::BuildKeyboardModifiers(QGuiApplication::queryKeyboardModifiers()); + AztfVi::KeyboardModifiers keyboardModifiers; + AztfVi::EditorModifierKeyRequestBus::BroadcastResult( + keyboardModifiers, &AztfVi::EditorModifierKeyRequestBus::Events::QueryKeyboardModifiers); + + return keyboardModifiers.Shift(); } void EditorViewportWidget::SetViewportId(int id) @@ -1039,7 +1044,6 @@ void EditorViewportWidget::ConnectViewportInteractionRequestBus() { AzToolsFramework::ViewportInteraction::MainEditorViewportInteractionRequestBus::Handler::BusConnect(GetViewportId()); AzToolsFramework::ViewportInteraction::EditorEntityViewportInteractionRequestBus::Handler::BusConnect(GetViewportId()); - AzToolsFramework::ViewportInteraction::EditorModifierKeyRequestBus::Handler::BusConnect(); m_viewportUi.ConnectViewportUiBus(GetViewportId()); AzFramework::InputSystemCursorConstraintRequestBus::Handler::BusConnect(); @@ -1050,7 +1054,6 @@ void EditorViewportWidget::DisconnectViewportInteractionRequestBus() AzFramework::InputSystemCursorConstraintRequestBus::Handler::BusDisconnect(); m_viewportUi.DisconnectViewportUiBus(); - AzToolsFramework::ViewportInteraction::EditorModifierKeyRequestBus::Handler::BusDisconnect(); AzToolsFramework::ViewportInteraction::EditorEntityViewportInteractionRequestBus::Handler::BusDisconnect(); AzToolsFramework::ViewportInteraction::MainEditorViewportInteractionRequestBus::Handler::BusDisconnect(); } diff --git a/Code/Editor/EditorViewportWidget.h b/Code/Editor/EditorViewportWidget.h index 6b46524394..49930a2a13 100644 --- a/Code/Editor/EditorViewportWidget.h +++ b/Code/Editor/EditorViewportWidget.h @@ -92,7 +92,6 @@ class SANDBOX_API EditorViewportWidget final , private AzFramework::InputSystemCursorConstraintRequestBus::Handler , private AzToolsFramework::ViewportInteraction::MainEditorViewportInteractionRequestBus::Handler , private AzToolsFramework::ViewportInteraction::EditorEntityViewportInteractionRequestBus::Handler - , private AzToolsFramework::ViewportInteraction::EditorModifierKeyRequestBus::Handler , private AzFramework::AssetCatalogEventBus::Handler , private AZ::RPI::SceneNotificationBus::Handler { @@ -212,9 +211,6 @@ private: // EditorEntityViewportInteractionRequestBus overrides ... void FindVisibleEntities(AZStd::vector& visibleEntities) override; - // EditorModifierKeyRequestBus overrides ... - AzToolsFramework::ViewportInteraction::KeyboardModifiers QueryKeyboardModifiers() override; - // Camera::EditorCameraRequestBus overrides ... void SetViewFromEntityPerspective(const AZ::EntityId& entityId) override; void SetViewAndMovementLockFromEntityPerspective(const AZ::EntityId& entityId, bool lockCameraMovement) override; diff --git a/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.cpp b/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.cpp index b909689c22..1b3645630e 100644 --- a/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.cpp +++ b/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.cpp @@ -9,8 +9,19 @@ #include #include +#include + namespace AzFramework { + ClickDetector::ClickDetector() + { + m_timeNowFn = [] + { + const auto now = AZStd::chrono::high_resolution_clock::now(); + return AZStd::chrono::time_point_cast(now).time_since_epoch(); + }; + } + ClickDetector::ClickOutcome ClickDetector::DetectClick(const ClickEvent clickEvent, const ScreenVector& cursorDelta) { const auto previousDetectionState = m_detectionState; @@ -26,11 +37,13 @@ namespace AzFramework if (clickEvent == ClickEvent::Down) { - const auto now = std::chrono::steady_clock::now(); + const auto now = m_timeNowFn(); if (m_tryBeginTime) { - const std::chrono::duration diff = now - m_tryBeginTime.value(); - if (diff.count() < m_doubleClickInterval) + using FloatingPointSeconds = AZStd::chrono::duration; + + const auto diff = now - m_tryBeginTime.value(); + if (FloatingPointSeconds(diff).count() < m_doubleClickInterval) { return ClickOutcome::Nil; } @@ -43,7 +56,8 @@ namespace AzFramework } else if (clickEvent == ClickEvent::Up) { - const auto clickOutcome = [detectionState = m_detectionState] { + const auto clickOutcome = [detectionState = m_detectionState] + { if (detectionState == DetectionState::WaitingForMove) { return ClickOutcome::Click; @@ -66,4 +80,9 @@ namespace AzFramework return ClickOutcome::Nil; } + + void ClickDetector::OverrideTimeNowFn(AZStd::function timeNowFn) + { + m_timeNowFn = AZStd::move(timeNowFn); + } } // namespace AzFramework diff --git a/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.h b/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.h index f95924550a..70bdeb4619 100644 --- a/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.h +++ b/Code/Framework/AzFramework/AzFramework/Viewport/ClickDetector.h @@ -8,6 +8,7 @@ #pragma once +#include #include #include @@ -21,10 +22,9 @@ namespace AzFramework //! (mouse down with movement and then mouse up). class ClickDetector { - //! Alias for recording time of mouse down events - using Time = std::chrono::time_point; - public: + ClickDetector(); + //! Internal representation of click event (map from external event for this when //! calling DetectClick). enum class ClickEvent @@ -51,6 +51,10 @@ namespace AzFramework void SetDoubleClickInterval(float doubleClickInterval); //! Override the dead zone before a 'move' outcome will be triggered. void SetDeadZone(float deadZone); + //! Override how the current time is retrieved. + //! This is helpful to override when it comes to simulating different passages of + //! time to avoid double click issues in tests for example. + void OverrideTimeNowFn(AZStd::function timeNowFn); private: //! Internal state of ClickDetector based on incoming events. @@ -65,7 +69,9 @@ namespace AzFramework float m_deadZone = 2.0f; //!< How far to move before a click is cancelled (when Move will fire). float m_doubleClickInterval = 0.4f; //!< Default double click interval, can be overridden. DetectionState m_detectionState; //!< Internal state of ClickDetector. - AZStd::optional