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

Updated softbody handling to allow for area/softbody collision detection and application of area gravity #50785

Merged
merged 1 commit into from
Aug 16, 2021

Conversation

jeffrey-cochran
Copy link
Contributor

@jeffrey-cochran jeffrey-cochran commented Jul 23, 2021

  • Fixes Issue #36693
  • Added detection for soft-body-area collisions. The implementation is based on the current detection for rigid-body-area collisions.
  • Added area-specific gravity to softbody predict_motion so that soft-bodies are affected by changes in gravity that may result from intersection with an area.
  • Attached is a small scene that illustrates the use of this update: softbody-areas.zip

@jeffrey-cochran jeffrey-cochran requested a review from a team as a code owner July 23, 2021 18:25
@Calinou Calinou added this to the 4.0 milestone Jul 23, 2021
Copy link
Contributor

@pouleyKetchoupp pouleyKetchoupp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Gravity override works well.

Apart from my review (mostly code cleaning) I have a couple extra comments:

  • Please add a description for the PR - even if short - so there's a record in the future. You can indicate that it's based on the body implementation, link to fixed issue or addressed proposal when relevant (I think there was an issue about area gravity in this case). If you have a test project you can easily share (unless you use a MRP from a bug report) it can be a plus to help with reviewing, but it's not mandatory.

  • This PR doesn't add support for Area scene nodes to detect soft bodies and notify scripts for custom code by users (like it's done for detected bodies and areas) so monitored_soft_bodies in Area3DSW is currently not used.

This will be needed eventually, but it can be added in a separate PR as it requires a few more steps to be functional:

-New method in PhysicsServer3D for soft body monitor callback:

virtual void area_set_monitor_callback(RID p_area, Object *p_receiver, const StringName &p_method) = 0;
virtual void area_set_area_monitor_callback(RID p_area, Object *p_receiver, const StringName &p_method) = 0;

(and implementation in PhysicsServer3DSW)

-Handle soft bodies in Area3DSW::call_queries():

void Area3DSW::call_queries() {

-Register monitoring callback in Area3D:

godot/scene/3d/area_3d.cpp

Lines 280 to 287 in fef27e9

if (monitoring) {
PhysicsServer3D::get_singleton()->area_set_monitor_callback(get_rid(), this, SceneStringNames::get_singleton()->_body_inout);
PhysicsServer3D::get_singleton()->area_set_area_monitor_callback(get_rid(), this, SceneStringNames::get_singleton()->_area_inout);
} else {
PhysicsServer3D::get_singleton()->area_set_monitor_callback(get_rid(), nullptr, StringName());
PhysicsServer3D::get_singleton()->area_set_area_monitor_callback(get_rid(), nullptr, StringName());
_clear_monitoring();
}

-Implement monitoring callback in Area3D, new method similar to bodies:

void Area3D::_body_inout(int p_status, const RID &p_body, ObjectID p_instance, int p_body_shape, int p_area_shape) {

(it will need new specific signals for soft bodies)

  • New methods get_overlapping_soft_bodies and overlaps_soft_body exposed to script:

    godot/scene/3d/area_3d.cpp

    Lines 560 to 564 in fef27e9

    ClassDB::bind_method(D_METHOD("get_overlapping_bodies"), &Area3D::get_overlapping_bodies);
    ClassDB::bind_method(D_METHOD("get_overlapping_areas"), &Area3D::get_overlapping_areas);
    ClassDB::bind_method(D_METHOD("overlaps_body", "body"), &Area3D::overlaps_body);
    ClassDB::bind_method(D_METHOD("overlaps_area", "area"), &Area3D::overlaps_area);

-Updated documentation for new methods and signals in Area3D: https://github.com/godotengine/godot/blob/master/doc/classes/Area3D.xml

@@ -928,16 +928,63 @@ void SoftBody3DSW::apply_forces() {
}
}

void SoftBody3DSW::_compute_area_gravity_and_dampenings(const Area3DSW *p_area) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather remove area_linear_dam and area_angular_damp completely and rename the method _compute_area_gravity since damping values are not used for soft bodies.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should be renamed to _compute_area_gravity or something more generic. Apart from that it all looks fine!

@pouleyKetchoupp
Copy link
Contributor

When the last comment about the method name is addressed, please squash your commits (see interactive rebase) and it should be good to go!

@akien-mga
Copy link
Member

For the reference, the email used to author your commits (see https://patch-diff.githubusercontent.com/raw/godotengine/godot/pull/50785.patch) doesn't seem to be linked to your GitHub account, which is why they appear with a default GitHub avatar instead of yours.

It's not a problem for Git but if you want to claim ownership so that they are attributed to your account, you can add this email as secondary email in your GH settings.

@jeffrey-cochran
Copy link
Contributor Author

diff.githubusercontent.com/raw/godotengine/godot/pull/50785.patch) doesn't seem to be

Thanks, I actually just noticed this the other day when I was searching for my commits lol. I'll do that

@jeffrey-cochran jeffrey-cochran force-pushed the softbody-areas branch 2 times, most recently from fd08df1 to 70b99bd Compare August 16, 2021 02:47
@akien-mga
Copy link
Member

Thanks! And congrats for a great GSoC 2021 🎉

@akien-mga akien-mga merged commit 7013199 into godotengine:master Aug 16, 2021
@skaiware
Copy link

Congrats @jeffrey-cochran
I guess it could hardly be backported to the 3x branch ?
Kind

@pouleyKetchoupp
Copy link
Contributor

@skaiware It should be possible to backport it in a future 3.x version, although it needs more work to make a Bullet version as well (this is Godot Physics only for now), and there are still some things to add to the nodes API to make it fully functional.

@jeffrey-cochran
Copy link
Contributor Author

Congrats @jeffrey-cochran
I guess it could hardly be backported to the 3x branch ?
Kind

I will probably backport it to 3.x in the near future. Just don't have time in the next week or so to sort out finer details.

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.

SoftBody is not affected by space override
5 participants