# Question weird issue with rigidbody - AddForce.Impluse causes constant movement with a movement function

Discussion in 'Physics' started by SassyPantsy, Feb 3, 2021.

1. ### SassyPantsy

Joined:
May 17, 2020
Posts:
141
Weird Issue. Code at the bottom.
So I have a rigidbody and I'm trying to implement movement and jumping. Each function works, but once I implement movement, instead of working as intended, the jumping method launches the object upwards without stopping. No matter what ForceMode I use, no matter if I even use one - any function that's relating to moving the object, once the input movement method is inserted, works as if I was simply writing Vector3.up in Update.

Code (everything's in update):
Code (CSharp):
1. void Update()
2.         {
3.            //3d movement input calculation
4.             x = Input.GetAxisRaw("Horizontal");
5.             y = Input.GetAxisRaw("Vertical");
6.
7.             Vector3 movePosition = transform.right * x + transform.forward * y;
8.             Vector3 newPosition = new Vector3(movePosition.x, rb.velocity.y, movePosition.z);
9.
10.             //jump method - works if nothing else movement related is implemented
11.             bool grounded = Physics.CheckSphere(new Vector3(transform.position.x, transform.position.y - 1, transform.position.z), 0.4f, layerMask);
12.             if (Input.GetKeyDown(KeyCode.Space) && grounded)
13.             {
15.             }
16.
17.
18.             if (Input.GetKey(KeyCode.LeftShift) && Input.GetKey(KeyCode.W))
19.             {
20.                 charging = true;
21.                 acceleration = 6f;
22.                 if (currentSpeed < chargeSpeed)
23.                 {
24.                     currentSpeed += Time.deltaTime * acceleration;
25.                     rb.velocity = newPosition.normalized * currentSpeed;
26.                 }
27.                 rb.velocity = newPosition.normalized * currentSpeed;
28.                 acceleration = 2f;
29.             }
30.             else
31.             {
32.                 charging = false;
33.                 if (currentSpeed > moveSpeed || Input.GetKeyUp(KeyCode.LeftShift))
34.                 {
35.                     currentSpeed -= Time.deltaTime * deceleration;
36.                     rb.velocity = newPosition.normalized * currentSpeed;
37.                 }
38.                 else
39.                 {
40.                     if (currentSpeed < moveSpeed)
41.                     {
42.                         currentSpeed += Time.deltaTime * acceleration;
43.                         rb.velocity = newPosition.normalized * currentSpeed;
44.                     }
45.                 }
46.
47.             }
Thanks.

2. ### AlTheSlacker

Joined:
Jun 12, 2017
Posts:
326
Try this:

Code (CSharp):
1. using System.Collections;
2. using System.Collections.Generic;
3. using UnityEngine;
4.
5. public class Movement : MonoBehaviour
6. {
7.     [SerializeField] private float moveAcceleration;
8.     [SerializeField] private float chargeAcceleration;
9.     [SerializeField] private float moveSpeed;
10.     [SerializeField] private float chargeSpeed;
11.     [SerializeField] private float jumpForce;
12.     private Rigidbody rb;
13.     private bool jumpNow = false;
14.     private float acceleration;
15.     private Vector3 targetMoveVelocity;
16.
17.     private void Start()
18.     {
19.         rb = GetComponent<Rigidbody>();
20.     }
21.
22.
23.     void Update()
24.     {
25.         //2D movement input calculation
26.         float x = Input.GetAxisRaw("Horizontal");
27.         float z = Input.GetAxisRaw("Vertical");
28.
29.         acceleration = moveAcceleration;
30.         float speedLimit = moveSpeed;
31.
32.         // charge mode only available when using shift and forward, this will also include forward diagonal
33.         if (Input.GetKey(KeyCode.LeftShift) && Input.GetKey(KeyCode.W))
34.         {
35.             acceleration = chargeAcceleration;
36.             speedLimit = chargeSpeed;
37.         }
38.
39.         // calculate requested move velocity
40.         targetMoveVelocity = (transform.right * x + transform.forward * z).normalized * speedLimit;
41.
42.         // jump request detection
43.         if (Input.GetKeyDown(KeyCode.Space)) jumpNow = true;
44.
45.     }
46.
47.
48.     private void FixedUpdate()
49.     {
50.         // general guide, physics interaction should take place in FixedUpdate
51.
52.         // test if grounded (assumes player is in layer 3 - important to avoid testing against the player's own collider)
53.         bool grounded = Physics.CheckSphere(new Vector3(transform.position.x, transform.position.y - 1, transform.position.z), 0.05f, 3);
54.
55.         // if jump requested and grounded then jump
56.         if (jumpNow && grounded) rb.AddForce(Vector3.up * jumpForce, ForceMode.Impulse);
57.
58.         // several FixedUpdates may run before the next Update, so stop jump here
59.         jumpNow = false;
60.
61.         // adjust rb velocity for requested input, you may wish to enclose this in a grounded check
62.
63.         //if (grounded)
64.         //{
65.
66.         // set target velocity to include any y component from jumping / falling
67.         Vector3 targetVelocity = new Vector3(targetMoveVelocity.x, rb.velocity.y, targetMoveVelocity.z);
68.
69.         // velocity delta and ForceMode.VelocityChange guarantees the change,
70.         // you may prefer to use a force to allow the player to be moved by external forces
71.         Vector3 velocityDelta = new Vector3(targetVelocity.x - rb.velocity.x, 0 , targetVelocity.z - rb.velocity.z) ;
72.         rb.AddForce(velocityDelta * acceleration * Time.fixedDeltaTime, ForceMode.VelocityChange);
73.
74.         //}
75.
76.     }
77. }
Make sure your player collider has a rounded base e.g. capsule or sphere.

Some tips:
Physics in FixedUpdate()
Input in Update() [I believe you can get axis data in FixedUpdate, but for simplicity and consistency, I suggest stay in Update]
Be careful with your variable naming, if you catch yourself generating a velocity from a scalar of a normalized position you should be questioning what is happening there.
Whilst you can directly change the rb velocity, I would recommend using AddForce instead so that the next physics cycle includes a consistent set of forces and outcomes.

SassyPantsy likes this.
3. ### SassyPantsy

Joined:
May 17, 2020
Posts:
141
THANK YOU.
Not only did it work, I actually managed to finally learn something.
I do have a few questions, and I'd be very happy for answers, because I'm learning for 7 months by myself, and I'm having a really hard time finding knowledge that isn't just the code itself (Fixed Update is a good example to something I never see done, and constantly hear about). If you don't have time for answering everything, I'd be very happy if you could recommend good resources for this knowledge (expecially one for decelerating after .AddForce lol).

1. Why are you declaring float x&z in Update? considering they're calculated every frame and in multiple places, wouldn't it be more optimal to cache them as state variables?
2. Why set everything to private? What's the difference between that and leaving it as default (which is nothing)?
3. Considering Input.GetKeyDown is already a bool, why not assign it's value directly to jumpNow? Is there a specific reason for the if statement?
4. I didn't understand the meaning of "important to avoid testing against the player's own collider", and I'd be happy to know what you meant. Also, my player is 2 units tall, and the .CheckSphere's position was 1 unit below him, so I've set the .Check Sphere's values to the original transform.position, which worked. Wondering if there's a reason for that (my player has a collider on itself, not on the child 3D model component).
5.why are you detracting targetVelocity.z by rb.Velocity.z, and why aren't you doing the same things for the .x values? also, if you've gone trough the trouble of placing rb.velocity.y in the target vector, did you set the y values on the velocityDelta as 0?
6. Why sperate targetVelocity from targetMoveVelocity? why not combine them and do the calculations in Update? is it because of the preference of completely separating Physics calculations?

And last question - Do you take students?

4. ### AlTheSlacker

Joined:
Jun 12, 2017
Posts:
326
Well, I'm just an amateur at this, so someone will probably turn up and explain why I'm wrong, but here goes...

1. I'm declaring x & z in Update as they are only used in Update. It is good coding practice to keep the scope (where variables are accessible) to the absolute minimum, in this case just the Update method. This prevents you accidentally changing them or accessing the wrong variable from another part of your code.

2. The default scope within a class is private, so you are right, you don't need to type it, I just like to remind myself what I'm doing.

3. Input.GetKeyDown is a bool. It is the result of keyboard input and is trapped once per frame (i.e. at the Update() frequency), which is why I'm checking for it in Update. I have never tested whether you can set its value by code, but as it is indicating the state of a button press it is not something I would want to do outside of test automation. For example, if another method is also looking at the same Input.GetKeyDown you then have a race condition between the two methods, even in different classes (check my minimum scope comment above). The next bit may seem a bit weird... For reference https://docs.unity3d.com/Manual/ExecutionOrder.html

The frame rate may vary depending on user settings and load, so the frequency that Update runs at cannot be considered a constant. The FixedUpdate method runs once for each physics cycle. The duration of a physics cycle for the purpose of the physics calculations is fixed at Time.fixedDeltaTime, but the frequency that the FixedUpdate method runs at is at the mercy of computational load. It is entirely possible to run multiple FixedUpdates without running any Updates and vice-versa depending on project settings and performance. So the safest assumption to make is that if it can get into a race condition between the two, it will.

For this reason it is very convenient to capture the state of the button in Update and flag this state for action in the next FixedUpdate. Equally you can see the requirement to reset that flag once it has been acted upon in FixedUpdate - in case FixedUpdate runs again before we get to re-test the input in Update (double jump).

4. The Physics.CheckSphere is looking for colliders if it finds one it is assuming it is the ground (there are more sophisticated approaches, but this will do). If you have a capsule collider around your player, you don't want to check that and flag it as being grounded. The player's collider can be on any part of that heirarchy of game objects, you just need to avoid testing against it. Sorry it is not clear for me what your original setup was.

5. targetVelocity is the velocity vector that the player would like and the rigidbody already has a velocity vector, subtracting one from the other gives you the required change in velocity (often referred to as a delta).

I'm not sure I understand your comment "why are you detracting targetVelocity.z by rb.Velocity.z, and why aren't you doing the same things for the .x values"... my line reads "(targetVelocity.x - rb.velocity.x, 0 , targetVelocity.z - rb.velocity.z)". Did you mean "y"? y is the up/down part of the global vector, so gravity and jump are sorting that out, you don't want to change it here, hence the delta value of 0. It's a fair question, if I am going to do that, why did I bother putting rb.velocity.y in the targetVelocity vector? Well, I'm overly focused on making sure my variables contain the data indicated by their name and the target velocity really needs to include whatever the current rb.velocity.y is (even though at the moment I don't want to change it). But if I do decide to use it later on it will be what it says it is, which may spare me an unexpected bug.

6. targetMoveVelocity is purely determined by user input so I work it out at the same time as I get the user input. targetVelocity includes the current rb.velocity.y so needs to be calculated in FixedUpdate. You are right though, if performance was everything here, you could set the y value to 0 and just use the targetMoveVelocity. Equally I could have just used a Vector2 and chosen a different name and stopped worrying about it. I guess I was on autopilot for a quick solution and didn't bake in some code refactoring

LOL I think you could find many more proficient teachers than me here!

SassyPantsy likes this.
5. ### AlTheSlacker

Joined:
Jun 12, 2017
Posts:
326

Working out forces for deceleration.

All you really need to remember is Force = mass x acceleration

And that you can get acceleration from SUVAT equations https://en.wikipedia.org/wiki/Equations_of_motion
Scroll down past the nasty stuff to "Constant translational acceleration in a straight line" and you will see some simple equations that you can re-arrange to get a =(v - u) / t. Remember that forces are just applied for Time.fixedDeltaTime, (t)

SassyPantsy likes this.
6. ### SassyPantsy

Joined:
May 17, 2020
Posts:
141
You called yourself amateur, but I don't know by what standards lol. Thanks for the help and for the lengthy reply, made a lot of things a lot more clear. I'll look into SUVAT equations to figure out the necessary ingredients for the 3D movement controller, guess escaping math isn't a valid option anymore

AlTheSlacker likes this.