Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify the PhysicsBodies interface and its methods #1384

Closed
madmiraal opened this issue Aug 18, 2020 · 22 comments
Closed

Simplify the PhysicsBodies interface and its methods #1384

madmiraal opened this issue Aug 18, 2020 · 22 comments

Comments

@madmiraal
Copy link

Describe the project you are working on:
Any game that uses PhysicsBodies.

Describe the problem or limitation you are having in your project:
Currently, Godot has three types of PhysicsBody: RigidBody, KinematicBody and StaticBody. RigidBody also has four modes: Rigid, Character, Kinematic and Static. Not only is this confusing, but there are functions available in KinematicBody that are not available to a RigidBody and visa versa. Furthermore, functions like KinematicBody's move_and_slide() have lots of parameters that can be confusing and don't always do what is expected (most notably stop_on_slope).

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
A KinematicBody is just a RigidBody with infinite mass (or zero inverse mass). A StaticBody is just a KinematicBody with zero (or constant) velocity. The same way a RigidBody in Character mode doesn't rotate (because it effectively has infinite rotational inertia, or zero inverse rotational inertia). Furthermore, sliding and snapping are really just PhysicsMaterial properties, and stop_on_slope can be programmed in to work as expected.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
A RigidBody already has the modes needed, so we just need to make the functions that are available to KinematicBody available to a RigidBody (in Kinematic mode). At the same time, since all of the parameters to the move_and_collide(), move_and_slide() and move_and_slide_with_snap() can be made properties of the PhysicsBody, the PhysicsMaterial, or, in the case of the up_direction and floor_max_angle, the Space or Area. These methods can then be removed, because they will no longer be needed.

If this enhancement will not be used often, can it be worked around with a few lines of script?:
No, this will actually simplify scripts.

Is there a reason why this should be core and not an add-on in the asset library?:
This is a change to how core physics works.

Finally, its worth noting that these changes should make implementing some of the other proposals (e.g. #236, #364, #628, #667 #740) easier.

@TheDuriel
Copy link

TheDuriel commented Aug 18, 2020

What you're proposing sounds similar to what Unity does/did, Which is to make everything a Rigidbody and let the devs deal with the pains that causes when you want custom integration.

I'd rather we rename the RigidBody modes, (since the Kinematic Mode is in fact a Kinematic__s__ mode, not a KinematicBody mode.) than to make vast changes under the hood to how these bodies are treated.

A kinematic body is NOT a rigidbody with infinite mass or similar. And it should really stay that way.

@madmiraal
Copy link
Author

@TheDuriel I'm not suggesting that we remove any of the functionality, I'm suggesting simplifying the current implementation. For example, instead of using move_and_slide(linear_velocity) with a KinematicBody, you would just change the linear_velocity property of a RigidBody in Kinematic mode. Similarly, all the other parameters of move_and_slide() would just be properties that tend to remain constant, but can be changed when required.

@TheDuriel
Copy link

That would literally not lead to the same behavior.

@madmiraal
Copy link
Author

That would literally not lead to the same behavior.

If it's implemented correctly it would.

@TheDuriel
Copy link

How would one, in this new system, implement responses to individual collision events as you are causing them?
RigidBodies by their very nature do no expose this.

@AndreaCatania
Copy link

AndreaCatania commented Sep 8, 2020

I like the idea to merge RigidBody and StaticBody. I'm worrying about merging also the KinematicBody.

As I envision it, the kinematic body algorithm should be 100% customizable, and be able to just set the body velocity is really not enough.
There are games where you want to change stance, climb walls, follows paths, etc... All things that require a much bigger control over the kinematic algorithm.

If a game just needs a character that walk around, can always use a RigidBody with the mode set as Character and change the linear velocity.

@nezvers
Copy link

nezvers commented Sep 14, 2020

For RigidBody (and 2D) it's painful for player-controlled objects that it doesn't have an API for detecting being on the ground, getting collision normal. For all that need to get PhysicsDirectSpace and do IntersectShape (for which normal is available only for closest collision) or IntersectRaycast (for which needs many casts or it doesn't detect full shapes bottom collision cases).

@TheDuriel
Copy link

You actually don't. The State of the rigidbodies _integrate forces provides the necessary data without additional raycasting being needed.

@nezvers
Copy link

nezvers commented Sep 14, 2020

You actually don't. The State of the rigid bodies _integrate forces provides the necessary data without additional raycasting being needed.

Yes, but that's the problem. Why for simple movement I need to get into another callback that works async?
I've used it plenty and it's unnecessary to use it if I don't need to change linear_velocity on the go.

@TheDuriel
Copy link

If you're using a RigidBody you are already past the point of simple movement. And its one line of code to get the collision normals dot product and compare it. So its not much different from what a kinematic body offers. In fact, you could even wrap it in your own is_on_floor(state) function.

@nezvers
Copy link

nezvers commented Sep 18, 2020

If you're using a RigidBody you are already past the point of simple movement. And its one line of code to get the collision normals dot product and compare it. So its not much different from what a kinematic body offers. In fact, you could even wrap it in your own is_on_floor(state) function.

I have gone also very not-simple roads on RigidBody and I know well how unpleasant it can be.
I already have made my own is_on_floor() methods but it shouldn't be like that. Now I have few lines in _physics_process to move rolling ball character, but to limit jumping only from the floor I need to create bigger code in _integrated_forces or parse information from direct space to know the normals of collisions. KinematicBody have direct access to KinematicCollision, why RiggidBody can't have it too? Not only normals are needed but collision force too.

Because of such lack of access from RigidBody2D I once was forced to create my own RigidKinematicBody2D.

@mini-glitch
Copy link

Merging RigidBody and StaticBody is fine, I don't think there's any real need for those to be separate.

But for KinematicBody setting linear_velocity instead of move_and_slide() is not a valid replacement. move_and_slide() et al, by nature of being functions, have some inherent functionality that isn't replicable by just setting a property. move_and_slide() calculates a new velocity for the body and returns it. This return value can be used as an event, allowing you to immediately respond to the changes that occurred, functionality otherwise missing from KinematicBody. The function move_and_collide() returns collision data, and can optionally not actually perform the movement but just test a potential movement. This is incredibly useful for precisely controlled character movement, especially when combined with the fact that since it's a method, you can call it multiple times in the same physics step. Example use cases being: doing multiple tests before choosing one to commit to or handling multiple bounces in one physics step for an extremely fast moving object.

@LikeLakers2
Copy link

That would literally not lead to the same behavior.

If it's implemented correctly it would.

I'm somewhat bothered by this reply. Yeah, if something is implemented a certain way, it could lead to the same behavior as another way... but you could say the same about using many Sprite3D nodes to build a character model.

So... I'm kinda curious, and kinda suspicious of this answer. How much research have you done on this point? Have you actually tested that a certain implementation would lead to the same results?

@name-here
Copy link

It sounds like merging RigidBody and KinematicBody would remove a basic but customizable/extensible thing, and replace it with something complex, where you have to disable or work around all of its extra features if you want to make something custom. That doesn't sound like an improvement to me.

@yosoyfreeman
Copy link

yosoyfreeman commented Oct 28, 2020

I think there is a really bad idea to merge kinematic bodies into rigidbodies.

@bnyu
Copy link

bnyu commented Nov 18, 2020

For now, KinematicBody2D can only push RigidBody2D, and the RigidBody2D will keep the velocity.

I would like set a KinematicBody2D's linear_velocity directly(or use move_and_slide), and it will push another KinematicBody2D away that will not keep the velocity.

Can it be easily applied after this change?

@AndreaCatania
Copy link

@bnyu This can be integrated. Though, if you need a KinematicBody that can be pushed by one or many RigidBodies, most likely you don't want to use a KinematicBody but a RigidBody in Character mode. Probably you need to abstract some functionality on top of the RigidBody to move it as you desire and don't allow that it slides down slopes.

In order to allow a RigidBody to push a KinematicBody it's required to add a dynamic algorithm, similar to the one the physics engine already provides for RigidBody. Since we already have that, the best thing would be just do the opposite: add the feature I propose few lines above to the RigidBody.

@Bropocalypse
Copy link

Bropocalypse commented Dec 3, 2020

What looks simpler for the engine developer isn't necessarily good for the game developer. Sure, merging these things would make some things easier for the former, but it would cause great confusion for the latter.
I don't think it's a good move to ask game devs to start with something complex and break it into pieces to extract what they need- Rather, it is in the spirit of Godot's node system to start with small basic parts and bring them together into something that behaves in complex ways that they can expect. Physics engines are exceedingly complex, and it can cause a lot of problems for the unwitting dev if they're asked to implement pieces that they can't tell offhand whether they are or are not relevant to the problem they're trying to solve. "Here is a physics object- Now, disable this, disable this, disable this, don't use this function because it won't affect anything even though its name makes it sound like it would, don't use this function because even though its name sounds like it does what you want it actually will counteract what you want, and ignore all these things you see in the inspector. And what were you thinking even asking about THIS thing over here?" That's the kind of advice you get when trying to find out why the little pixelman isn't moving correctly on the screen.
In a perfect world that, while we cannot reach it, but can aspire to approach it, the game dev would be able to simply assemble small parts relevant to their project and then refine the micro of its behaviors into what they want.

@madmiraal
Copy link
Author

To clarify, the proposal is to simplify things for the game developer (not the engine developer). For example, currently, a KinematicBody and a RigidBody in Kinematic mode are the same thing under the hood, but the game developer is able to do things with a KinematicBody that they can't do with a RigidBody in Kinematic mode and visa versa. This proposal is about merging the two; so that a RigidBody in Kinematic mode can do everything that both can do at the moment.

Similarly, functions like move_and_collide(), move_and_slide(), etc. will be merged and controlled via properties. Instead of having separate functions with lots of different parameters (currently, for example, one takes distance and the other velocity) for different outcomes, the outcome is decided by, for example, the PhysicsMaterial properties: should it bounce or not; should it slide or not; and if so, by how much.

@TheDuriel
Copy link

This doesn't make anything easier for game or engine developers. Kinematic bodies represent an abstract API. Merging it with RigidBodies which literally do different stuff all together does not make this API easier to use.

The Kinematic mode of a RigidBody is not identical to a KinematicBody. Nor is it intended to be used as one. It is there for when you need to momentarily acquire the properties of the Mode.

The only thing I see happening here is: Two relatively huge classes get merged into one big mess. The API becomes harder to use due to an abundance of properties and functions you won't care about most of the time. And the behavior of Kinematic Bodies be literally different.

@name-here
Copy link

Merging all the PhysicsBody types is like merging Node2D and Control. They don't do the same thing. They're not supposed to do the same thing. And you either loose functionality that users have to re-implement in script/code, or you end up with a bloated Node with too many options you don't care about most of the time.

Well we're at it, why don't we make all the Joint types just one node with different options? Make the Light types one node? Combine CPUParticles and GPUParticles? (this one might be okay, though, since they're for almost the same thing) Considering how many different nodes are separated out by functionality so they don't all have a bunch of extra options you just ignore for most use cases, this seems not only bad for usability, but also inconsistent with the rest of the engine's design.

The entire point of KinematicBody is that it doesn't have physics interactions built-in, and it lets you implement your own using move_and_slide() and such. This is especially useful in things like 2D platformers, where custom (inaccurate) physics for things like the character jumping often feel better to control. Why would we want to make people disable a bunch of options that don't apply for simple use cases like this?

It seems more reasonable to remove RigidBody's extra modes. Though, as @TheDuriel mentioned, they can be used for PhysicsBodies that need to change modes while the game is running, so maybe just update the docs to mention what they're good for? Additionally, from what other people said, it seems they don't behave quite like their Node counterparts, as the docs claim. Maybe this is causing some of the disagreement?

Also, I just remembered one thing that StaticBody2Ds can do that other PhysicsBodies can't: you can scale them using their transform and it scales them up correctly. This contradicts the docs, but it works in practice (docs should probably be updated rather than "fixing" this).

@madmiraal
Copy link
Author

Superseded by #2184.

@aaronfranke aaronfranke closed this as not planned Won't fix, can't repro, duplicate, stale Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests