Add optional delta parameter to move_and_slide

[monitz87]

This overrides the implicit delta derived from context that is currently being used. If not passed, the method functions as it normally does. This change allows finer control over the method's behavior without sacrificing ease of use.

Explanation for why this is necessary here

This is my first contribution to the project, so I apologize in advance if I botched something. I read the contribution guidelines but there's a chance I missed something

[AThousandShips]

Thank you for your contribution!

You need to add the new parameter to the documentation, use --doctool to generate this is the easiest way, see here

[monitz87]

Thank you for your contribution!

You need to add the new parameter to the documentation, use --doctool to generate this is the easiest way, see here

Done. My bad for missing that part 😅

[AThousandShips]

What happens when delta is 0.0? This shouldn't happen with the current behaviour so feeding it 0.0 might break things, the check should probably be delta <= 0 instead

[monitz87]

Fair enough. Should I just make the default 0.0?

[AThousandShips]

I think -1 is clearer so leave it as that

[monitz87]

Should probably document that passing non-positive delta leads to unexpected behavior

[AThousandShips]

That it uses the default one yes, that it ignores it, not necessarily unexpected behaviour, should be clear what it does

[AThousandShips]

Might want to change the check to !(delta > 0) to catch NaN but not sure if needed

Will do more style and detail review later but will leave for testing for right now, can test and investigate when I have more time

[monitz87]

Edit: my bad should have been no trailing zero

Why is that? In the current docs I see examples of both

[AThousandShips]

Unsure, it's stripped by the generator I think, which is an issue, can't find it right now though

Edit: might have updated since then, had it backwards in any case

[AThousandShips]

The current thing that's failing is compatibility related, a bit of a complex thing to add so no rush with that right now, will guide you through it when reviewers have gone through the feature itself

[Calinou]

This doesn't sound sufficient on its own to perform rollback. If you're catching up with server lag, performing move_and_slide() once with a really large delta is different from performing many physics steps at once (it will be much less precise). The original proposal is explicitly asking for the simulation of multiple physics frames at once.

[monitz87]

The current thing that's failing is compatibility related, a bit of a complex thing to add so no rush with that right now, will guide you through it when reviewers have gone through the feature itself

Thanks :D

[monitz87]

This doesn't sound sufficient on its own to perform rollback.

It's not sufficient on its own, but it's still a necessary step to be able to use move_and_slide together with rollbacks, and even if stepping the physics engine wasn't a thing, it still doesn't make much sense to not allow users to have control over the delta. This brings the API closer to move_and_collide and it makes it more intuitive to boot.

The original proposal is explicitly asking for the simulation of multiple physics frames at once.

Technically, this is a separate issue from being able to step the physics engine, even if they're related to the same use case, so it made sense to me to raise a separate PR for it.

If you're catching up with server lag, performing move_and_slide() once with a really large delta is different from performing many physics steps at once (it will be much less precise).

PS: just to be clear, the comment this PR refers to doesn't allude to performing move_and_slide once with a really large delta. It refers to performing it once per physics frame with a fixed delta, but having the engine tick faster, so that the client actually simulates the game faster and sends inputs to the server at a higher frequency when the server signals that it is being starved

[nicobatty]

I was actually wondering if anyone suggested this change! I did run into this problem myself and noticed it was also mentioned as a caveat in a network library from @foxssake called netfox.

[WantToSignUp]

This doesn't sound sufficient on its own to perform rollback. If you're catching up with server lag, performing move_and_slide() once with a really large delta is different from performing many physics steps at once (it will be much less precise). The original proposal is explicitly asking for the simulation of multiple physics frames at once.

With this PR, multiple simulation steps for character movement could be easily implemented by just calling move_and_slide() multiple times in a for loop, with smaller delta, no?
Multiplayer games may implement their own fixed timestep movement code outside the _physics_process(which has some quirks thanks to the way it keeps delta stable), so custom delta is much needed for move_and_slide().

[Synthetic-Dev]

@monitz87 I thank you for this contribution, glad to see this feature is finally getting added after the over 2 years of discussion about it in issues! Been banging my head against a wall trying to figure out why I was getting constant desyncing in my netcode to find that move_and_slide was the culprit.