-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Restore RigidBody2/3D, SoftBody names in physics #64894
Restore RigidBody2/3D, SoftBody names in physics #64894
Conversation
7424aaf
to
d4ac774
Compare
Header guards for SoftBody need fixing: https://github.com/godotengine/godot/runs/8022198750?check_suite_focus=true |
You'll have to remove the rename from the project converter: https://github.com/godotengine/godot/runs/8022198755?check_suite_focus=true |
f315e40
to
468cbcd
Compare
This may sound a bit picky, and that's because it is, but is this both of my previous PRs merged together? As in, does it also include comments and very specific mentions of these Nodes in them, such as " |
468cbcd
to
96c4041
Compare
@Mickeon I checked quickly and found some missing things (an enum, etc.), so since this is a huge change that can introduce nasty bugs, I found it easier and safer make myself from scratch, comparing it with the original PR that introduced these changes. I just felt more confident doing it that way! |
That's ok! I just wanted to make sure our combined thoroughness made the PR more complete, including comments, too. |
If I am being honest, this makes it even worse, now Rigid is an alias for Dynamic, which is not technically correct, although based on current pragmatic usage I guess it is. :( |
@@ -1340,11 +1340,11 @@ | |||
<constant name="BODY_MODE_KINEMATIC" value="1" enum="BodyMode"> | |||
Constant for kinematic bodies. In this mode, a body can be only moved by user code and collides with other bodies along its path. | |||
</constant> | |||
<constant name="BODY_MODE_DYNAMIC" value="2" enum="BodyMode"> | |||
Constant for dynamic bodies. In this mode, a body can be pushed by other bodies and has forces applied. | |||
<constant name="BODY_MODE_RIGID" value="2" enum="BodyMode"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Would it be better to leave these as BODY_MODE_DYNAMIC*
? Would they apply to soft bodies or other body types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to understand how the bodies are organised internally to use the PhysicServer.
All bodies are RigidBodies with different modes. For example, a StaticBody is a RigidBody with infinite mass. As this can be difficult for a beginner to understand, there are several nodes.
The only exception is the SoftBody which inherits from MeshInstance.
In one of the two proposals the user has made a small tree that may help to understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, thanks for clarifying!
96c4041
to
f8cc88f
Compare
Thanks! |
In the future, please try to include the old names in the commit message when renaming classes, e.g. "Renamed RigidDynamicBody3D to RigidBody3D". Makes it much faster to fix projects after rebasing Godot :) |
@JohanAR You should not do anything, the old names have been added to the compatibility classes: https://github.com/godotengine/godot/pull/64894/files#diff-e4c9d70a3ecc449ede1e49b7ea01ed9128d2931c21e284d7a50d3f63cd7a13e2R1046-R1047 |
Is that something you have to enable in the editor? Because it definitely refused to run my test project before I had renamed all the RigidDynamicBody3D in the code |
Oh yes sorry I was not precise, in the code if you use it as a type, yes you have to modify it, it is in the editor it is automatic (so only fully automatic for those who do not type). |
Close: godotengine/godot-proposals#5183
Superseed #64548
Superseed #64546
I found some missing in the mentioned PR so I preferred to go back to zero to be sure as much as possible that everything is there, because this is the kind of PR that can break things.
It would be cool if this could be merged quickly because I've already had conflicts :D
@rburing If you can take a look.