From 7eabc7664c4abfa4f8baf31cb3918646021370a7 Mon Sep 17 00:00:00 2001 From: eheimburg <74330250+eheimburg@users.noreply.github.com> Date: Mon, 12 Apr 2021 07:17:16 -0400 Subject: [PATCH 1/5] Adds move-check timeout to untargeted Actions [GOMPS-467] --- .../Client/Game/Character/ClientInputSender.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Assets/BossRoom/Scripts/Client/Game/Character/ClientInputSender.cs b/Assets/BossRoom/Scripts/Client/Game/Character/ClientInputSender.cs index fedd459d3..8cf65c0a5 100644 --- a/Assets/BossRoom/Scripts/Client/Game/Character/ClientInputSender.cs +++ b/Assets/BossRoom/Scripts/Client/Game/Character/ClientInputSender.cs @@ -219,7 +219,7 @@ private void PerformSkill(ActionType actionType, SkillTriggerStyle triggerStyle, if (GetActionRequestForTarget(hitTransform, actionType, triggerStyle, out ActionRequestData playerAction)) { - //Don't trigger our move logic for another 500ms. This protects us from moving just because we clicked on them to target them. + //Don't trigger our move logic for a while. This protects us from moving just because we clicked on them to target them. m_LastSentMove = Time.time + k_TargetMoveTimeout; ActionInputEvent?.Invoke(playerAction); @@ -227,7 +227,14 @@ private void PerformSkill(ActionType actionType, SkillTriggerStyle triggerStyle, } else if(actionType != ActionType.GeneralTarget ) { - // clicked on nothing... perform a "miss" attack on the spot they clicked on + // clicked on nothing... perform an "untargeted" attack on the spot they clicked on. + // (Different Actions will deal with this differently. For some, like archer arrows, this will fire an arrow + // in the desired direction. For others, like mage's bolts, this will fire a "miss" projectile at the spot clicked on.) + + // Don't trigger our move logic for a while. This protects us from moving immediately after + // starting the untargeted action (which would abort the action on the server) + m_LastSentMove = Time.time + k_TargetMoveTimeout; + var data = new ActionRequestData(); PopulateSkillRequest(k_CachedHit[0].point, actionType, ref data); From 161205538f5eadc95f65604b2f3488d636aef90c Mon Sep 17 00:00:00 2001 From: eheimburg <74330250+eheimburg@users.noreply.github.com> Date: Wed, 14 Apr 2021 06:26:59 -0400 Subject: [PATCH 2/5] new approach: only abort interruptible actions --- .../Client/Game/Character/ClientInputSender.cs | 4 ---- .../Server/Game/Character/ServerCharacter.cs | 13 ++++++++++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Assets/BossRoom/Scripts/Client/Game/Character/ClientInputSender.cs b/Assets/BossRoom/Scripts/Client/Game/Character/ClientInputSender.cs index 8cf65c0a5..a8ad24683 100644 --- a/Assets/BossRoom/Scripts/Client/Game/Character/ClientInputSender.cs +++ b/Assets/BossRoom/Scripts/Client/Game/Character/ClientInputSender.cs @@ -231,10 +231,6 @@ private void PerformSkill(ActionType actionType, SkillTriggerStyle triggerStyle, // (Different Actions will deal with this differently. For some, like archer arrows, this will fire an arrow // in the desired direction. For others, like mage's bolts, this will fire a "miss" projectile at the spot clicked on.) - // Don't trigger our move logic for a while. This protects us from moving immediately after - // starting the untargeted action (which would abort the action on the server) - m_LastSentMove = Time.time + k_TargetMoveTimeout; - var data = new ActionRequestData(); PopulateSkillRequest(k_CachedHit[0].point, actionType, ref data); diff --git a/Assets/BossRoom/Scripts/Server/Game/Character/ServerCharacter.cs b/Assets/BossRoom/Scripts/Server/Game/Character/ServerCharacter.cs index 6986ee231..8de730f4b 100644 --- a/Assets/BossRoom/Scripts/Server/Game/Character/ServerCharacter.cs +++ b/Assets/BossRoom/Scripts/Server/Game/Character/ServerCharacter.cs @@ -129,7 +129,18 @@ private void OnClientMoveRequest(Vector3 targetPosition) { if (NetState.NetworkLifeState.Value == LifeState.Alive && !m_Movement.IsPerformingForcedMovement()) { - ClearActions(false); + // if we're currently playing an interruptible action, cancel it! + if (m_ActionPlayer.GetActiveActionInfo(out ActionRequestData data)) + { + if (GameDataSource.Instance.ActionDataByType.TryGetValue(data.ActionTypeEnum, out ActionDescription description)) + { + if (description.ActionInterruptible) + { + ClearActions(false); + } + } + } + m_ActionPlayer.CancelRunningActionsByLogic(ActionLogic.Target, true); //clear target on move. m_Movement.SetMovementTarget(targetPosition); } From 6dcb67f123362b678d5b07860786b6b2ecd4011b Mon Sep 17 00:00:00 2001 From: eheimburg <74330250+eheimburg@users.noreply.github.com> Date: Tue, 20 Apr 2021 04:40:45 -0400 Subject: [PATCH 3/5] Subtly change meaning of ActionInterruptible boolean to mean "don't move while performing it" --- .../Scripts/Server/Game/Action/ActionPlayer.cs | 9 +++++++++ .../Server/Game/Action/StealthModeAction.cs | 6 ------ .../Server/Game/Character/ServerCharacter.cs | 16 ++++------------ .../Scripts/Shared/Data/ActionDescription.cs | 2 +- 4 files changed, 14 insertions(+), 19 deletions(-) diff --git a/Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs b/Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs index 43257a1bc..ac4ea6b63 100644 --- a/Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs +++ b/Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs @@ -11,6 +11,8 @@ public class ActionPlayer { private ServerCharacter m_Parent; + private ServerCharacterMovement m_Movement; + private List m_Queue; private List m_NonBlockingActions; @@ -30,6 +32,7 @@ public class ActionPlayer public ActionPlayer(ServerCharacter parent) { m_Parent = parent; + m_Movement = parent.GetComponent(); m_Queue = new List(); m_NonBlockingActions = new List(); m_LastUsedTimestamps = new Dictionary(); @@ -134,6 +137,12 @@ private void StartAction() return; // ... so it's important not to try to do anything more here } + // if this Action is interruptible, that means movement would interrupt it. So stop any active movement! + if (m_Queue[0].Description.ActionInterruptible && !m_Movement.IsPerformingForcedMovement()) + { + m_Movement.CancelMove(); + } + // remember that we successfully used this Action! m_LastUsedTimestamps[m_Queue[0].Description.ActionTypeEnum] = Time.time; diff --git a/Assets/BossRoom/Scripts/Server/Game/Action/StealthModeAction.cs b/Assets/BossRoom/Scripts/Server/Game/Action/StealthModeAction.cs index 6cead3f08..c728e9e26 100644 --- a/Assets/BossRoom/Scripts/Server/Game/Action/StealthModeAction.cs +++ b/Assets/BossRoom/Scripts/Server/Game/Action/StealthModeAction.cs @@ -20,12 +20,6 @@ public override bool Start() { m_Parent.NetState.RecvDoActionClientRPC(Data); - // not allowed to walk while going stealthy! - var movement = m_Parent.GetComponent(); - if (!movement.IsPerformingForcedMovement()) - { - movement.CancelMove(); - } return true; } diff --git a/Assets/BossRoom/Scripts/Server/Game/Character/ServerCharacter.cs b/Assets/BossRoom/Scripts/Server/Game/Character/ServerCharacter.cs index 8de730f4b..b45225349 100644 --- a/Assets/BossRoom/Scripts/Server/Game/Character/ServerCharacter.cs +++ b/Assets/BossRoom/Scripts/Server/Game/Character/ServerCharacter.cs @@ -129,14 +129,14 @@ private void OnClientMoveRequest(Vector3 targetPosition) { if (NetState.NetworkLifeState.Value == LifeState.Alive && !m_Movement.IsPerformingForcedMovement()) { - // if we're currently playing an interruptible action, cancel it! + // if we're currently playing an interruptible action, interrupt it! if (m_ActionPlayer.GetActiveActionInfo(out ActionRequestData data)) { if (GameDataSource.Instance.ActionDataByType.TryGetValue(data.ActionTypeEnum, out ActionDescription description)) { if (description.ActionInterruptible) { - ClearActions(false); + m_ActionPlayer.ClearActions(false); } } } @@ -150,19 +150,11 @@ private void OnLifeStateChanged(LifeState prevLifeState, LifeState lifeState) { if (lifeState != LifeState.Alive) { - ClearActions(true); + m_ActionPlayer.ClearActions(true); m_Movement.CancelMove(); } } - /// - /// Clear all active Actions. - /// - public void ClearActions(bool alsoClearNonBlockingActions) - { - m_ActionPlayer.ClearActions(alsoClearNonBlockingActions); - } - private void OnActionPlayRequest(ActionRequestData data) { if (!GameDataSource.Instance.ActionDataByType[data.ActionTypeEnum].IsFriendly) @@ -217,7 +209,7 @@ public void ReceiveHP(ServerCharacter inflicter, int HP) //that's handled by a separate function. if (NetState.HitPoints <= 0) { - ClearActions(false); + m_ActionPlayer.ClearActions(false); if (IsNpc) { diff --git a/Assets/BossRoom/Scripts/Shared/Data/ActionDescription.cs b/Assets/BossRoom/Scripts/Shared/Data/ActionDescription.cs index 60d0fd50e..32178612b 100644 --- a/Assets/BossRoom/Scripts/Shared/Data/ActionDescription.cs +++ b/Assets/BossRoom/Scripts/Shared/Data/ActionDescription.cs @@ -68,7 +68,7 @@ public class ActionDescription : ScriptableObject [Tooltip("Prefab to spawn that will manage this action's input")] public BaseActionInput ActionInput; - [Tooltip("Is this Action interruptible by other action plays. Generally, actions with short exec times should not be interruptible in this way.")] + [Tooltip("Is this Action interruptible by other action plays or by movement? (Implicitly stops movement when action starts.) Generally, actions with short exec times should not be interruptible in this way.")] public bool ActionInterruptible; [System.Serializable] From a7af74a2ab727f2661a21d41699359da69756d3b Mon Sep 17 00:00:00 2001 From: eheimburg <74330250+eheimburg@users.noreply.github.com> Date: Thu, 22 Apr 2021 07:29:19 -0400 Subject: [PATCH 4/5] comment tweaks --- Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs | 2 +- Assets/BossRoom/Scripts/Shared/Data/ActionDescription.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs b/Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs index ac4ea6b63..fd6924970 100644 --- a/Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs +++ b/Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs @@ -137,7 +137,7 @@ private void StartAction() return; // ... so it's important not to try to do anything more here } - // if this Action is interruptible, that means movement would interrupt it. So stop any active movement! + // if this Action is interruptible, that means movement should interrupt it. So stop any movement that's already happening before we begin if (m_Queue[0].Description.ActionInterruptible && !m_Movement.IsPerformingForcedMovement()) { m_Movement.CancelMove(); diff --git a/Assets/BossRoom/Scripts/Shared/Data/ActionDescription.cs b/Assets/BossRoom/Scripts/Shared/Data/ActionDescription.cs index 32178612b..ac0f53f94 100644 --- a/Assets/BossRoom/Scripts/Shared/Data/ActionDescription.cs +++ b/Assets/BossRoom/Scripts/Shared/Data/ActionDescription.cs @@ -68,7 +68,7 @@ public class ActionDescription : ScriptableObject [Tooltip("Prefab to spawn that will manage this action's input")] public BaseActionInput ActionInput; - [Tooltip("Is this Action interruptible by other action plays or by movement? (Implicitly stops movement when action starts.) Generally, actions with short exec times should not be interruptible in this way.")] + [Tooltip("Is this Action interruptible by other action-plays or by movement? (Implicitly stops movement when action starts.) Generally, actions with short exec times should not be interruptible in this way.")] public bool ActionInterruptible; [System.Serializable] From 6608ed17be905e51d54b285297edd7a6cc205bb3 Mon Sep 17 00:00:00 2001 From: eheimburg <74330250+eheimburg@users.noreply.github.com> Date: Thu, 22 Apr 2021 07:35:56 -0400 Subject: [PATCH 5/5] better comment --- Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs b/Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs index fd6924970..5ecea282c 100644 --- a/Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs +++ b/Assets/BossRoom/Scripts/Server/Game/Action/ActionPlayer.cs @@ -137,7 +137,8 @@ private void StartAction() return; // ... so it's important not to try to do anything more here } - // if this Action is interruptible, that means movement should interrupt it. So stop any movement that's already happening before we begin + // if this Action is interruptible, that means movement should interrupt it... character needs to be stationary for this! + // So stop any movement that's already happening before we begin if (m_Queue[0].Description.ActionInterruptible && !m_Movement.IsPerformingForcedMovement()) { m_Movement.CancelMove();