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

Add finger collisions and pickup collisions to collision hands #690

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

BastiaanOlij
Copy link
Member

@BastiaanOlij BastiaanOlij commented Oct 24, 2024

Still very early work but these are enhancements on the collision hands and pickup logic to automatically add collision shapes for finger digits and held objects.

The held objects logic works decently though it completely panics on the sniper riffle in the pickables demo, haven't figured out why yet, everything else seems to work.
One thing that we had in @DigitalN8m4r3 original implementation of this is that letting go of objects will make the object collide with the hand collision usually resulting in objects flying off.
Also there is no support for weighted objects yet, I may look into that as part of this.

The collisions on fingers are somewhat simplistic as there is no logic where we test collisions along the movement of fingers, but as they are part of the collision hands, they do effect things alright. I think it's good enough for our animated hands, but it wouldn't suffice when used with full hand tracking. Still its better than nothing.

Contributed by Khronos Group through the Godot Integration Project

@BastiaanOlij BastiaanOlij added the enhancement New feature or request label Oct 24, 2024
@DigitalN8m4r3
Copy link
Contributor

DigitalN8m4r3 commented Oct 24, 2024

storing hand collision layers on the pickable upon pickup, upon drop erasing/clearing it and using a tween to get retrieve it back. Thats the theory i got for fly off issue, now am not sure if this will really work out since the collision starts right when you drop.

The sniper rifle might have something turnen on collisionwise that breaks it, will look into it since its the most complicated pickable

@BastiaanOlij
Note i still did not tested it yet but i
Just read "# If we're two handing an object, both collision hands will get copies."

Now this sounds to me like a major Problem, sadly i dont have a proper generic6dof joint solution/example but i do feel that instead of actualy copying the pickable collision shape, it would make way more sense to have the actual one working when grabbed but for that we would need to use some sort of pinjoint amd connect the collision hand and pickable. At the same time we might then be able to use the proper weight of the object to give the weight feel.

@DigitalN8m4r3
Copy link
Contributor

DigitalN8m4r3 commented Oct 24, 2024

alright now i know whats causing the issues with the sniper rifle, the firearm slide that i introduced, its collisionshape3d.
no idea yet how to fix this, so for the time being it might just be best
to set the "sniper_rifle/FirearmSlide/HandleOrigin/InteractableHandle/CollisionShape3D" Node to disabled.
maybe someone else got a better idea for such things

Ooh and regarding the collision hands/finger collision/copied collision for pickables...
damn i gotta say this feels way better then i expected, especialy considering that you can use complex collisionshapes, nice, gives actual hope that this could become the real thing!

@BastiaanOlij
Copy link
Member Author

Now this sounds to me like a major Problem, sadly i dont have a proper generic6dof joint solution/example but i do feel that instead of actualy copying the pickable collision shape, it would make way more sense to have the actual one working when grabbed but for that we would need to use some sort of pinjoint amd connect the collision hand and pickable. At the same time we might then be able to use the proper weight of the object to give the weight feel.

I'm pretty sure others are further along with this than I am (looking at dedm0zajs work for instance) but so far the time I've had experimenting with joints have just frustrated the heck out of me, they do not play nice when it's a characterbody3d that is driving everything. I do believe that this is the right way forward for the future but it will require a bunch more experimenting and it requires a structural change to how we setup XR tools.

So at the moment I feel that for our current XR Tools implementation, we should stick with the approach we currently have, but before we get to far with XR Tools 2, this is something that is in my view to look more closely at.

@BastiaanOlij
Copy link
Member Author

alright now i know whats causing the issues with the sniper rifle, the firearm slide that i introduced, its collisionshape3d.
no idea yet how to fix this, so for the time being it might just be best
to set the "sniper_rifle/FirearmSlide/HandleOrigin/InteractableHandle/CollisionShape3D" Node to disabled.
maybe someone else got a better idea for such things

Thats a good find. Godot physics actually has an option to record objects you don't want to test collisions with so I think I'm going to enhance the logic to walk the tree of the item you pick up, and add collision exceptions for all physics body children.

@DigitalN8m4r3
Copy link
Contributor

alright now i know whats causing the issues with the sniper rifle, the firearm slide that i introduced, its collisionshape3d.
no idea yet how to fix this, so for the time being it might just be best
to set the "sniper_rifle/FirearmSlide/HandleOrigin/InteractableHandle/CollisionShape3D" Node to disabled.
maybe someone else got a better idea for such things

Thats a good find. Godot physics actually has an option to record objects you don't want to test collisions with so I think I'm going to enhance the logic to walk the tree of the item you pick up, and add collision exceptions for all physics body children.

aah exactly, this should solve it... am on the way to test this with normal interactables, think it might be rooted more into the essence of picking something, grabbing and if that is the case, we might need to lock it to a class check.. but yeah, just talking without evidence, yet

@DigitalN8m4r3
Copy link
Contributor

alright now i know whats causing the issues with the sniper rifle, the firearm slide that i introduced, its collisionshape3d.
no idea yet how to fix this, so for the time being it might just be best
to set the "sniper_rifle/FirearmSlide/HandleOrigin/InteractableHandle/CollisionShape3D" Node to disabled.
maybe someone else got a better idea for such things

Thats a good find. Godot physics actually has an option to record objects you don't want to test collisions with so I think I'm going to enhance the logic to walk the tree of the item you pick up, and add collision exceptions for all physics body children.

aah exactly, this should solve it... am on the way to test this with normal interactables, think it might be rooted more into the essence of picking something, grabbing and if that is the case, we might need to lock it to a class check.. but yeah, just talking without evidence, yet

@BastiaanOlij
apparently am right
see attached video

CollisionIssueInteractables.mp4

@BastiaanOlij
Copy link
Member Author

Ok, I've got the collision exception code in. I was surprised to find we already had some of the logic existing on our grab helper object so I improved that code.

We now create exceptions to the object we pick up and it's children.
We also remove the exceptions but with half a second of delay.

That last bit did have me scratching my head as there doesn't seem to be any code freeing the grab object (memory leak?) but it seemed to get freed before my code ran. Maybe there is some default behavior in Godot that if you do not nominate a base class, it's subclasses from RefCounted? Though with RefCounted callables (so when you use a timer) should be retained.

Anyway, I changed the logic to use a static method and pass all needed info to it. That seems to work.
This logic can be further improved in the near future where we delay removing the exception until there is no more collision, but for now this should solve everything but one or two edge cases. Defo a lot better than it was before.

@BastiaanOlij BastiaanOlij marked this pull request as ready for review October 28, 2024 02:53
@BastiaanOlij BastiaanOlij force-pushed the enhance_collision_hands branch 2 times, most recently from d3d4084 to 1754794 Compare October 28, 2024 02:58
@BastiaanOlij BastiaanOlij added this to the 4.4.0 milestone Oct 28, 2024
_digit_collision_shapes[bone_name] = collision_node

if collision_node:
# TODO it would require a far more complex approach,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is something for a future PR. Also this logic would likely be more suited to be a SkeletonModifier3D implementation so that the skeleton is also limited in movement, something even more powerful when combined with full hand tracking.

That might be an XRT2 thing though.

if not is_instance_valid(on_collision_hand):
return

# This can be improved by checking if we're still colliding and only
Copy link
Member Author

Choose a reason for hiding this comment

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

This is also for a follow up PR. It would require manually checking collisions on the physics server. Would be a cool improvement but for later.

@@ -28,6 +28,10 @@ signal xr_ended
signal xr_failed_to_initialize


## XR active flag
Copy link
Member Author

Choose a reason for hiding this comment

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

Just to make the CI happy...

@DigitalN8m4r3
Copy link
Contributor

Ok, I've got the collision exception code in. I was surprised to find we already had some of the logic existing on our grab helper object so I improved that code.

We now create exceptions to the object we pick up and it's children. We also remove the exceptions but with half a second of delay.

That last bit did have me scratching my head as there doesn't seem to be any code freeing the grab object (memory leak?) but it seemed to get freed before my code ran. Maybe there is some default behavior in Godot that if you do not nominate a base class, it's subclasses from RefCounted? Though with RefCounted callables (so when you use a timer) should be retained.

Anyway, I changed the logic to use a static method and pass all needed info to it. That seems to work. This logic can be further improved in the near future where we delay removing the exception until there is no more collision, but for now this should solve everything but one or two edge cases. Defo a lot better than it was before.

Looking good, gives this almost the feel we would want.

Btw since you mentioned that you dont know yet what we could/should do with the fingers.

My suggestion would be to let the fingers bend out depending upon surface collision, this would give the controller hands another level of depth. 10° degrees for bending the fingers out, the out part might probably be easier taking the palm direction upon colliding with a surface.

@BastiaanOlij BastiaanOlij merged commit f9701c5 into GodotVR:master Nov 13, 2024
2 checks passed
@BastiaanOlij BastiaanOlij deleted the enhance_collision_hands branch November 13, 2024 10:32
squidt pushed a commit to squidt/godot-xr-tools that referenced this pull request Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants