Keep rotation of grabbed objects faithful to Portal on PC

[Deconimus]

See Issue #27

Grabbed objects now remain at a constant distance from the player on the XZ-plane (ground plane). Effectively, the object now moves along a straight upward line in worldspace when the player looks up or down. The target height (Y component) of the object is calculated the same way as before, which I found also has a nice smoothing effect on the up-/downward object movement due to the implied projection.

Additionally, grabbed objects are now no longer rotated towards the player. The original rotation of the object relative to the player's looking direction (on the XZ plane) is maintained while grabbing, the same way as in original Portal.

Lastly, I fixed a minor bug along the way, which caused objects to be let go of when loading a savegame. This occurred when the object was held through a portal, such that player->grabbingThroughPortal != 0.

grabbing_objects.mp4

[Deconimus]

The original rotation of the object relative to the player's looking direction (on the XZ plane) is maintained while grabbing, the same way as in original Portal.

I noticed this is not always the case in Portal. There's special handling for cubes and radios, maybe some other objects too. Cubes are rotated, if the looking-direction is close to surface-normal of one of the cubes 6 faces (simplified). Radios are always rotated such that they face the player like it was implemented with all objects in Portal64.

This could be implemented using the decorId hack from triggers and fizzlers. I'll look into it. I'm thinking maybe we should just add a function called decorIdForCollisionObject(struct CollisionObject* collider) with a comment of this being a hack, so that we can keep track and replace it with a proper implementation more easily.

[Deconimus]

There's special handling for cubes and radios, maybe some other objects too. Cubes are rotated, if the looking-direction is close to surface-normal of one of the cubes 6 faces (simplified). Radios are always rotated such that they face the player like it was implemented with all objects in Portal64.

Implemented this and fixed some corner-case bugs related to grabbing through portals.

[Deconimus]

portal_objects_rotate.mp4

portal64_objects_rotate.mp4

[mwpenny]

There are 3 main changes here:

1. Keep grabbed objects in front of player (original issue)
2. New behavior around grabbed object rotation
3. Fix for dropping held objects through portals while loading game

Item 3 is quite small, but 2 is fairly independent and also more complex than the others. Please try to keep distinct changes to separate issues/PRs.

I'm not entirely sold on the rotation change though. It's what the original game does, but Portal 64 diverges when it makes sense - either out of necessity (i.e., simplifying due to technical limitations), or for usability and N64 conventions. Here, playing with an N64 controller already makes it more difficult to manipulate objects. Rotating them toward the player makes it easier to position them in a level.

This feels like one of those cases where it's reasonable that Portal 64 doesn't match the original. It also keeps the code simple and avoids edge cases like the ones you had to fix. When trying this out I also saw that grabbed cubes are dropped when passing through portals. I'm not sure if this is due to the rotation or other changes. I'll have time to take a closer look at these changes in a few days.

I'm not completely opposed, but it doesn't seem worth it due to usability and complexity (as an aside, separate issues are helpful for discussing things like this so the rest of the PR changes can be merged in the meantime).

What are your thoughts?

[mwpenny]

Consolidating this is a good call

[Deconimus]

2 is fairly independent and also more complex than the others.

Yes, this PR would best have been split in two orginally. It grew a bit out of proportions, since I initially thought that atleast the "Y-Axis rotation" needed to change, that happens when moving an object up and down. With the way the objects moves in a straight line in front of the player, this otherwise felt unnatural. Especially for cubes, placing them at higher or lower positions would have them needlessly tilted.

Here, playing with an N64 controller already makes it more difficult to manipulate objects. Rotating them toward the player makes it easier to position them in a level.

I understand the sentiment, but I don't necessarily agree. A few thoughts:

To me personally the way it is implemented in this branch feels more polished, I found the object rotating felt a bit "wobbly" before. Especially since cubes were needlessly rotated 180° or 90° before. The grabbing behavior in source games and Portal specifically has a distinct feel to it. Maybe it's just a matter of taste, but I think matching that helps with making this port feel more "complete".

Gameplay-wise, with how it works in this branch, cubes still get rotated towards you when it makes sense and the cutoff value could even still be changed to snap the rotation more often. Other objects that can be grabbed aren't gameplay relevant (yet), but with the current logic these can easily be made to rotate towards the player. Maybe playtesting and seeing if some passages actually do become harder could help here.

One more thing to consider would be turrets. When these will be implemented, it will be crucial to preserve their rotation relative to the player, like it happens here.

It also keeps the code simple and avoids edge cases like the ones you had to fix.

The edge cases I had to fix were mostly due to me being tired and not thinking with portals :) I think it will be a problem keeping this fairly robust in the long rung. Especially since the game's codebase already provides a great set of utility functions regarding transformations and quaternions.

Speaking of technical complexity and limitations, the only part where computations did become significantly more involved is the playerSetGrab function, which is only called once per grab. I wouldn't worry about this too much here, unless profiling on real N64 hardware reveals otherwise.

When trying this out I also saw that grabbed cubes are dropped when passing through portals. I'm not sure if this is due to the rotation or other changes.

You're correct, this is most likely due to the rotation. This should be an easy fix though.

[Deconimus]

I made a new PR #35 that just contains the changes that are necessary for #27.

I do still think that keeping the rotation faithful to Portal is the way to go. But aside from that, even if deciding otherwise, the code here could easily be used to just change the grab behavior for turrets in the future (see shouldRotateTowardsPlayer() in player.c).

You're correct, this is most likely due to the rotation. This should be an easy fix though.

And it was. Funnily enough, I had to fix the same issue when making #35. It was just due to the forward direction not being updated correctly.

[mwpenny]

I see what you mean about the feel, and you're right that turrets shouldn't auto-rotate toward the player.

Overall, I agree that polish and faithfulness are important. But parity with the original game shouldn't be the only factor taken into account when making such changes. We also need to consider what works well for the way Portal 64 will run and be played. Even if a change would make the game more like the original, sometimes it makes sense not to match exactly or it isn't worth the technical complexity to do so.

I wouldn't worry about this too much here, unless profiling on real N64 hardware reveals otherwise.

Sorry if I was unclear - my complexity concern is related to architecture, not performance. There's still a lot of work to go on this project, and not keeping it clean will make progress difficult over time, particularly when it comes to changes affecting core gameplay. In this case, the tight coupling of held object type to player code in exchange for this polish is a negative tradeoff that has me uneasy.

Rather than having object-specific logic intertwined in the player code, I'd prefer cleaner boundaries via separate functions/data that are called/looked up by playerSetGrabbing(). Ideally, rotation type/snap normals/threshold/etc. could be looked up on a per-object basis so this remains organized and extensible (e.g., if other auto-rotating objects are added). playerSetGrabbing() doesn't need to care about what kind of object is being grabbed.

I do still think that keeping the rotation faithful to Portal is the way to go.

I understand your points and think you're right that it could be tweaked to find the sweet spot with time. I wouldn't be opposed if this were integrated into the game more cleanly as mentioned above. Ultimately though, it seems very personal and subjective.

I anticipate there will be some players who prefer Source engine style rotation, and others who prefer easier object manipulation - just like the two of us. I also asked some Portal fans I know and they thought auto-rotating was the default behavior in the original game. So I think a good middle ground would be an option to enable/disable auto-rotation (can be a separate change). That way we're not forcing players out of their preferences.

[Deconimus]

In this case, the tight coupling of held object type to player code in exchange for this polish is a negative tradeoff that has me uneasy.

Rather than having object-specific logic intertwined in the player code, I'd prefer cleaner boundaries via separate functions/data that are called/looked up by playerSetGrabbing(). Ideally, rotation type/snap normals/threshold/etc. could be looked up on a per-object basis so this remains organized and extensible (e.g., if other auto-rotating objects are added). playerSetGrabbing() doesn't need to care about what kind of object is being grabbed.

In the long run I definitely agree with you that a more object-oriented approach will be beneficial here. But I also wasn't envisioning the current state of the code as "final" for the project by any means. The changes you propose require rather little change in player.c but more effort elsewhere. This could be made possible if we had access to the underlying decor_object (if any) of the collisionBox in playerSetGrabbing() (see my comment on #36). Then we could simply call a decorObjectRotationType(...) function etc. I don't think "per-object" lookup is necessary in a literal sense, rather a "per-type" lookup that is done in the right place.

I anticipate there will be some players who prefer Source engine style rotation, and others who prefer easier object manipulation - just like the two of us. I also asked some Portal fans I know and they thought auto-rotating was the default behavior in the original game.

Very true. The snapping of cube rotation probably feels close enough to auto rotating that you don't usually notice it unless paying close attention. Ultimately, this could be used as an argument for both the original rotation behavior doing a good job and also that it perhaps doesn't matter that much in the end, since auto-rotation feels close enough. The superfluous 180° and 90° rotations for cubes are what threw me off personally.

So I think a good middle ground would be an option to enable/disable auto-rotation (can be a separate change). That way we're not forcing players out of their preferences.

This sounds like a very good idea.

[mwpenny]

But I also wasn't envisioning the current state of the code as "final" for the project by any means.
[...]
I don't think "per-object" lookup is necessary in a literal sense, rather a "per-type" lookup that is done in the right place.

Yes, not necessarily literal, but able to be determined dynamically based on what is being grabbed without hard-coding in the player code. Initial implementations don't need to be perfect by any means, but fuzzy boundaries should be avoided. For now, for example, a function similar to decorObjectRotationType() or decorObjectGetRotation() can accept the decor ID as a parameter, then be improved as needed. I don't think that will be a lot of effort.

The superfluous 180° and 90° rotations for cubes are what threw me off personally.

Yes, this definitely contributes to that "wobbliness". Even without the broader rotation changes, this could be improved by snapping to the closest face (using the unit cube for everything).

[Deconimus]

For now, for example, a function similar to decorObjectRotationType() or decorObjectGetRotation() can accept the decor ID as a parameter, then be improved as needed.

Good point, that would make sense. Perhaps returning a variable with rotation flags rather than a simple type could be best, e.g. ROTATE_TOWARDS_PLAYER, ROTATE_USING_LOOK_TRANSFORM, ROTATE_SNAP_TO_CUBE_NORMALS.

Even without the broader rotation changes, this could be improved by snapping to the closest face (using the unit cube for everything).

I agree. Ideally I'd still utilize the rotation preserving code here w.r.t. player.grabRotationBase for that, to avoid calculating the nearest normal every frame.

Going forward, what are your thoughts on type-based behavior for the "non-source" grabbing to preserve the way non-gameplay relevant decor objects like radios were rotated before? I'm thinking about implementing the changes in 3 steps that can be merged separately:

1. Cube normal snapping for all objects
2. RotationType for decorObjects as discussed above (only different flags for radios at that point)
3. Source-like behavior via menu option using the introduced RotationType (low priority polish for the future)

Let me know what you think. Step 2 and 3 can also be done together, if you'd rather keep the rotation exactly the same for all objects per default. But with 1 & 2 implemented, it feels like the primary points towards Portal parity would be achieved without affecting the current behavior too much. The implementation details about how snapping works and which objects don't turn towards the player in source-like behavior would almost be an afterthought at that point.

[mwpenny]

Perhaps returning a variable with rotation flags rather than a simple type could be best

Sure. Then the process could be: lookup rotation type -> compute rotation based on type -> apply rotation. Each rotation type computation could be utility function, keeping it easy to manage and add more.

[...] avoid calculating the nearest normal every frame

Yes, agreed. Per-frame 3D calculations can add up otherwise. I've found a few instances just while working on chamber 14.

Going forward, what are your thoughts [...]

This sounds good overall, and no need to do 2 and 3 at once. The cube normal snapping is an improvement over the extra rotations.

Something to keep in mind is that non-decor objects are grabbable too (eventually turrets, but even now with security cameras). The first implementation doesn't need to be perfect and can be revised later, but it's good to be aware of. For now, for example, it would be good enough to just fall back to cube snapping for non-decor objects in change 2. This is related to your final point:

The implementation details about how snapping works and which objects don't turn towards the player in source-like behavior would almost be an afterthought at that point.

Agreed, it will be quite flexible which is great.

[Deconimus]

Something to keep in mind is that non-decor objects are grabbable too (eventually turrets, but even now with security cameras). The first implementation doesn't need to be perfect and can be revised later, but it's good to be aware of. For now, for example, it would be good enough to just fall back to cube snapping for non-decor objects in change 2. This is related to your final point:

With this in mind, perhaps it's a good idea not to have the rotation-type related code in the decor_object.* sources. Perhaps adding grab_rotation.* sources with functions like grabRotationTypeForDecorId(...) etc. would be a better option.

[mwpenny]

Yeah that's a good way of doing it. Rotation type getters and even the rotation functions themselves could be organized together instead of spread across the sources for the various object types. Then everything related to rotation would be in one place.

What I meant by what you quoted is just that not all rotated objects will be able to be treated the same. We'll need the separation at some level (e.g., switch/case on object type). This is less important now, but good to keep in mind.

[Jgoodwin64]

Interesting read.