Skip to content

Fix grab with scaled pickables by swapping inverse for affine_inverse#707

Merged
BastiaanOlij merged 2 commits intoGodotVR:masterfrom
davehakim:GXR-619-FixGrabOfScaledPickables
May 19, 2025
Merged

Fix grab with scaled pickables by swapping inverse for affine_inverse#707
BastiaanOlij merged 2 commits intoGodotVR:masterfrom
davehakim:GXR-619-FixGrabOfScaledPickables

Conversation

@davehakim
Copy link
Copy Markdown
Contributor

@davehakim davehakim commented Feb 8, 2025

Jolt physics appears to support the scaling of physics bodies in response to the scaling of an ancestor node. This has great potential for tabletop XR games where one might want the pieces to be 'pickable' (a subclass of RigidBody3D) while at the same time allowing dynamic scaling of the game board (and all the pieces on it!)

The grab related classes however don't allow grabbing scaled XRToolsPickables by default (well, at very least the grab position is off) because they use inverse transforms and not affine_inverse which respects node scaling. This PR changes all the relevant inverse transforms to affine_inverse which should fix that.

To test:

  1. Using godot 4.3 add the Godot Jolt framework
  2. Choose jolt as the 3d physics engine for the demo project
  3. Run the project and enter the pickables scene

Without the changes in this pr the cubes would not be pickable (they disappear or shrink in response to attempted pickups)
With the change they pick up fine (usually 🤷) at the correct grab position

Issues:

  • This pr mostly works but not 100% of the time 😂 . For reasons I have yet to discover (which is part of why I'm opening this draft PR! To get assistance from developers who know this code better than I do!) sometimes what I think should occur as a normal pickup seems to happen as a ranged pickup.
  • This pr does not (yet) work in the case of ranged pickups.

Note 1 : The changes to pickable_demo are only here to illustrate the issue and need to be cleaned up or out before this pr gets considered for merge.
Note 2 : I expect this addresses the cause of #613

To work around the 'unexpected scaling' glitch in my game I've just disabled ranged grabs (by always treating ranged grabs in the code the same as non ranged i.e. always doing this in the pickup function

var grab := Grab.new(grabber, self, by_grab_point, true)

but I figure before this pr becomes something more general purpose though the ranged grab issue(s) needs to be solved

@davehakim davehakim changed the title GXR-619: Fix grab with scaled pickables by swapping inverse for affine_inverse Fix grab with scaled pickables by swapping inverse for affine_inverse Feb 8, 2025
@davehakim davehakim marked this pull request as draft February 8, 2025 23:00
Copy link
Copy Markdown
Member

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

This is good, we should merge this. If there is a chance to revert the changes on pickable_demo.tscn as they are not part of the fix, that would be great.

@BastiaanOlij BastiaanOlij added the enhancement New feature or request label Mar 21, 2025
@BastiaanOlij BastiaanOlij added this to the 4.5.0 milestone Mar 21, 2025
@davehakim
Copy link
Copy Markdown
Contributor Author

This is good, we should merge this. If there is a chance to revert the changes on pickable_demo.tscn as they are not part of the fix, that would be great.

Hi @BastiaanOlij , sorry for the delay in circling back to this. I've reverted the demo changes so the PR could merge as is and it will fix scaling issues when directly grabbing an XRToolsPickable . Two things tho

  1. LERP ranged grabs still need fixes that I haven't looked into yet with this pr (because my game doesn't use them ... and I don't have a great understanding of LERPing 😁 )
  2. Snap ranged grabs appear to need this change which isn't yet in my PR (since I don't understand the full impact of it)
var grab := Grab.new(grabber, self, by_grab_point, true)

2a. I see some non-ranged grabs executing as ranged grabs (it looks like picked_up_ranged on XRToolsFunctionPickup can be true even in some cases when can_ranged_grab on the XRToolsPickable is not) so even in some cases of non-ranged grabs we can get some scaling issues unless I make the change listed in 2

So what do you think @BastiaanOlij

a. Get this pr in as is ?
b. Update ranged grab snap case to set precise Grab argument to true and then look to merge ?
c. Or don't attempt to merge until we solve ranged LERP grabs (which I'd need help to do 😁 )

@davehakim davehakim marked this pull request as ready for review April 22, 2025 19:44
@BastiaanOlij BastiaanOlij merged commit 6f47ef8 into GodotVR:master May 19, 2025
@BastiaanOlij
Copy link
Copy Markdown
Member

Thanks @davehakim , I'm happy to merge this as is and see further improvements in coming PRs.

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