-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
link component: allows setting previewDistance #4425
Conversation
The tests are failing in CI because "apt-get install failed". They all pass on my machine. |
Thanks. I think if anything this could be automated instead of adding additional properties to the component |
Does the current PR meet your standards? |
src/components/link.js
Outdated
@@ -199,7 +205,7 @@ module.exports.Component = registerComponent('link', { | |||
cameraWorldPosition.setFromMatrixPosition(camera.matrixWorld); | |||
distance = elWorldPosition.distanceTo(cameraWorldPosition); | |||
|
|||
if (distance > 20) { | |||
if (distance > this.previewDistance + 5) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why + 5
and not making previewDistance 25
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left the functionality unchanged, when the scale is unchanged.
The original code in master shows the preview when the camera is closer than 15m, and turns the link to face the camera when the camera is farther than 20m (you could call that the "link-lookAt distance'). In between, the center of the link just shows red, but the link isn't turned to face the camera.
I vary the preview distance, rather than the link-lookAt distance, because neither should be zero or less. Also, the preview distance is used in several places in the code, and link-lookAt distance only one.
Any change in the relationship between these parameters or change to the behavior of links with scale 1, should be done in a different PR, I think.
Thanks for the patience. I made a few more comments. |
Thank you! |
Description:
For the link component, allows setting the distance within which the preview image is shown. The default is good for standard size portals used when virtually walking, but the larger portals required by higher speeds (or cyclopean architecture) need to let the preview image be seen at a greater distance. See https://elfland-glider.surge.sh/yggdrasil/
Changes proposed: