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

fix queries from direct space state(support for kinematic bodies). Also update to godot-cpp 4.1 #13

Closed

Conversation

Ughuuu
Copy link
Contributor

@Ughuuu Ughuuu commented Jul 6, 2023

@rburing
Copy link
Owner

rburing commented Jul 6, 2023

I think a point should not have a radius, no matter how small. See https://gamedev.stackexchange.com/a/1516/2351 which suggests to use b2Fixture::TestPoint in a loop over fixtures, after the AABB query.

@Ughuuu Ughuuu changed the title fix queries from direct space state fix queries from direct space state - WIP Jul 6, 2023
@Ughuuu
Copy link
Contributor Author

Ughuuu commented Jul 6, 2023

Hm, I did something similar, but why would I not just use fixture->GetShape() ? Why get all the shapes of the body, if I know which shape collided.
Testing with the physics test I now also pass the 1 pixel away test.(Eg testing a point 1 pixel further than a shape and it doesnt return collision).

I think each shape has an AABB so it collides with each shape that might intersect, so that callback will be called at some point for all fixtures anyway, so there is no need to do foreach in there. It might be a performance issue if I do.
Will test a bit more after, rn doing implementation for the other functions also.

@Ughuuu Ughuuu force-pushed the fix-helper-functions-to-make-queries-work branch from 413ba63 to eeeec02 Compare July 6, 2023 16:31
@Ughuuu
Copy link
Contributor Author

Ughuuu commented Jul 6, 2023

In this PR I also updated to godot 4.1

@Ughuuu Ughuuu changed the title fix queries from direct space state - WIP fix queries from direct space state. Also update to godot-cpp 4.1 - WIP Jul 6, 2023
@Ughuuu Ughuuu force-pushed the fix-helper-functions-to-make-queries-work branch 5 times, most recently from c9a6a65 to 3a306b4 Compare July 7, 2023 20:37
update to godot 4.1

update for 4.1

update submodules?

lint

update readme

update readme

add canvas instance id
@Ughuuu Ughuuu force-pushed the fix-helper-functions-to-make-queries-work branch from 3a306b4 to 2d1ca91 Compare July 7, 2023 20:37
@Ughuuu
Copy link
Contributor Author

Ughuuu commented Jul 7, 2023

Added a basic implementation for these:

_intersect_shape
_cast_motion
_collide_shape
_rest_info

and for the one on physics_server:

_shape_collide

For the non motion vector ones:
_intersect_shape
_collide_shape
_collide_shape
_rest_info

I did a aabb check and then checked distance between all of them(not sure if there is a better solution)

For the ones that I think require motion check:
_cast_motion
_shape_collide

I did a aabb test(for ones that dont have 2 shapes only as param) and then a sweep test.

@Ughuuu Ughuuu force-pushed the fix-helper-functions-to-make-queries-work branch 2 times, most recently from c351251 to 6845f6f Compare July 7, 2023 21:43
@Ughuuu Ughuuu changed the title fix queries from direct space state. Also update to godot-cpp 4.1 - WIP fix queries from direct space state. Also update to godot-cpp 4.1 Jul 7, 2023
@Ughuuu Ughuuu marked this pull request as ready for review July 7, 2023 21:49
lint

lint

implement sweep test

add missing get_contact_local_velocity_at_position function for 4.1

write basic implementation for shape test

small error check

write correct implementation for _shape_collide

fix queries

lint

fix point intersect precision

fix constant central force

lint

remove unused

lint

write closest values

update readme
@Ughuuu Ughuuu force-pushed the fix-helper-functions-to-make-queries-work branch from cc8fdb8 to 5d3f602 Compare July 7, 2023 21:50
@Ughuuu Ughuuu changed the title fix queries from direct space state. Also update to godot-cpp 4.1 fix queries from direct space state(support for kinematic bodies). Also update to godot-cpp 4.1 Jul 7, 2023
@Ughuuu
Copy link
Contributor Author

Ughuuu commented Jul 7, 2023

For point query, it's a simple aabb query and then test point containment, as you described.
For shape query, its a aabb query that returns some shapes(based on the aabb of the shapeA received as input) and then test distance between all resulting shapes and shapeA.
For motion query, it uses aabb query on begin/end location(to get all the shapes that might intersect with it), and then does a sweep query between shapeA and each shape.

Also updated the godot and box2d submodules, not sure if i did that correctly, let me know if it looks ok.

@Ughuuu
Copy link
Contributor Author

Ughuuu commented Jul 10, 2023

Updated the intersects_shape and other functions from direct space similar to it. Still something wrong with it when rotation objects. Also, if the shape starts already colliding with an object, it should not report collision(for test_move for eg.). That is also not yet done. Will open an issue for that also.

Started work on physics_server side for _body_test_motion, _body_collide_shape and _shape_collide.

Currently passing 107/150.

Once those are also done it will be ready for review.

update those

exclude bodies in query

fix intersect point to pass all tests

update shape api

fix ray tests

fix raycast tests
@Ughuuu Ughuuu force-pushed the fix-helper-functions-to-make-queries-work branch from 2067c45 to 7ba5168 Compare July 10, 2023 20:19
@Ughuuu
Copy link
Contributor Author

Ughuuu commented Jul 15, 2023

Updated the B_TO_G_FACTOR to be 100(since in godot gravity is 980, and in box2d its 9.8, means the factor has to be 100).

Also fix how density works and how inertia is computed for shapes(now instead of density of 1, density is put to be 1/computed_mass_of_shape). This way, the computed inertia is without the computed mass(box2d computes the object estimated mass, but we want to always hardcode it to a value, as is in godot). It also matches what godot does.

Currently passing 110/150 tests (a big part of these are CharacterBody2DTests, which need the correct impl for safe/unsafe fraction), as described here: #18

@Ughuuu Ughuuu force-pushed the fix-helper-functions-to-make-queries-work branch 3 times, most recently from 8348bd6 to f7dde05 Compare July 20, 2023 13:35
@Ughuuu Ughuuu force-pushed the fix-helper-functions-to-make-queries-work branch 2 times, most recently from 84e1d67 to 8c01353 Compare July 20, 2023 15:46
update intersect shape(almost fixed, 2 more tests to fix)

fix direct space functions

fix max contacts count

upd

fix type conversion to match current godot space

fix density and space transform

refactor

fix pointer crash

add manifold code

update collision normal

fix intersect_shape tests

add shape collide impl

add impl for safe/unsafe fractions

apply b2Common hack

fix macos issue

update path of lib to addons/physics_server_box2d/

fix typo

rename artifactts

use wildcard pattern for addons folder

fix normal being zero
@Ughuuu Ughuuu force-pushed the fix-helper-functions-to-make-queries-work branch from 655850f to b5e1b8e Compare July 21, 2023 17:31
@@ -15,14 +15,19 @@ class Box2DCollisionObject;

class Box2DShape {
RID self;
Box2DCollisionObject *body;
Copy link
Owner

Choose a reason for hiding this comment

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

I do not understand this. The Box2DShape is supposed to be a resource (hence the RID) that can be shared by multiple bodies. For example it should be possible to create 10 bodies all referring to one and the same circle shape resource. Can you test an example like that? The rest of the PR looks very good, but I don't think this part can work like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, that would be a problem indeed. I guess it would have a reference to only one of the bodies.
Right now this is used to keep a track of the body, and it's used in case of eg. _intersect_shape to return collider_id and collider.
However, I see now that there is a method on the b2Fixture called GetBody, so I wouldn't need this reference, as I could use that.
Good catch.

Copy link
Owner

Choose a reason for hiding this comment

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

@Ughuuu indeed, that sounds like the correct approach.

@@ -31,7 +36,9 @@ class Box2DShape {
virtual Variant get_data() const = 0;

virtual int get_b2Shape_count(bool is_static) const = 0;
virtual b2Shape *get_transformed_b2Shape(int p_index, const Transform2D &p_transform, bool one_way, bool is_static) = 0;
Copy link
Owner

Choose a reason for hiding this comment

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

The transform also isn't part of the resource (each body can transform the shape differently), which explains why I had the method like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect, wasn't sure of that. Updating that.

@Ughuuu Ughuuu closed this Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants