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

Snap grab rotations to cube normals #50

Merged
merged 8 commits into from
Apr 6, 2024

Conversation

Deconimus
Copy link
Contributor

@Deconimus Deconimus commented Mar 28, 2024

As discussed in PR #31, specifically this implements "step 1" from this comment.

All grabbed objects are now rotated towards the player according to the closest cube normal facing the player. The calculations with the surface normals are done only once per grab, after this the relative rotation is preserved. Right now, the normal snapping is done in playerSetGrabbing(..) which is a bit bulky, but this will change with the planned future refactorings. Per frame the impact is only one additional quaternion multiplication in playerUpdateGrabbedObject(..).

Related refactorings:

  • Added playerPortalGrabRotate(..) in addition to playerPortalGrabTransform(..), since a point transformation is not always needed.
  • Similarly, added collisionSceneGetPortalRotation(..) to collision_scene which is a lot simpler to compute. Only utilized in playerPortalGrabRotate(..) so far.
  • playerGetMoveBasis(..) now takes a Quaternion instead of a Transform struct as first parameter, since only the rotation was used in the first place. Calls where we just want to calc the forward-facing dir vector relative to a given rotation are now simpler (e.g. in playerThrowObject(..)).

@Deconimus Deconimus marked this pull request as draft March 29, 2024 22:46
@Deconimus
Copy link
Contributor Author

I noticed object rotation will not be "cancelled out" as before, if the object-space upwards direction of an object was rotated in world-space before the grab. This would've been consistent with my former goal of source-like behavior but isn't helpful here.

I'll have to update playerSetGrabbing(..) accordingly.

@Deconimus Deconimus marked this pull request as ready for review March 30, 2024 23:19
@Deconimus
Copy link
Contributor Author

The cube normal snapping is now reworked to snap the closest normal towards the player while also considering the object's relative upward direction. This way, the grabRotation should now work just the like the previous implementation minus the superfluous rotations.

While searching for bugs and cornercases, I've found #52, which is unfortunately unrelated to the rotation here and instead a bug of the grabPoint transform introduced in #35.

@mwpenny mwpenny added the enhancement New feature or request label Apr 4, 2024
@mwpenny
Copy link
Owner

mwpenny commented Apr 4, 2024

I noticed object rotation will not be "cancelled out" as before, if the object-space upwards direction of an object was rotated in world-space before the grab. This would've been consistent with my former goal of source-like behavior but isn't helpful here.

Can you explain this issue a bit more? Through looking at your changes and trying it out before/after the follow-on fix I was unable to reproduce what I thought you were talking about. So I think I'm misunderstanding.

src/player/player.c Outdated Show resolved Hide resolved
src/physics/collision_scene.c Outdated Show resolved Hide resolved
@Deconimus
Copy link
Contributor Author

Deconimus commented Apr 4, 2024

Can you explain this issue a bit more? Through looking at your changes and trying it out before/after the follow-on fix I was unable to reproduce what I thought you were talking about. So I think I'm misunderstanding.

Before the fix, grabbing a cube which relative top/bottom sides weren't parallel to the world XZ planes resulted in buggy behavior. E.g. when throwing a cube in the air against a wall or pushing it against a wall while still grabbing it so that it rotates.

Here's a direct comparison:

grab_rotation_bug.mp4
grab_rotation_fixed.mp4

@Deconimus Deconimus requested a review from mwpenny April 4, 2024 10:39
@mwpenny
Copy link
Owner

mwpenny commented Apr 5, 2024

Thanks. Your comment about Source-like behavior makes more sense now too. Good catch!

src/player/player.c Outdated Show resolved Hide resolved
src/player/player.c Outdated Show resolved Hide resolved
@Deconimus Deconimus requested a review from mwpenny April 5, 2024 11:00
@mwpenny
Copy link
Owner

mwpenny commented Apr 6, 2024

Thank you for this. It is a noticeable improvement to how the game feels.

@mwpenny mwpenny merged commit d5fea9a into mwpenny:master Apr 6, 2024
@mwpenny mwpenny mentioned this pull request Apr 15, 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