Feedback Movement override ability best practices

Fair warning, this will be a long post.

I am playing around with abilities right now and have seen tutorials with ability code contained inside the main PlayerController script, and watched the tutorial about Create an Ability System with Scriptable Objects. I began playing around with different implementation strategies.

In this case, I'm playing around with a "Dash" ability that triggers a dash in the current direction for a specified amount of time which overrides other movement input (e.g. Hollow Knight or Mega Man X without early cancel). I'm using top-down, so X/Y directions are ground movement.

Using the ScriptableObject ability system from the tutorial, triggerable abilities that fire off immediately and are done were easy. I can conceive the holding down to charge an ability would be easy as well. However, with dash overriding the normal movement for a specified duration from a single press, it seemed a little more complicated and I'm looking for some input on best practices for this kind of ability. I understand there is no one right way, but I'm certain there are some bad ways!

I'll give a few examples of things I've played around with. I don't feel fully satisfied with either option and I'm not sure if it's just my lack of game dev experience or if there is a much better way to accomplish this.

Basic implementation is very easy:

PlayerController.cs

Code (CSharp):

1.        
2. void FixedUpdate()
3. {
4.         float horizontal = input.Horizontal;
5.         float vertical = input.Vertical;
6.  
7.         if (!Mathf.Approximately(horizontal, 0.0f) || !Mathf.Approximately(vertical, 0.0f))
8.         {
9.             lookDirection.Set(horizontal, vertical);
10.             lookDirection.Normalize();
11.         }
12.  
13.         if (dashing)
14.         {
15.             if (dashRemaining > 0)
16.             {
17.                 dashRemaining -= Time.deltaTime;
18.                 velocity = lookDirection * dashSpeed;
19.             }
20.             else
21.             {
22.                 dashing = false;
23.             }
24.         }
25.         else
26.         {
27.             velocity = (new Vector2(horizontal, vertical)) * speed;
28.         }
29.  
30.         rigidbody2d.velocity = velocity;
31.  
32.         if (input.Ability1)
33.         {
34.             Dash();
35.         }
36. ...
37. }

This works fine, but I was curious to play around with the SciptableObject technique for abilities to move away from having one giant monolithic PlayerController class and allow for greater flexibility. For example, the PlayerController wouldn't need to know any details about Dash or other movement overriding abilities. An alternative is of course to use other MonoBehavior classes instead of ScriptableObject, but that still needs to be able to override the default movement.

So far I've considered two possibilities:

Add an "IsActive" property to the ability object, and have a "Continue" method that will continue running even if the button to trigger the ability is not pressed.

The code for that would look like this:

PlayerController.cs

Code (CSharp):

1.        
2.         if (input.Ability1Pressed)
3.         {
4.             dashAbility.TriggerAbility();
5.         }
6.         else if (dashAbility.IsActive)
7.         {
8.             dashAbility.ContinueAbility();
9.         }

Dash.cs

Code (CSharp):

1. public class Dash : Ability
2.     {
3.         private PlayerController controller;
4.         private Rigidbody2D rigidbody2d;
5.         private float cooldown = 1.0f;
6.         private float offCooldownTime;
7.         private float dashVelocity = 24.0f;
8.         private WaitForSeconds dashDuration = new WaitForSeconds(.2f);
9.  
10.         public override void Initialize(PlayerController controller)
11.         {
12.             this.controller = controller;
13.             rigidbody2d = controller.GetComponent<Rigidbody2D>();
14.             Name = "Dash";
15.         }
16.  
17.         public override void TriggerAbility()
18.         {
19.             if (Time.time > offCooldownTime && !IsActive)
20.             {
21.                 controller.StartCoroutine(TriggerDash());
22.                 DoDash();
23.             }
24.         }
25.  
26.         private IEnumerator TriggerDash()
27.         {
28.             IsActive = true;
29.             yield return dashDuration;
30.             IsActive = false;
31.             offCooldownTime = Time.time + cooldown;
32.         }
33.  
34.         public override void ContinueAbility()
35.         {
36.             DoDash();
37.         }
38.  
39.         private void DoDash()
40.         {
41.             rigidbody2d.velocity = controller.lookDirection * dashVelocity;
42.         }
43.     }

This method seems to work fine, but it feels a little weird to me. Better than keeping all of the ability code in PlayerController though.

Another option is to make use of Actions to override the movement temporarily, and then replace the default implementation.

PlayerController.cs

Code (CSharp):

1. [HideInInspector] public Action Move;
2.  
3. void Start()
4.     {
5. ...
6.         Move = RegularMovement;
7. ...
8.     }
9.  
10.  
11. void FixedUpdate()
12. {
13. ...
14.     Move();
15. ...
16. }
17.  
18. private void RegularMovement()
19.     {
20.         rigidbody2d.velocity = new Vector2(horizontal, vertical) * speed;
21.     }

Dash.cs

Code (CSharp):

1.     public class Dash : Ability
2.     {
3.         private PlayerController controller;
4.         private Rigidbody2D rigidbody2d;
5.         private float cooldown = 1.0f;
6.         private float offCooldownTime;
7.         private float dashVelocity = 24.0f;
8.         private WaitForSeconds dashDuration = new WaitForSeconds(0.2f);
9.         private float dashRemaining;
10.  
11.         private Action previousAction;
12.         private Action dashAction;
13.  
14.         public override void Initialize(PlayerController controller)
15.         {
16.             this.controller = controller;
17.             rigidbody2d = controller.GetComponent<Rigidbody2D>();
18.             Name = "Dash";
19.             dashAction = DashMovement;
20.         }
21.  
22.         public override void TriggerAbility()
23.         {
24.             if (Time.time > offCooldownTime && !IsActive)
25.             {
26.                 controller.StartCoroutine(TriggerDash());
27.             }
28.         }
29.  
30.         private IEnumerator TriggerDash()
31.         {
32.             IsActive = true;
33.             previousAction = controller.Move;
34.             controller.Move = dashAction;
35.             yield return dashDuration;
36.             IsActive = false;
37.             controller.Move = previousAction;
38.             offCooldownTime = Time.time + cooldown;
39.         }
40.  
41.         public void DashMovement()
42.         {
43.             rigidbody2d.velocity = controller.lookDirection * dashVelocity;
44.         }
45.     }

This option makes it so there can only really be one override taking place at any time and safeguards would need to be put into place to avoid bugs of course.

I've tested and all of these methods work fine... But I'm unsure what would be consider good/bad practice.

Thanks for taking the time to read this and I appreciate any feedback

 

I'm running into the same wall as you are. I'm looking into using a finite state machine for the players movement and having abilities force the player to a new state when triggered.

 

What about having a class responsable for keeping tracks of which component/behaviors are active or not ?
You could have the same input for basic movement and dashing and just disable the component "basic movement" while the "behaviors controller" enable the dash component.

I used to have this kind of setup and it worked really nice. Just populate the behaviors controller List or Array of behaviors and enable/disable them as you need.

 

I agree with RemDust. If you're already thinking along those lines, why not have separate components for each behavior, this is exactly what a component object model (COM) is for.

additionally, I wouldn't even populate them all in advance, I'd dynamically attach only the scripts that apply for the session, if there are seams between the levels, or if there is a perk shop of sorts. if you need them enabled/disabled in local (continuous gameplay) time, then you can do it as well, without having an overcrowded object to begin with.