godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.99k stars 21.16k forks source link

PhysicalBone2D does not work #78367

Open renleidedadi opened 1 year ago

renleidedadi commented 1 year ago

Godot version

v4.0.3.stable.official [5222a99f5]

System information

Windows 10 - Godot v4.0.3.stable - Vulkan (Forward+) - NVIDIA GeForce GTX 1650 (nvidia, 31.0.15.1694) - Intel Core i5-10400F CPU @ 2.90GHz

Issue description

i make a 2D ragdoll with 4.0.3, PhysicalBone2D doesn't work, but RigidBody2D work, how to use PhysicalBone2D?

in this video, left is PhysicalBone2D, right is RigidBody2D:

https://github.com/godotengine/godot/assets/67264008/1fd8fe7b-95f7-4300-854b-4c07bf2045ea

Steps to reproduce

  1. download the project.
  2. run main_scene.
  3. left is PhysicalBone2D, right is RigidBody2D.

Minimal reproduction project

PhysicalBone2D in 4.0.3.zip

stilestr commented 1 year ago

I don't think you set up the skeleton correctly. However, I tried my best to follow the documentation and it was too confusing to get this to work properly, so I couldn't do it either.

After googling some, it didn't seem like anyone has ever used PhysicalBone2D to make a ragdoll, and the only posts I found were unanswered questions asking how to get it to work.

So if anyone knows how to actually get this to work and can set up a working 2D skeleton using PhysicalBone2D, I'd be happy to help write some documentation or a tutorial on how to get it to work.

renleidedadi commented 1 year ago

I don't think you set up the skeleton correctly. However, I tried my best to follow the documentation and it was too confusing to get this to work properly, so I couldn't do it either.

After googling some, it didn't seem like anyone has ever used PhysicalBone2D to make a ragdoll, and the only posts I found were unanswered questions asking how to get it to work.

So if anyone knows how to actually get this to work and can set up a working 2D skeleton using PhysicalBone2D, I'd be happy to help write some documentation or a tutorial on how to get it to work.

yes, PhysicalBone2D is difficult to use without tutorial, i think this issue should also be add "documentation" label. another situation is PhysicalBone2D might has something bugs: #78479 (PhysicalBone2D is broken?), but i ignorant of C++, i can't fix it

stilestr commented 1 year ago

I don't think you set up the skeleton correctly. However, I tried my best to follow the documentation and it was too confusing to get this to work properly, so I couldn't do it either. After googling some, it didn't seem like anyone has ever used PhysicalBone2D to make a ragdoll, and the only posts I found were unanswered questions asking how to get it to work. So if anyone knows how to actually get this to work and can set up a working 2D skeleton using PhysicalBone2D, I'd be happy to help write some documentation or a tutorial on how to get it to work.

yes, PhysicalBone2D is difficult to use without tutorial, i think this issue should also be add "documentation" label. another situation is PhysicalBone2D might has something bugs: #78479 (PhysicalBone2D is broken?), but i ignorant of C++, i can't fix it

Yeah, I just made that bug report yesterday while trying to research this one, lol. It seems that PhysicalBone2D might just be a highly neglected (and highly broken?) feature. Would be great to see if anyone has ever gotten it to work properly, cause it's hard to find any examples online

Xorblax commented 1 year ago

I concur on PhysicalBone2D not working great. I would honestly hold off on using them at all, as they're causing my editor to crash on startup now. I think what I did was move my normal bones that are referenced by physical bones, to another skeleton, or something. I don't exactly remember, I was trying everything just to get the ragdoll to work. This is the error log image

StrikeForceZero commented 1 year ago

They seem to have their physics integration disabled in their constructor for some reason (ever since they were first added). https://github.com/godotengine/godot/blob/851bc640ddf7225a4b52bb15cc50c249df119953/scene/2d/physical_bone_2d.cpp#L290C1-L293

PhysicalBone2D::PhysicalBone2D() {
    // Stop the RigidBody from executing its force integration.
    PhysicsServer2D::get_singleton()->body_set_collision_layer(get_rid(), 0);
    PhysicsServer2D::get_singleton()->body_set_collision_mask(get_rid(), 0);
    PhysicsServer2D::get_singleton()->body_set_mode(get_rid(), PhysicsServer2D::BodyMode::BODY_MODE_STATIC);
        // ...

Removing those calls to the physics server allows the bones to act like rigid bodies. Very curious to know the reasoning behind this.

StrikeForceZero commented 1 year ago

I suspect the skeleton modification stack is supposed to do something about it, but until I find it after digging through the engines source or someone comments, you can bypass the physics integrator being disabled without patching and compiling the engine from the source with the following:

func call_child_recursive(node: Node2D, f: Callable):
    f.call(node)
    for child in node.get_children():
        call_child_recursive(child, f)

func update_bone(bone: Node2D):
    if bone is PhysicalBone2D:
            # in case you forgot to check it
        bone.simulate_physics = true
        # this will undo the cpp constructor
        bone.freeze = true
        bone.freeze = false

func _ready():
    for child in $Skeleton2D.get_children():
        if child is PhysicalBone2D:
            call_child_recursive(child, update_bone)

if you want to know why does this work (here is the cpp code we execute to undo the constructor call)

void RigidBody2D::_apply_body_mode() {
    if (freeze) {
        switch (freeze_mode) {
            case FREEZE_MODE_STATIC: {
                set_body_mode(PhysicsServer2D::BODY_MODE_STATIC);
            } break;
            case FREEZE_MODE_KINEMATIC: {
                set_body_mode(PhysicsServer2D::BODY_MODE_KINEMATIC);
            } break;
        }
    } else if (lock_rotation) {
        set_body_mode(PhysicsServer2D::BODY_MODE_RIGID_LINEAR);
    } else {
        set_body_mode(PhysicsServer2D::BODY_MODE_RIGID); // <--- we want to execute this code path so we must first set freeze to true and then false to get here
    }
}
Dioinecail commented 1 year ago

Hello, i tried the workaround above but the PhysicalBone2D still doesn't seem to work My setup is:

PhysicalBody2D
  Skeleton2D
    Bone2D
    PhysicalBone2D

Skeleton2D contains SkeletonModification2DPhysicalBones with exactly 1 bone set in Joint 0 NodePath

PhysicalBone2D targets Bone2D but the editor still spits an error about incorrect modification stack

StrikeForceZero commented 1 year ago

@Dioinecail So there might have been some details left out of my original comment. I just attempted to do a clean room working example using @renleidedadi 's minimal reproduction. I eventually remembered that the physical bones needs to have a joint + collision shape to really do anything. It's unfortunately really tedious, but if you want a working example you can see the modified code here

Make sure to read the comments in the main_scene.gd

godot-PhysicalBone2D-Workaround-example.zip

Edit: my editor also spits out that error about the modificationstack

scene/resources/skeleton_modification_2d_physicalbones.cpp:157 - Cannot update PhysicalBone2D cache: modification is not properly setup!

Really hoping there is some detail we are missing and could make this much easier :)

Dioinecail commented 1 year ago

@StrikeForceZero i got to the joint part while looking through the C++ code but i couldn't figure out that you need to have a collision shape but i guess it's only natural since it inherits RIgidBody2D

gonna try the "full" setup tomorrow :)

Dioinecail commented 1 year ago

yup, it works now

you were right, there is probably a bug with initialization or constructor

making bones do

for bone in yourTargetBones:
  bone.freeze = true
  bone.freeze = false

was enough to make it work

The most confusing part is setting everything up correctly you need to have RigidBody2D probably as parent to everything Skeleton2D with ModificationStack that contains SkeletonModification2DPhysicalBones and Joint X Nodepath's target your bones (if you change your bone name or path, it breaks) PhysicalBone2D needs to have a PinJoint2D that has RigidBody2D or PhysicalBone2D connected to another PhysicalBone2D in the inspector PhysicalBone2D needs to have a RigidBody2D or Skeleton2D or another PhysicalBone2D as parent PhysicalBone2D needs to have a CollisionShape2D as child PhysicalBone2D needs to have a Bone 2d Nodepath selected and correct Bone 2d Index selected as well! PhysicalBone2D needs to have Simulate Physics checked on

and my hierarchy looks like this

RigidBody2D
  Node2D (just a root for skeleton)
    Skeleton2D
      BodyBone
        TargetBone2D
      PhysicalBone2D
        PinJoint2D
        CollisionShape2D

Overall yeah I would advise to avoid using PhysicalBone2Ds until they are properly documented and the freezing bug is fixed :D

lilykiwi commented 1 year ago

While I'd like to see this feature fixed and documented, it seems as though #65768 has an explanation for why it's not been touched yet:

The ModificationStack made a lot of APIs implemented in the Skeleton that should not be there, since it was implemented without going through a formal review by core maintainers due to it was GSOC project. That must be removed and reimplement outside of Skeleton. Also, the current ModificationStack is significantly broken and needs to be refactored and reimplemented as a Node. Unfortunately, this refactoring is taking a long time, sorry.

Additionally that PR included a fix such that "PhysicalBone2D no longer relies on SkeletonModification2DPhysicalBones to update bone positions", although this wasn't present in #65801, the PR that was actually merged.

Unfortunately this implies that the feature is currently in a broken state, with an "experimental feature" warning on the documentation being the only indicator of this. Sadly it looks like we're just going to need to wait until this feature is reworked and in a stable state.

YuriSizov commented 1 month ago

Removing those calls to the physics server allows the bones to act like rigid bodies. Very curious to know the reasoning behind this.

Just FYI, the problem here is things getting out of sync. We have a chain of inheritance as follows:

And we have the 2D physics server. In Godot when a node is based on a server, there is often a two-part approach to properties. They exist both on the node class and in the server counterpart. The body mode member is one such property.

Normally, to keep things in sync you'd use CollisionObject2D::set_body_mode. But for some reason the PhysicalBone2D node modifies the server-side value directly via PhysicsServer2D::get_singleton()->body_set_mode. This leads to a desync where the member inherited from CollisionObject2D believes that the node is in the rigid mode (the default for any RigidBody2D instance), but the server thinks the node has the static mode (because that's what the PhysicalBone2D constructor tells it).

Toggling the freeze property implicitly forces CollisionObject2D::set_body_mode to be called with different values to re-sync the node with the server, but the fix can be effectively boiled down to this:

extends Node2D

@onready var _skeleton = %Skeleton

func _ready()->void:
    var modification_stack: SkeletonModificationStack2D = _skeleton.get_modification_stack()
    var modification_physical_bones: SkeletonModification2DPhysicalBones = modification_stack.get_modification(0)
    modification_physical_bones.fetch_physical_bones()

    for i in modification_physical_bones.physical_bone_chain_length:
        var p_bone_path := modification_physical_bones.get_physical_bone_node(i)
        var p_bone: PhysicalBone2D = _skeleton.get_node(p_bone_path)
        PhysicsServer2D.body_set_mode(p_bone.get_rid(), PhysicsServer2D.BODY_MODE_RIGID)

Whether or not the current implementation is considered experimental, the fix to this particular problem is simple and can be done right now. PhysicalBone2D should be passing these values via the inherited API and not access the server directly.

The usability of the whole thing is another matter, of course...