Fix Bullet Physics Server #47508

kalysti · 2021-03-30T20:53:45Z

Bullet2 physics compatibility for to new rendering server api in godot 4. Only small changes were necessary.

Next step: PhysX Integration (in progress)

aaronfranke · 2021-03-30T21:26:04Z

Next step: PhysX Integration (in progress)

IIRC the plan was to have support for pluggable physics engines via GDNative, so integrating more physics engines into core likely is not desired.

kalysti · 2021-03-30T21:36:19Z

Next step: PhysX Integration (in progress)

IIRC the plan was to have support for pluggable physics engines via GDNative, so integrating more physics engines into core likely is not desired.

Does not matter. Then I deliver it as a gd native module. I am currently at around 75% and I will be finished by the end of the week and will start the first tests. But I think that in the long term bullet should be replaced anyway.

aaronfranke · 2021-03-30T21:41:16Z

@sboronczyk It does matter, because even if a feature is fully 100% implemented and working, it may not actually be desired to be merged into the engine. In general, if a big feature hasn't been approved by a core maintainer, a PR for it will be ignored.

kalysti · 2021-03-30T21:47:29Z

@sboronczyk It does matter, because even if a feature is fully 100% implemented and working, it may not actually be desired to be merged into the engine. In general, if a big feature hasn't been approved by a core maintainer, a PR for it will be ignored.

Okay. Not important to me though. I need PhysiX for my purposes and of course I like to share it with the community (whether it will be delivered as a core module or as a GDNative module is another question). If a core mantainer thinks it makes sense, that would of course be so much the better. But back to the topic :-) Thats PR is only for the bullet physics. We can talk about it further via discord. greetings sj

BastiaanOlij · 2021-03-31T02:47:41Z

@sboronczyk It does matter, because even if a feature is fully 100% implemented and working, it may not actually be desired to be merged into the engine. In general, if a big feature hasn't been approved by a core maintainer, a PR for it will be ignored.

But it can still be tested and evaluated and used by people and thus gain useful insights for a better implementation down the road.
Just because it is never merged because the long term plans require a different implementation isn't good enough reason to tell people not to even bother.

kalysti · 2021-03-31T11:13:55Z

Thank you guys for your different opinions. But we should keep the PR free for important questions about the request and not talk about other PRs or projects. I think that's just confusing :-)

pouleyKetchoupp

Thanks for fixing this! The plan is to move Bullet to a separate plugin, but in the meantime it's good to have it working again, and these changes would be needed anyway.

I haven't tested yet. For info, you can use this test suite to check for regression and test with multi-threaded configuration:
https://github.com/godotengine/godot-demo-projects/tree/master/3d/physics_tests

doc/classes/ProjectSettings.xml

modules/bullet/area_bullet.cpp

modules/bullet/bullet_physics_server.cpp

modules/bullet/constraint_bullet.h

modules/bullet/register_types.cpp

modules/bullet/space_bullet.cpp

modules/bullet/space_bullet.h

kalysti · 2021-04-14T14:47:41Z

Okai tomorrow i will start to check and round the pr up.

modules/bullet/area_bullet.h

doc/classes/ProjectSettings.xml

modules/bullet/bullet_physics_server.cpp

modules/bullet/bullet_physics_server.h

modules/bullet/register_types.cpp

modules/bullet/space_bullet.h

modules/bullet/space_bullet.cpp

akien-mga · 2021-04-22T09:36:24Z

You mention "Bullet3", but I don't see any changes to the modules/bullet/SCsub so I think it's still using the Bullet2 API, no?
(See Bullet3Dynamics vs BulletDynamics in thirdparty/bullet, and other related Bullet3 vs Bullet folders - we only compile the Bullet ones, albeit they do include a couple shared headers from the AFAIK never finished Bullet3 API.)

Once this is ready, please remember to squash the commits into one, as the history of whitespace fixes in PRs is not relevant to the engine's Git history.

kalysti · 2021-04-22T23:32:00Z

You mention "Bullet3", but I don't see any changes to the modules/bullet/SCsub so I think it's still using the Bullet2 API, no?
(See Bullet3Dynamics vs BulletDynamics in thirdparty/bullet, and other related Bullet3 vs Bullet folders - we only compile the Bullet ones, albeit they do include a couple shared headers from the AFAIK never finished Bullet3 API.)

Once this is ready, please remember to squash the commits into one, as the history of whitespace fixes in PRs is not relevant to the engine's Git history.

sorry my mistake. of course it's bullet 2 (I assumed that godot is already using bullet3). but i can extend it to bullet 3 (but guess that will cause more problems than fix). I therefore recommend changing the whole thing to bullet 3 when bullet is delivered as a native plugin. your decision!

pouleyKetchoupp · 2021-04-22T23:35:37Z

@sboron There's no plan to update Bullet. I've updated the PR title to reflect more accurately what it does.

kalysti · 2021-04-22T23:35:59Z

@sboron There's no plan to update Bullet. I've updated the PR title to reflect more accurately what it does.

Okai sorry for the misscommunication.

kalysti · 2021-04-22T23:54:21Z

I compressed all commits into one and adjusted the commit name. Changes are included (except for the unresolved ones).

servers/physics_3d/physics_server_3d_sw.cpp

modules/bullet/space_bullet.h

fix whitespaces :-) Remove whitespaces Fix whitespaces fix whitespaces space_bullet.h cleanup cleanup Add bullet2 to godot4 fix wrong line

modules/bullet/space_bullet.h

akien-mga · 2021-04-23T10:19:34Z

Nitpick but I'd suggest using "Fix and re-enable Bullet physics for 4.0" instead of "Add bullet 2 physics to godot 4". The latter sounds like you're adding a full blown implementation of bullet2 for Godot, which we've already had since Godot 3.0, but it was disabled a couple of months ago during API changes.

pouleyKetchoupp

I've stopped reviewing when I've found again some conversations marked as resolved without the comments being addressed.

I'm not going to spend extra time reviewing the same things over and over again, so you need to stop doing that. Make sure you address all the comments that were made before, including conversations you've closed, and answer to them if you don't agree, and then you can request another review.

Edit: The general idea behind lots of my comments is, this PR is meant to make the Bullet server work again, but it doesn't need to entirely match headers from Godot Physics when not needed. Each server implementation has some specific variations, and it's not needed to copy all methods and members over just for the sake of making things identical.

pouleyKetchoupp · 2021-05-03T18:38:34Z

modules/bullet/area_bullet.cpp

 	for (int i = overlappingObjects.size() - 1; 0 <= i; --i) {
-		overlappingObjects[i].object->on_exit_area(this);
+		overlappingObjects.write[i].object->on_exit_area(this);


.write doesn't seem to be needed here and is going to cause unnecessary copies.

pouleyKetchoupp · 2021-05-03T18:40:05Z

modules/bullet/area_bullet.h

@@ -139,6 +140,9 @@ class AreaBullet : public RigidCollisionObjectBullet {
 	_FORCE_INLINE_ void set_spOv_priority(int p_priority) { spOv_priority = p_priority; }
 	_FORCE_INLINE_ int get_spOv_priority() { return spOv_priority; }

+	_FORCE_INLINE_ void set_priority(int p_priority) { priority = p_priority; }
+	_FORCE_INLINE_ int get_priority() const { return priority; }


These methods and the priority member are not needed, they are not used anywhere.

pouleyKetchoupp · 2021-05-03T18:41:47Z

modules/bullet/bullet_physics_server.cpp

+BulletPhysicsServer3D::BulletPhysicsServer3D(bool p_using_threads) {
+	using_threads = p_using_threads;
+	active = true;
+	doing_sync = false;


active and doing_sync are already initialized in the header, they don't need to be set here.

pouleyKetchoupp · 2021-05-03T18:49:37Z

modules/bullet/bullet_physics_server.h

-	mutable RID_PtrOwner<RigidBodyBullet> rigid_body_owner;
-	mutable RID_PtrOwner<SoftBodyBullet> soft_body_owner;
-	mutable RID_PtrOwner<JointBullet> joint_owner;
+	Set<SpaceBullet *> active_spaces;


This was marked as resolved, but wasn't addressed.

Blackiris · 2021-06-15T20:19:47Z

Sorry to bump on it, is there any way to help unlock this PR?

I have tried this branch and except some conflict because of some API changes (Transform -> Transform3D, PhysicsServer3D::MODE_DYNAMIC_LOCKED -> PhysicsServer3D::BODY_MODE_DYNAMIC_LOCKED...)

Just got a crash on the editor side that is easy to fix, but otherwise, the main features are mostly back (need to check the constraints maybe).

kalysti requested review from a team as code owners March 30, 2021 20:53

Calinou added enhancement topic:physics labels Mar 30, 2021

Calinou added this to the 4.0 milestone Mar 30, 2021

Calinou mentioned this pull request Mar 30, 2021

Add bullet 3 physics to godot 4 #47507

Closed

pouleyKetchoupp suggested changes Apr 6, 2021

View reviewed changes

kalysti requested review from a team as code owners April 22, 2021 03:53

aaronfranke requested a review from a team April 22, 2021 04:14

kalysti mentioned this pull request Apr 22, 2021

PhysicsServer3D errors when starting new project #46167

Closed

akien-mga reviewed Apr 22, 2021

View reviewed changes

pouleyKetchoupp changed the title ~~Add bullet 3 physics to godot 4~~ Fix Bullet Physics Server Apr 22, 2021

pouleyKetchoupp reviewed Apr 22, 2021

View reviewed changes

servers/physics_3d/physics_server_3d_sw.cpp Outdated Show resolved Hide resolved

pouleyKetchoupp reviewed Apr 22, 2021

View reviewed changes

modules/bullet/space_bullet.h Outdated Show resolved Hide resolved

kalysti and others added 3 commits April 23, 2021 02:00

Add bullet 2 physics to godot 4

3533372

fix whitespaces :-) Remove whitespaces Fix whitespaces fix whitespaces space_bullet.h cleanup cleanup Add bullet2 to godot4 fix wrong line

remove set_static_global_body

3618a6a

Add the right ref to documentation

4023dc5

pouleyKetchoupp reviewed Apr 23, 2021

View reviewed changes

modules/bullet/space_bullet.h Outdated Show resolved Hide resolved

kalysti added 2 commits April 23, 2021 02:16

Remove lock, unlock and is_locked

52c55cb

remove _3D_DISABLED, and singletonbullet

3e1bc78

akien-mga mentioned this pull request Apr 29, 2021

bullet: Sync with upstream 3.17 #48299

Merged

kalysti requested a review from pouleyKetchoupp May 1, 2021 13:19

pouleyKetchoupp suggested changes May 3, 2021

View reviewed changes

Calinou mentioned this pull request Jul 5, 2021

GodotPhysics does not have an intermediate axis theorem #50173

Closed

akien-mga added the needs work label Jul 6, 2021

Calinou mentioned this pull request Jul 30, 2021

[Bullet] Jittering against two discrete, connected walls #34596

Closed

Calinou added the topic:3d label Mar 4, 2022

akien-mga mentioned this pull request Mar 9, 2022

Remove unused Bullet module and thirdparty code #58946

Merged

akien-mga closed this in #58946 Mar 10, 2022

akien-mga added the archived label Mar 10, 2022

Calinou removed the needs work label Mar 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Bullet Physics Server #47508

Fix Bullet Physics Server #47508

kalysti commented Mar 30, 2021 •

edited

Loading

aaronfranke commented Mar 30, 2021

kalysti commented Mar 30, 2021

aaronfranke commented Mar 30, 2021

kalysti commented Mar 30, 2021 •

edited

Loading

BastiaanOlij commented Mar 31, 2021

kalysti commented Mar 31, 2021

pouleyKetchoupp left a comment

kalysti commented Apr 14, 2021

akien-mga commented Apr 22, 2021

kalysti commented Apr 22, 2021

pouleyKetchoupp commented Apr 22, 2021

kalysti commented Apr 22, 2021

kalysti commented Apr 22, 2021

akien-mga commented Apr 23, 2021

pouleyKetchoupp left a comment •

edited

Loading

pouleyKetchoupp May 3, 2021

pouleyKetchoupp May 3, 2021

pouleyKetchoupp May 3, 2021

pouleyKetchoupp May 3, 2021

Blackiris commented Jun 15, 2021

Fix Bullet Physics Server #47508

Fix Bullet Physics Server #47508

Conversation

kalysti commented Mar 30, 2021 • edited Loading

aaronfranke commented Mar 30, 2021

kalysti commented Mar 30, 2021

aaronfranke commented Mar 30, 2021

kalysti commented Mar 30, 2021 • edited Loading

BastiaanOlij commented Mar 31, 2021

kalysti commented Mar 31, 2021

pouleyKetchoupp left a comment

Choose a reason for hiding this comment

kalysti commented Apr 14, 2021

akien-mga commented Apr 22, 2021

kalysti commented Apr 22, 2021

pouleyKetchoupp commented Apr 22, 2021

kalysti commented Apr 22, 2021

kalysti commented Apr 22, 2021

akien-mga commented Apr 23, 2021

pouleyKetchoupp left a comment • edited Loading

Choose a reason for hiding this comment

pouleyKetchoupp May 3, 2021

Choose a reason for hiding this comment

pouleyKetchoupp May 3, 2021

Choose a reason for hiding this comment

pouleyKetchoupp May 3, 2021

Choose a reason for hiding this comment

pouleyKetchoupp May 3, 2021

Choose a reason for hiding this comment

Blackiris commented Jun 15, 2021

kalysti commented Mar 30, 2021 •

edited

Loading

kalysti commented Mar 30, 2021 •

edited

Loading

pouleyKetchoupp left a comment •

edited

Loading