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

Physics 4.0 #43073

Closed
wants to merge 4 commits into from
Closed

Physics 4.0 #43073

wants to merge 4 commits into from

Conversation

madmiraal
Copy link
Contributor

@madmiraal madmiraal commented Oct 25, 2020

This PR is part of implementing godotengine/godot-proposals#1384 and the changes that @reduz wants to make to Physics for Godot 4.0.

However, it's important that we first agree the way forward. Therefore, I've created this initial suggestion as a draft PR. It is by no means complete. Instead, it is designed to generate the needed conversation.

The commits currently included in this PR:

  • Collapse PhysicsBody API: Make everything a RigidBody
  • Rename physics_body_* files to rigid_body_*.
  • Move PhysicalBone3D to its own file.
  • Create CharacterBody2D and CharacterBody3D

@pouleyKetchoupp
Copy link
Contributor

I'm all for starting a discussion about refactoring, but this PR and the proposal need to be clarified, especially since they are meant to discuss major changes in the API. The original proposal doesn't have a consensus as for now, and from what I can see this PR doesn't really implement things the way they are described in the proposal.

You need to take the proper steps before starting to discuss the implementation:

  1. Update the proposal to include the changes proposed by @reduz so an informed discussion can take place
  2. The proposal also needs to be more explicit about what use cases / problems it's trying to solve, so a consensus can be reached based on practical needs
  3. Update the PR description with a list of changes and explanations to make it easier to review
  4. Update the PR title to be more descriptive (Physics 4.0 is too vague, it should describe the main goal of the PR)

@yosoyfreeman
Copy link

yosoyfreeman commented Oct 28, 2020

Well, i found the idea of making all rigid bodies is a really, really bad idea. as the other said, there are totally different things and need different approaches. Instead i think the point should be improve the usability of kinematic bodies first. Controlling kinematic bodies is one of the core elements of most games, but in Godot is a really painful task.

@madmiraal
Copy link
Contributor Author

@yosoyfreeman Currently, a KinematicBody is just a PhysicsBody in Kinematic mode and a RigidBody is just a PhysicsBody in Rigid mode:

KinematicBody2D::KinematicBody2D() :
PhysicsBody2D(PhysicsServer2D::BODY_MODE_KINEMATIC) {

RigidBody2D::RigidBody2D() :
PhysicsBody2D(PhysicsServer2D::BODY_MODE_RIGID) {

The commit "Collapse PhysicsBody API: Make everything a RigidBody" simply removes this obfuscation, and makes the KinematicBody specific functions available to RigidBodies in Kinematic mode.

@LikeLakers2
Copy link
Contributor

LikeLakers2 commented Oct 28, 2020

Then calling the node a "Rigid body" (even as a class name) would be misleading -- especially if a setting causes it to no longer act as anything of a Rigid body.

I would also argue that this would become more confusing for beginners if the documentation had to list functions that could only be used under certain modes.

(P.S. I have to agree with yosoyfreeman, this PR title isn't the greatest. The PR is not so much "The next generation of physics in Godot!" as it is a refactoring of the physics nodes. Perhaps you could update the PR title to reflect that?)

@yosoyfreeman
Copy link

yosoyfreeman commented Oct 28, 2020

@yosoyfreeman Currently, a KinematicBody is just a PhysicsBody in Kinematic mode and a RigidBody is just a PhysicsBody in Rigid mode:

KinematicBody2D::KinematicBody2D() :
PhysicsBody2D(PhysicsServer2D::BODY_MODE_KINEMATIC) {

RigidBody2D::RigidBody2D() :
PhysicsBody2D(PhysicsServer2D::BODY_MODE_RIGID) {

The commit "Collapse PhysicsBody API: Make everything a RigidBody" simply removes this obfuscation, and makes the KinematicBody specific functions available to RigidBodies in Kinematic mode.

In my opinion, one of the points of godot is the Node system, that allows you to have small pieces that perform a specific function.
From a user perspective(not only the new ones) i found that mixing the nodes would be confusing and against the simplicity of knowing what is you object simply by looking at the node. Also, even if they are the "same" on the engine side, kinematic and rigidbodies are opposite things in terms of using the engine. (For example, modify the linear velocity is not the same as having a kinematic body with a sliding function that you can use in a lot of custom ways.). Also the pipeline is so different for both. Being dealing with _integrated_forces is so different from just use _physics_process()

Kinematic bodies are mainly used for characters, and currently move_and_slide provide slide, floor detection, snapping etc (not in the current broken state). I think it would be a great idea to expose the floor vector, snap vector etc to the kinematic body (leaving only one function to slide the bodies)

in terms of unify things, i think this kind of things could be exposed on the rigidbodies too. Covering the necesity of some functions, making it more clear to the end user and still avoiding character controllers ( as i understand that the team does not like character controllers).

I hope my opinion is clear. i apologize for my poor english. Also if i did something wrong. Im so new to open source/github and stuff.

have a nice day.

@TokageItLab
Copy link
Member

TokageItLab commented Oct 29, 2020

As noted in #30311 and #42368, the current stop_on_slope of KinematickBody avoids the physics calculation (so it's crude). Currently RigidBody's Mode Kinematic does not implement stop_on_slope, and no amount of tweaking of Mode Rigid's PhysicsMaterial value will stop on the slope. If we are going to integrate KinematickBody in RigidBody, we should to implement a correct way to prevent the player from sliding down the slope.

And... I'm skeptical about the need for CharacterBody. The way the character moves, changes in acceleration, behavior during jumps, landing decisions, etc. vary so much from game to game. It's difficult to integrate them all into a CharacterBody, and it's can by we only have to distribute various sample projects in the Asset Library.

@yosoyfreeman
Copy link

As noted in #30311 and #42368, the current stop_on_slope of KinematickBody avoids the physics calculation (so it's crude). Currently RigidBody's Mode Kinematic does not implement stop_on_slope, and no amount of tweaking of Mode Rigid's PhysicsMaterial value will stop on the slope. If we are going to integrate KinematickBody in RigidBody, we should to implement a correct way to prevent the player from sliding down the slope.

And... I'm skeptical about the need for CharacterBody. The way the character moves, changes in acceleration, behavior during jumps, landing decisions, etc. vary so much from game to game. It's difficult to integrate them all into a CharacterBody, and it's can by we only have to distribute various sample projects in the Asset Library.

I think there is no necessity for a characterbody. If you create a characterbody node, you will need another 4 nodes, kinematic and rigidbody character body 2d and the equivalents for 3D.

Also, the current problem is no confusion or lack or organization. Indeed, merge kinematic body into rigidbodies and add 4 new node will only make the whole problem bigger.

Also, there is no logic reason to start changing or adding this kind of things when the current problem is that there are not solid ways to deal with slopes and snapping. Part of that problems seems to be the physics implementation itself at reduzio says, the other part of the problem is that the moving algorithm does not work.

I think instead of that, the focus should be on fixing the physics regressions and work on the movement function itsealf for kinematic bodies. And if its possible to provide a similar approach for rigidbodies.

That should keep things clear and intuitive and extend the use of the physics bodies (dynamic or kinematic) for using that functions not only for character, but for all kind of moving objects (like projectiles that snap to the ground, etc).

if not, you should use a characterbody to create that kind of super generic moving objects. And i find that messy.

Nice day.

@TokageItLab
Copy link
Member

TokageItLab commented Nov 1, 2020

There are currently several Node and RigidBody's Mode things that have a similar role to both. There are some physics calculations that need to be fixed, but even leaving that aside, I'm agreeing with @reduz and @madmiraal on solving the confusion. However, in order to solve the confusion, we need to

  1. Removing "Mode" of RigidBody and keeping some separate Nodes.
  2. Removing several Nodes and integrating them into the RigidBody's Mode.

We have to choose one between these two.

@madmiraal explained that the current implementation of KinematicBody functions etc. will not be removed, but still maybe @yosoyfreeman wants, if anything, the first solution. It's true that in a Node based game engine, RigidBody having Mode looks strange. So I think @madmiraal has to be clear why the second solution should be chosen (e.g. because there are cases where Mode is changed dynamically in the script, etc.).

@Riteo
Copy link
Contributor

Riteo commented Nov 4, 2020

In my opinion, one of the points of godot is the Node system, that allows you to have small pieces that perform a specific function.

Firmly agree with this one. The idea of merging everything into one node really doesn't sound like in the spirit of Godot.
I have no idea on how the physics engine is built, but the fact that internally they are all the same thing isn't something that nodes should account for.

@bnyu
Copy link

bnyu commented Nov 19, 2020

Can I change a RigidBody2D's linear_velocity very often? I mean is it will performed very well like KinematicBody2D?

For now, this is not recommend, guess there may be performance issues. But I really need this cause a KinematicBody2D does not collide with each other(not just interact with it like with StaticBody2D)

Maybe only solution is custom_integrator?

@Calinou
Copy link
Member

Calinou commented Nov 19, 2020

Maybe only solution is custom_integrator?

If you want behavior that's closer to KinematicBody (no automatically applied gravity), you need to enable custom_integrator. That said, the same recommendation about linear_velocity still applies in this case.

@yosoyfreeman
Copy link

So, as far we discussed here i think we agree with a few things:

-Simplify Rigid bodies (The current modes are just combinations of different common Rigid bodies configurations)

-Reforce Kinematic bodies and movement algorithm (work on a functional sliding algorithm and expose things like snapping, floor vector etc on the kinematic bodies )

This also should be more clear and effective than integrate a new character bodie (wich is a much deeper thing and should not be necesary with correct kinematic physics)

@Riteo
Copy link
Contributor

Riteo commented Nov 23, 2020

-Reforce Kinematic bodies and movement algorithm (work on a functional sliding algorithm and expose things like snapping, floor vector etc on the kinematic bodies )

@yosoyfreeman honestly I find this kind of excessive, IMO the KinematicBody node should stay that, a kinematic body.
For all the bells and whistles used in character controller I think that there should be a more specialized node, as proposed some time ago. This approach could also allow to implement, as community assets, possible alternative implementations of a character controller or anything else, depending on the needs of the user. I also found kind of unwanted to have these utilities(especially move_and_slide) and unused variables(on_floor, IIRC) bound directly to what's in practice the simplest colliding moving object you have out of the box.

So, I propose so split all of KinematicBody's utilities(move_and_slide, floor snapping, etc) into a specialized child node, which could allow more uses and implementation made by the user with only what they want.

Edit: Made my intent clearer

@yosoyfreeman
Copy link

-Reforce Kinematic bodies and movement algorithm (work on a functional sliding algorithm and expose things like snapping, floor vector etc on the kinematic bodies )

@yosoyfreeman honestly I find this kind of excessive, IMO the KinematicBody node should stay that, a kinematic body.
For all the bells and whistles used in character controller I think that there should be a more specialized node, as proposed some time ago. This approach could also allow to implement, as community assets, possible alternative implementations of a character controller or anything else, depending on the needs of the user. I also found kind of unwanted to have these utilities(especially move_and_slide) and unused variables(on_floor, IIRC) bound directly to what's in practice the simplest colliding moving object you have out of the box.

So, I propose so split all of KinematicBody's utilities(move_and_slide, floor snapping, etc) into a specialized child node, which could allow more uses and implementation made by the user with only what they want.

Edit: Made my intent clearer

I think i Am not explaining well. Sorry for my English. I was saying that the Kinematic body should stay as a separate node.

Personally i Don't get the point of creating nodes for the movement functions. A Kinematic body without movement functions is in fact a static body.

As for the character controller node i still think is Unnecesary and no really posible right Now. The current Kinematic body movement is not working, so anything that inherit from the is going to be a work around, wich is what we are trying to avoid.

Again, sorry for my poor language.

@Riteo
Copy link
Contributor

Riteo commented Nov 24, 2020

I think i Am not explaining well. Sorry for my English. I was saying that the Kinematic body should stay as a separate node.

Don't worry, I got what you mean and I think I explained myself the wrong way. I too think that KinematicBodies should stay separate from RigidBodies, but I also think that all utility(not essential) methods are excessive and should be moved in a separate extending node.

Personally i Don't get the point of creating nodes for the movement functions. A Kinematic body without movement functions is in fact a static body.

I'm not saying to remove all of its current methods, only all of them that aren't strictly necessary. Let me explain:
Imagine that you're creating a fancy new FPS character controller based on KinematicBody that you want to share it with people; Since it had some special requirement, you made every single movement method from scratch, not using move_and_slide or any other utility that came with the class with the exception of move_and_collide. Now you have a lot of unused/potentially breaking functions which you have to tell people about to not use, or use some workaround in order to make sure that people can't call them in the first place. Wouldn't it be nicer if KinematicBody had only the strict necessary to create a moving, simple colliding body?

As for the character controller node i still think is Unnecesary and no really posible right Now. The current Kinematic body movement is not working, so anything that inherit from the is going to be a work around, wich is what we are trying to avoid.

Yeah, currently some core behaviour of KinematicBodys is pretty broken(See #35945), but still IMO all of the other utilities functions are fluff, especially move_and_slide which also changes some public variables. I think that every node should be specialized in doing only one thing very well(within reason) and in the case of KinematicBody, that one thing is to be a simple body that isn't affected by physics; As of now I think that KinematicBodies lean more towards a CharacterBody. I've also heard a bunch of times the suggestion of renaming KinematicBody to CharacterBody, KinematicCharacter or similiar alternatives, so I'm inclined to think that I'm not the only one here that thinks this way.

tl;dr: I'm not saying that all of the utilities inside a KinematicBody are useless and should be removed from Godot Engine itself, but that should instead be moved into a node that extends it, since IMO that goes outside of its direct scope, being it the simplest body you can get out of the box on Godot.

Again, sorry for my poor language.

You're actually pretty clear and understandable. I'm not a native speaker either, so the issue probably comes from my side.

@yosoyfreeman
Copy link

yosoyfreeman commented Nov 25, 2020

Okay i think i get your point now. My biggest fear with renaming "KinematicBody" to "CharacterBody" is that a kinematic body can be used as much more than characters. For example, they are used frequently to create platforms, obstacles and any kind of object with a specific movement, such as bullets, props and other things. So we would need another node (characterbody). But in my experience trying to offer an all in one character objects is extremely difficult due to the immense differences between games.

I am not against the creation of a character node, but i think that should be the next step on the chain.

Thats why i said that we should focus on "fixing" the current physics issues and the sliding method. (Andrea catania did an awesome implementation with bullet) but maybe a simplier method.

Once we have a working kinematic body core (Movement and snapping) we will be more ready to approach more specific things like step offset and where to put them (on a character body or as separated nodes, or inside the kinematic body)

@Riteo
Copy link
Contributor

Riteo commented Nov 25, 2020

My biggest fear with renaming "KinematicBody" to "CharacterBody" is that a kinematic body can be used as much more than characters.

Exactly, that's why I proposed not to rename it, but to create an extending node(CharacterBody) and move its utility methods in there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants