godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.1k stars 69 forks source link

Average floor normal while in contact with multiple valid floor planes on `move_and_slide()` #9254

Open yosoyfreeman opened 5 months ago

yosoyfreeman commented 5 months ago

Describe the project you are working on

Godot general physics improvements

Describe the problem or limitation you are having in your project

As is done correctly here, walls will be averaged to check if they constitute a valid floor plane.

The problem is that here we just simply iterate until we find one.

The result is that the consistency of the floor is higher between two walls than between two valid floors. The floor normal between two floors is inconsistent and will flicker directions.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Average all the valid floor planes to provide a better representation of the floor as already done with walls.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Exactly like is done with the walls.

If this enhancement will not be used often, can it be worked around with a few lines of script?

It will be used often.

Is there a reason why this should be core and not an add-on in the asset library?

It is expected behaviour.

Calinou commented 5 months ago

This makes sense to do as a default, but are there any downsides to averaging all valid floor planes? I wonder if there are specific use cases where doing this is undesired.

yosoyfreeman commented 5 months ago

I personally don't think there are any downsides. The floor plane becomes abstract at the moment we average walls to create a new imaginary floor plane, so I believe the abstraction is valid here too.

On the other side, is easy enough to iterate over the slide collisions to check if any specific plane was a floor itself, while modifying what is reported by get_floor_normal is not possible.

It can always be an optional parameter set to true to average them. In the really specific case you don't need it, we just provide the most recent one like we currently do. But i do think it makes sense as a default, as is often what users expect and need to work with in order to calculate more accurate custom movement.