Skip to content

Added position-warp option - #29

Open
thedart76 wants to merge 2 commits into
jure:mainfrom
thedart76:position-warp
Open

Added position-warp option#29
thedart76 wants to merge 2 commits into
jure:mainfrom
thedart76:position-warp

Conversation

@thedart76

Copy link
Copy Markdown

Hi Jure,

As anticipated over X (Twitter), this PR implements a simple edit to your component to add the position warp option.

Reference on the Oculus Techniques and Best Practices:
https://developer.oculus.com/resources/locomotion-design-turns-teleportation/#position-warp

I added the positionWarpDuration property to the component's schema so the users can tweak it as they want or need. I initially set its default value to 0.5 sec, but after using this locomotion method in immersive mode for a few minutes, I changed it to 1 sec as it provides more comfort.

NOTE: rather than a fixed speed, the fixed animation duration is an intended design choice: the resulting variable movement speed (proportional to the distance of the selected destinations) is indeed what makes this locomotion method feel natural, producing the result of "walking" to closer destinations and "running" to the ones farther away from the initial position of the user. This is, at least, what I wanted and needed to achieve in my prototype for an in-house project 🙂

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a position-warp teleportation method to the blink-controls component, providing an alternative locomotion option where the player smoothly animates to the teleport destination rather than instantly appearing there. This follows the Oculus best practices for VR locomotion.

Changes:

  • Added teleportationMethod and positionWarpDuration schema properties to configure position-warp behavior
  • Modified teleportation logic to animate position changes when position-warp is enabled
  • Delayed rotation application until after position animation completes for position-warp
  • Updated README documentation to describe the new properties

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
src/index.js Added schema properties and conditional logic to animate position and defer rotation when position-warp method is enabled
README.md Documented the two new configuration properties for position-warp teleportation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/index.js
Comment on lines +430 to +435
rig.setAttribute('animation', {
property: 'position',
to: `${newRigLocalPosition.x} 0 ${newRigLocalPosition.z}`,
easing: 'linear',
dur: this.data.positionWarpDuration
});

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

Setting an animation attribute without removing or managing previous animations could cause conflicts if multiple teleportations are triggered rapidly. A-Frame's animation component will handle multiple animations on the same property by queuing or replacing them, but it's safer to explicitly remove the animation attribute after it completes or before starting a new one to avoid unexpected behavior. Consider adding an 'animationcomplete' event listener to clean up the animation attribute, or ensure that setting a new animation properly replaces the old one.

Suggested change
rig.setAttribute('animation', {
property: 'position',
to: `${newRigLocalPosition.x} 0 ${newRigLocalPosition.z}`,
easing: 'linear',
dur: this.data.positionWarpDuration
});
// Remove any existing animation to avoid conflicts when teleporting rapidly.
rig.removeAttribute('animation')
rig.setAttribute('animation', {
property: 'position',
to: `${newRigLocalPosition.x} 0 ${newRigLocalPosition.z}`,
easing: 'linear',
dur: this.data.positionWarpDuration
})

Copilot uses AI. Check for mistakes.
Comment thread src/index.js
snapTurn: { default: true },
rotateOnTeleport: { default: true }
rotateOnTeleport: { default: true },
teleportationMethod: { default: ""},

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The schema property uses an empty string as the default value for teleportationMethod, but this is effectively checking for string equality with "position-warp". Consider using a more explicit default or using a oneOf constraint to validate acceptable values. Based on the codebase pattern seen in other schema properties like 'button' (line 44), this property should include a oneOf constraint to restrict it to valid values.

Suggested change
teleportationMethod: { default: ""},
teleportationMethod: { default: 'position-warp', oneOf: ['position-warp'] },

Copilot uses AI. Check for mistakes.
Comment thread src/index.js
snapTurn: { default: true },
rotateOnTeleport: { default: true }
rotateOnTeleport: { default: true },
teleportationMethod: { default: ""},

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The teleportationMethod property is missing an explicit type declaration. Looking at the schema conventions in this codebase, string properties should include type: 'string' for clarity and consistency (e.g., see lines 54-56 which use type: 'selector'). While A-Frame may infer the type from the default value, being explicit improves code clarity and maintainability.

Copilot uses AI. Check for mistakes.
Comment thread README.md
| snapTurn | If left/right thumbstick axis performs a 45 deg rotation | true |
| rotateOnTeleport | Will rotate player on teleport, facing the direction where the arrow is pointing | true |
| teleportationMethod | Set value to `position-warp` to enable this teleportation option. | |
| positionWarpDuration | How long the position warp animation should be. | 1000 |

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The documentation states "How long the position warp animation should be" with a default of 1000, but doesn't specify the unit (milliseconds). For consistency with the incrementalDrawMs property on line 46 which explicitly mentions "ms" in both the property name and description ("How long the ray animation should be | 500"), consider either: (1) adding "in milliseconds" to the description, or (2) renaming the property to positionWarpDurationMs to match the naming convention.

Copilot uses AI. Check for mistakes.
Comment thread src/index.js
if (this.data.teleportationMethod === "position-warp") {
rig.setAttribute('animation', {
property: 'position',
to: `${newRigLocalPosition.x} 0 ${newRigLocalPosition.z}`,

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The Y-coordinate is hardcoded to 0 in the animation target, but this ignores the Y-component from newRigLocalPosition.y. This will cause the rig to always animate to Y=0 regardless of the actual target position's Y-coordinate, which could result in the player ending up at the wrong height. The animation should use the full position vector including the Y-component.

Suggested change
to: `${newRigLocalPosition.x} 0 ${newRigLocalPosition.z}`,
to: `${newRigLocalPosition.x} ${newRigLocalPosition.y} ${newRigLocalPosition.z}`,

Copilot uses AI. Check for mistakes.
Comment thread src/index.js
Comment on lines +448 to +449
setTimeout(() => {
this.cameraRig.object3D.setRotationFromQuaternion(this.teleportOriginQuaternion)

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The setTimeout callback is not stored or cleaned up, which could cause issues if the component is removed or another teleportation happens before the timeout completes. If a user rapidly triggers multiple teleportations, multiple setTimeout callbacks will queue up, potentially causing the rotation to be set incorrectly. Consider storing the timeout ID in an instance variable and clearing it before setting a new one, or at the very least clearing it in the remove() lifecycle method.

Suggested change
setTimeout(() => {
this.cameraRig.object3D.setRotationFromQuaternion(this.teleportOriginQuaternion)
if (this.rotationTimeoutId) {
clearTimeout(this.rotationTimeoutId)
this.rotationTimeoutId = null
}
this.rotationTimeoutId = setTimeout(() => {
this.cameraRig.object3D.setRotationFromQuaternion(this.teleportOriginQuaternion)
this.rotationTimeoutId = null

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants