feat(Pointer): pointer implementation #86

bddckr · 2018-05-15T22:35:48Z

The pointer concept is the ability to designate an area of the scene
with a destination. A pointer consists of multiple independent
functionality which is implemented through a component that casts
and returns points that define the pointer beam, one that
generically renders points via game objects and a third component
handles the selection functionality of a pointer.

A pointer knows when it is activated, deactivated, when it starts
hovering (enters) a new object, when it finishes hovering (exits) an
object and knows what coordinates of the object it is hovering over.
These are all cast as events with a special pointer payload
containing information about the pointer target.

The Straight Line Cast implementation casts a straight line from an
origin point (e.g. controller) and stops when it collides with an
object (or reaches the set maximum length). The same applies to the
added Parabolic Line Cast, but it casts a parabolic line instead.

Additionally the introduction of more components that raycast means
the existing PhysicsCast class is moved to the new namespace
Cast.

bddckr · 2018-05-15T22:37:26Z

Fix remaining TODOs found in the code.
Docs, docs, docs.
~~Maybe figure out a way to not have to specify the PointsCast on multiple components (Pointer, PointsRenderer and another one...)~~ This is actually intended.
Remove BasePointer and others coming from the old pointer branch.
Ensure commit message lists the addition of the ParabolicLineCast for A pointer that emits a parabolic curve with a cursor that reacts to other scene objects #8.
~~Split into multiple commits perhaps.~~
Target the master branch after everything above is done.

Down the line:

Tests from the previous PR.
New tests for the new components and the parabolic line cast.

See #90.

thestonefox · 2018-05-16T05:59:30Z

Scripts/Cast/ParabolicLineCast.cs

+        /// prevent the cast being projected high into the sky and curving back down.
+        /// </summary>
+        [Tooltip("The maximum angle in degrees of the origin before the cast curve height is restricted. A lower angle setting will prevent the cast being projected high into the sky and curving back down.")]
+        [Range(1, 100)]


[Tooltip()] should be the last attribute before the parameter

/// <summary> /// The maximum angle in degrees of the origin before the cast curve height is restricted. A lower angle setting will /// prevent the cast being projected high into the sky and curving back down. /// </summary> [Range(1, 100)] [Tooltip("The maximum angle in degrees of the origin before the cast curve height is restricted. A lower angle setting will prevent the cast being projected high into the sky and curving back down.")] public float heightLimitAngle = 100f;

thestonefox · 2018-05-16T05:59:57Z

Scripts/Cast/ParabolicLineCast.cs

+            AdjustForEarlyCollisions(jointPosition, downPosition);
+        }
+
+        protected virtual Vector3 ProjectForwardBeam()


Protected methods also need XML docs

thestonefox · 2018-05-16T06:00:54Z

Scripts/Cast/PointsCast.cs

+        /// <summary>
+        /// The result of the most recent cast. <see langword="null"/> when the cast didn't hit anything.
+        /// </summary>
+        public RaycastHit? TargetHit;


field should be targetHit

Also why have RaycastHit nullable type? can't you just check RaycastHit.transform == null?

Nullable because I don't want the intrinsic knowledge of how Unity uses RaycastHit to be part of this - they could change that, too (but probably won't). This way a user just has to test for existence, rather than know about how RaycastHit handles its properties to signal "nothing was hit".

thestonefox · 2018-05-16T06:01:05Z

Scripts/Cast/PointsCast.cs

+        /// <summary>
+        /// The points along the the most recent cast.
+        /// </summary>
+        public IReadOnlyList<Vector3> Points;


thestonefox · 2018-05-16T06:01:45Z

Scripts/Cast/PointsCast.cs

+        /// An optional <see cref="ExclusionRule"/> to determine targets based on the set rules.
+        /// </summary>
+        [Tooltip("An optional ExclusionRule to determine targets based on the set rules.")]
+        public ExclusionRule exclusionRule;


this is called targetValidity elsewhere

thestonefox · 2018-05-16T06:10:24Z

Scripts/Pointer/PointerSelection.cs

+{
+    using UnityEngine;
+
+    public class PointerSelection : MonoBehaviour


thestonefox · 2018-05-16T06:12:08Z

Scripts/Tracking/SurfaceLocator.cs

@@ -3,6 +3,7 @@
    using UnityEngine;
    using UnityEngine.Events;
    using System;
+    using VRTK.Core.Cast;


why does this need Cast?

As specified in the commit message (and the PR description):

Additionally the introduction of more components that raycast means
the existing PhysicsCast class is moved to the new namespace
Cast.

These other classes are referencing PhysicsCast that now lives in that other namespace.

thestonefox · 2018-05-16T06:12:16Z

Scripts/Pointer/StraightPointer.cs

@@ -1,7 +1,7 @@
 namespace VRTK.Core.Pointer
 {
    using UnityEngine;
-    using VRTK.Core.Utility;
+    using VRTK.Core.Cast;


why does this need Cast?

See my other comment: These other classes are referencing PhysicsCast that now lives in that other namespace.

thestonefox · 2018-05-16T06:12:46Z

Scripts/Utility/BezierCurveGenerator.cs

+{
+    using UnityEngine;
+
+    public static class BezierCurveGenerator


thestonefox · 2018-05-16T06:21:46Z

Scripts/Cast/ParabolicLineCast.cs

+        /// the downward cast.
+        /// </summary>
+        [Tooltip("The maximum length of the projected cast. The x value is the length of the forward cast, the y value is the length of the downward cast.")]
+        public Vector2 maximumLength = new Vector2(10f, float.PositiveInfinity);


This is going to inherit the existing problem with the bezier pointer cast (i will attempt to explain with MSPaint obvs)

Normal cast:

Limited cast:
(Down doesnt reach floor so not valid target)

Simply angling the forward cast beam down means you can now reach the floor

There is an open bug for this issue:
ExtendRealityLtd/VRTK#1288

No viable solution as of yet (maybe it relies on rotation of beam? or always does a forward beam in world forward for working out viability of downward length?)

Moved this into #89.

I did the requested changes and added a comment explaining Cast namespace change.

The pointer concept is the ability to designate an area of the scene with a destination. A pointer consists of multiple independent functionality which is implemented through a component that casts and returns points that define the pointer beam, one that generically renders points via game objects and a third component handles the selection functionality of a pointer. A pointer knows when it is activated, deactivated, when it starts hovering (enters) a new object, when it finishes hovering (exits) an object and knows what coordinates of the object it is hovering over. These are all cast as events with a special pointer payload containing information about the pointer target. The Straight Line Cast implementation casts a straight line from an origin point (e.g. controller) and stops when it collides with an object (or reaches the set maximum length). The same applies to the added Parabolic Line Cast, but it casts a parabolic line instead. Additionally the introduction of more components that raycast means the existing `PhysicsCast` class is moved to the new namespace `Cast`.

thestonefox · 2018-05-23T08:20:51Z

superseded by #95

bddckr added the w_inprogress waffle.io label label May 15, 2018

thestonefox previously requested changes May 16, 2018

View reviewed changes

bddckr force-pushed the feat/pointers branch from 7e4f568 to e72c5dd Compare May 20, 2018 09:51

bddckr changed the base branch from feat/solid-straight-pointer to master May 20, 2018 09:52

bddckr changed the title ~~feat(Pointer): split functionality~~ feat(Pointer): pointer implementation May 20, 2018

bddckr changed the title ~~feat(Pointer): pointer implementation~~ feat(Pointer): pointer implementation - resolves #7, resolves #8 May 20, 2018

bddckr force-pushed the feat/pointers branch 2 times, most recently from 565bc31 to 1142693 Compare May 20, 2018 10:06

This was referenced May 20, 2018

feat(Pointer): solid straight pointer - fixes #7 #83

Closed

Test the pointer implementation #90

Closed

bddckr requested a review from thestonefox May 20, 2018 10:19

bddckr mentioned this pull request May 20, 2018

Change TransformData to use global scale instead of local scale #92

Closed

bddckr force-pushed the feat/pointers branch 9 times, most recently from 116d342 to 1cf96c8 Compare May 21, 2018 12:44

bddckr force-pushed the feat/pointers branch from 1cf96c8 to 348e854 Compare May 21, 2018 14:46

thestonefox changed the title ~~feat(Pointer): pointer implementation - resolves #7, resolves #8~~ feat(Pointer): pointer implementation May 23, 2018

thestonefox closed this May 23, 2018

thestonefox deleted the feat/pointers branch May 23, 2018 11:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Pointer): pointer implementation #86

feat(Pointer): pointer implementation #86

bddckr commented May 15, 2018 •

edited

Loading

bddckr commented May 15, 2018 •

edited

Loading

thestonefox May 16, 2018

thestonefox May 16, 2018

thestonefox May 16, 2018

bddckr May 19, 2018

thestonefox May 16, 2018

thestonefox May 16, 2018

thestonefox May 16, 2018

thestonefox May 16, 2018

bddckr May 19, 2018

thestonefox May 16, 2018

bddckr May 19, 2018

thestonefox May 16, 2018

thestonefox May 16, 2018

bddckr May 19, 2018

thestonefox commented May 23, 2018

feat(Pointer): pointer implementation #86

feat(Pointer): pointer implementation #86

Conversation

bddckr commented May 15, 2018 • edited Loading

bddckr commented May 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thestonefox commented May 23, 2018

bddckr commented May 15, 2018 •

edited

Loading

bddckr commented May 15, 2018 •

edited

Loading