Fix deceleration in CharacterBody 2D and 3D templates

[ubitux]

#73873 is stalled for more than a year, so I thought addressing small but concrete parts of the problem could help making progress.

Physics squad edit:

• Fixes CharacterBody2D template uses move_toward incorrectly #74216

[dalexeev]

The GDScript code style looks good to me, but I think it's up to the physics team to decide.

[rburing]

Let's replace the literal 0 by 0.0 everywhere.

[ubitux]

Done in a separate commit as it is unrelated to this change. I also replaced the other 0 for the input vector swizzling, I hope that's ok?

[AThousandShips]

No need to keep separate commits, you'll have to squash into one at the end anyway

[ubitux]

Just curious, is there an issue with keeping the 2 independent and functional commits?

[AThousandShips]

Because we keep things to one commit when ever possible, to not clutter things and makes managing things easier, what's the issue with having just one commit for one change?

You don't have to keep one while working, just for the final fixup so it can be merged

For more details see here

[ubitux]

what's the issue with having just one commit for one change?

It's not a single change since it's cosmetics on top of a fix, and I'm touching code that's unrelated to the fix itself (for example the input vector).

Also, It makes reverting or cherry-picking only the fix harder. It is also confusing to see mixed changes along with a fix, as it makes you question whether 0 vs 0.0 was actually part of the problem

[AThousandShips]

For these purposes it's a single change, both are part of the same change, if the two need to be separate they should be two PRs instead, the changes aren't going to be reverted separately as the change to float constants is perfectly safe

There are times when a PR needs multiple commits because of the scope of the change, this one is not one of them

[ubitux]

Your call, squashed 🤷

[rburing]

The bugfix this PR does is definitely welcome. I'm not completely sure about the name "deceleration" though, since it is a number and not a vector. (But maybe simplicity is more important than mathematical accuracy.)

Sorry for all the nitpicking but this code will be seen by many first-time users, so we want to get it right.

[ubitux]

The bugfix this PR does is definitely welcome. I'm not completely sure about the name "deceleration" though, since it is a number and not a vector. (But maybe simplicity is more important than mathematical accuracy.)

Sorry for all the nitpicking but this code will be seen by many first-time users, so we want to get it right.

I don't mind renaming that, what do you suggest? Slowdown? MomentumForce?

Changing into a vector feels like it's going to open a can of worms to me, but your call.

[AThousandShips]

Changing into a vector feels like it's going to open a can of worms to me, but your call.

I think the point is that deceleration isn't a scalar but a vector, so the name not the type is in question

[ubitux]

Changing into a vector feels like it's going to open a can of worms to me, but your call.

I think the point is that deceleration isn't a scalar but a vector, so the name not the type is in question

Sure, I'm happy to rename to anything, please let me know what you'd prefer.

[ubitux]

Also, alternatively, we could use the incoming move_toward_smooth() (#92236) unconditionally; something like:

velocity.x = move_toward_smooth(velocity.x, direction.x * SPEED, delta * STEEPNESS)
velocity.z = move_toward_smooth(velocity.z, direction.z * SPEED, delta * STEEPNESS)

This handles both the acceleration and the deceleration at the same time and actually simplifies the code by removing the branching

[AThousandShips]

That'd be something for a separate PR, would be good to get this fix in 4.3 and the other method won't be in 4.3, I'd also say it shouldn't be added to the template until it's been tested for a while after merger

[patwork]

godot/modules/gdscript/editor/script_templates/CharacterBody2D/basic_movement.gd

Lines 22 to 26 in fc6d8e6

	var direction := Input.get_axis("ui_left", "ui_right")
	if direction:
	velocity.x = direction * SPEED
	else:
	velocity.x = move_toward(velocity.x, 0.0, DECELERATION * delta)

Why do we even need smooth deceleration here if there is no smooth acceleration? Changing velocity.x from 0 to SPEED is instantaneous.

Why not just https://docs.godotengine.org/en/latest/tutorials/physics/using_character_body_2d.html#platformer-movement

	# Get the input direction.
	var direction = Input.get_axis("ui_left", "ui_right")
	velocity.x = direction * speed

[AThousandShips]

The deceleration is a standard syntax that's used in various places, if anything I'd say to add acceleration as well, but many games just have immediate speedup, but out of scope for this which just fixes the code to match what was originally intended

[patwork]

if anything I'd say to add acceleration as well

Even given that this is intended to be a simple character controller, making available the possibility to set both acceleration and deceleration rates seems preferable.

Example from kidscancode, noteworthy also for showing how to enable modification of values in the inspector (@export).

https://kidscancode.org/godot_recipes/4.x/2d/platform_character/index.html#friction-and-acceleration

[ubitux]

The point of this PR is to fix the code that has been broken for years, not rewrite it. A PR to rewrite it has been stalled forever, so the idea here is to make the code correct so that it can later be improved starting from a correct foundation.

There are many opinions on how the default template could be, and this could take a very long time before reaching a consensus, but I don't think it should prevent important fixes in the meantime.