change vertexWithProposedPosition array to be a map of VertexLabel to…

… Vector2, and update functions and usages accordingly, see #398

js/quadrilateral/model/QuadrilateralModel.ts

@@ -11,7 +11,7 @@
 import BooleanProperty from '../../../../axon/js/BooleanProperty.js';
 import Tandem from '../../../../tandem/js/Tandem.js';
 import QuadrilateralQueryParameters from '../QuadrilateralQueryParameters.js';
-import QuadrilateralShapeModel, { VertexWithProposedPosition } from './QuadrilateralShapeModel.js';
+import QuadrilateralShapeModel, { VertexLabelToProposedPositionMap } from './QuadrilateralShapeModel.js';
 import Vector2 from '../../../../dot/js/Vector2.js';
 import Utils from '../../../../dot/js/Utils.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
@@ -135,11 +135,11 @@ export default class QuadrilateralModel implements TModel {
   /**
    * Returns true if the two vertex positions are allowed for the quadrilateral.
    */
-  public areVertexPositionsAllowed( verticesWithProposedPositions: VertexWithProposedPosition[] ): boolean {
+  public areVertexPositionsAllowed( labelToPositionMap: VertexLabelToProposedPositionMap ): boolean {
 
     // Set the test shape to the current value of the actual shape before proposed positions
     this.quadrilateralTestShapeModel.setFromShape( this.quadrilateralShapeModel );
-    this.quadrilateralTestShapeModel.setVertexPositions( verticesWithProposedPositions );
+    this.quadrilateralTestShapeModel.setVertexPositions( labelToPositionMap );
     return QuadrilateralShapeModel.isQuadrilateralShapeAllowed( this.quadrilateralTestShapeModel );
   }
 

js/quadrilateral/model/QuadrilateralShapeDetectorTests.ts

@@ -17,7 +17,7 @@ QUnit.module( 'BooleanProperty' );
 QUnit.test( 'QuadrilateralShapeDetector', assert => {
 
   // Our test shape, with dummy Properties for the constructor (parts that we don't care about for testing)
-  const shapeModel = new QuadrilateralShapeModel( new NumberProperty( 0.25 ), new BooleanProperty( true ), new NumberProperty( 1 ) );
+  const shapeModel = new QuadrilateralShapeModel( new BooleanProperty( true ), new NumberProperty( 1 ) );
 
   // Create one of each shape to verify basic detection. Position values found with dev
   // tool window.printVertexPositions() - only available when running with ?dev

js/quadrilateral/model/QuadrilateralShapeModel.ts

@@ -35,12 +35,7 @@ import SideLabel from './SideLabel.js';
 import QuadrilateralConstants from '../../QuadrilateralConstants.js';
 
 // Used when verifying that QuadrilateralVertex positions are valid before setting to the model.
-export type VertexWithProposedPosition = {
-  vertex: QuadrilateralVertex;
-
-  // This may not be available if something goes wrong with the marker detection
-  proposedPosition?: Vector2;
-};
+export type VertexLabelToProposedPositionMap = Map<VertexLabel, Vector2>;
 
 type QuadrilateralShapeModelOptions = {
 
@@ -386,24 +381,17 @@ export default class QuadrilateralShapeModel {
    * all are set. This way you can safely set multiple at a time without transient states where the shape is
    * not valid.
    */
-  public setVertexPositions( verticesWithProposedPositions: VertexWithProposedPosition[] ): void {
+  public setVertexPositions( labelToPositionMap: VertexLabelToProposedPositionMap ): void {
 
     this.setPropertiesDeferred( true );
 
-    // set all positions
-    verticesWithProposedPositions.forEach( vertexWithProposedPosition => {
+    labelToPositionMap.forEach( ( positionValue, labelKey ) => {
+      const vertex = this.getLabelledVertex( labelKey );
 
       // this is a new Vector2 instance so even if x,y values are the same as the old value it will trigger
       // listeners without this check
-      const proposedPosition = vertexWithProposedPosition.proposedPosition!;
-
-      // Review - should VertexWithProposedPositions use VertexLabel instead of the Vertex?
-      // We need to look-up the Vertex again here because we may have been provided a Vertex from a different
-      // QuadrilateralShapeModel instance.
-      const thisVertex = this.getLabelledVertex( vertexWithProposedPosition.vertex.vertexLabel );
-      assert && assert( proposedPosition, 'proposedPosition must be defined to set positions' );
-      if ( !proposedPosition.equals( thisVertex.positionProperty.value ) ) {
-        thisVertex.positionProperty.set( proposedPosition );
+      if ( !positionValue.equals( vertex.positionProperty.value ) ) {
+        vertex.positionProperty.value = positionValue;
       }
     } );
 

js/quadrilateral/model/prototype/TangibleConnectionModel.ts

@@ -27,9 +27,10 @@ import Tandem from '../../../../../tandem/js/Tandem.js';
 import NullableIO from '../../../../../tandem/js/types/NullableIO.js';
 import quadrilateral from '../../../quadrilateral.js';
 import QuadrilateralConstants from '../../../QuadrilateralConstants.js';
-import QuadrilateralShapeModel, { VertexWithProposedPosition } from '../QuadrilateralShapeModel.js';
+import QuadrilateralShapeModel, { VertexLabelToProposedPositionMap } from '../QuadrilateralShapeModel.js';
 import QuadrilateralTangibleOptionsModel from './QuadrilateralTangibleOptionsModel.js';
 import MarkerDetectionModel from './MarkerDetectionModel.js';
+import VertexLabel from '../VertexLabel.js';
 
 export default class TangibleConnectionModel {
 
@@ -113,35 +114,16 @@ export default class TangibleConnectionModel {
   }
 
   /**
-   * Apply a series of checks on VertexWithProposedPositions to make sure that the requested shape does not cross
+   * Apply a series of checks on the proposed positions to make sure that the requested shape does not cross
    * and does not have overlap.
    */
-  public isShapeAllowedForTangible( vertexWithProposedPositions: VertexWithProposedPosition[] ): boolean {
+  public isShapeAllowedForTangible( labelToPositionMap: VertexLabelToProposedPositionMap ): boolean {
     let allowed = true;
 
-    let vertexAPosition: Vector2;
-    let vertexBPosition: Vector2;
-    let vertexCPosition: Vector2;
-    let vertexDPosition: Vector2;
-
-    const shapeModel = this.shapeModel;
-
-    // REVIEW: Do we know each of these has only one association? If not, an earlier definition could be overwritten
-    // by null later - Use a Map instead of VertexWithProposedPosition[] to improve.
-    vertexWithProposedPositions.forEach( vertexWithProposedPosition => {
-      if ( vertexWithProposedPosition.vertex === shapeModel.vertexA ) {
-        vertexAPosition = vertexWithProposedPosition.proposedPosition!;
-      }
-      if ( vertexWithProposedPosition.vertex === shapeModel.vertexB ) {
-        vertexBPosition = vertexWithProposedPosition.proposedPosition!;
-      }
-      if ( vertexWithProposedPosition.vertex === shapeModel.vertexC ) {
-        vertexCPosition = vertexWithProposedPosition.proposedPosition!;
-      }
-      if ( vertexWithProposedPosition.vertex === shapeModel.vertexD ) {
-        vertexDPosition = vertexWithProposedPosition.proposedPosition!;
-      }
-    } );
+    const vertexAPosition = labelToPositionMap.get( VertexLabel.VERTEX_A );
+    const vertexBPosition = labelToPositionMap.get( VertexLabel.VERTEX_B );
+    const vertexCPosition = labelToPositionMap.get( VertexLabel.VERTEX_C );
+    const vertexDPosition = labelToPositionMap.get( VertexLabel.VERTEX_D );
 
     // all positions defined
     allowed = !!vertexAPosition! && !!vertexBPosition! && !!vertexCPosition! && !!vertexDPosition!;

js/quadrilateral/view/QuadrilateralSideNode.ts

@@ -12,7 +12,7 @@ import QuadrilateralSide from '../model/QuadrilateralSide.js';
 import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js';
 import Vector2 from '../../../../dot/js/Vector2.js';
 import QuadrilateralVertex from '../model/QuadrilateralVertex.js';
-import QuadrilateralShapeModel from '../model/QuadrilateralShapeModel.js';
+import QuadrilateralShapeModel, { VertexLabelToProposedPositionMap } from '../model/QuadrilateralShapeModel.js';
 import QuadrilateralModel from '../model/QuadrilateralModel.js';
 import { Line, Shape } from '../../../../kite/js/imports.js';
 import SideDescriber from './SideDescriber.js';
@@ -30,6 +30,9 @@ const FOCUS_HIGHLIGHT_DILATION = 15;
 type SelfOptions = EmptySelfOptions;
 type SideNodeOptions = SelfOptions & StrictOmit<QuadrilateralMovableNodeOptions, 'grabbedSoundOutputLevel' | 'grabbedSound'>;
 
+// Reusable map that saves proposed vertex positions, to avoid excessive garbage.
+const scratchLabelToPositionMap: VertexLabelToProposedPositionMap = new Map();
+
 class QuadrilateralSideNode extends QuadrilateralMovableNode {
 
   // A reference to the model component.
@@ -276,16 +279,14 @@ class QuadrilateralSideNode extends QuadrilateralMovableNode {
           const proposedVertex1Position = side.vertex1.positionProperty.value.plus( smallestDeltaVector );
           const proposedVertex2Position = side.vertex2.positionProperty.value.plus( smallestDeltaVector );
 
-          const verticesWithProposedPositions = [
-            { vertex: side.vertex1, proposedPosition: proposedVertex1Position },
-            { vertex: side.vertex2, proposedPosition: proposedVertex2Position }
-          ];
+          scratchLabelToPositionMap.clear();
+          scratchLabelToPositionMap.set( side.vertex1.vertexLabel, proposedVertex1Position );
+          scratchLabelToPositionMap.set( side.vertex2.vertexLabel, proposedVertex2Position );
 
           // only update positions if both are allowed
-          // const positionsAllowed = quadrilateralModel.areVertexPositionsAllowed( side.vertex1, proposedVertex1Position, side.vertex2, proposedVertex2Position );
-          const positionsAllowed = quadrilateralModel.areVertexPositionsAllowed( verticesWithProposedPositions );
+          const positionsAllowed = quadrilateralModel.areVertexPositionsAllowed( scratchLabelToPositionMap );
           if ( positionsAllowed ) {
-            this.quadrilateralShapeModel.setVertexPositions( verticesWithProposedPositions );
+            this.quadrilateralShapeModel.setVertexPositions( scratchLabelToPositionMap );
           }
 
           this.updateBlockedState( !positionsAllowed, !inBounds );
@@ -356,21 +357,18 @@ class QuadrilateralSideNode extends QuadrilateralMovableNode {
     // moving two vertices at the same time we need to check the validity after both have moved, checking the shape
     // moving one vertex at a time may result in incorrect results since that is not the shape we are ultimately
     // going to create with this change.
-    this.scratchShapeModel.setVertexPositions( [
-      { vertex: this.scratchSide.vertex1, proposedPosition: proposedVertex1Position },
-      { vertex: this.scratchSide.vertex2, proposedPosition: proposedVertex2Position }
-    ] );
+    scratchLabelToPositionMap.clear();
+    scratchLabelToPositionMap.set( this.scratchSide.vertex1.vertexLabel, proposedVertex1Position );
+    scratchLabelToPositionMap.set( this.scratchSide.vertex2.vertexLabel, proposedVertex2Position );
+    this.scratchShapeModel.setVertexPositions( scratchLabelToPositionMap );
 
     const isShapeAllowed = QuadrilateralShapeModel.isQuadrilateralShapeAllowed( this.scratchShapeModel );
     if ( isShapeAllowed ) {
 
       // signify to the Alerter that it will be time to generate a new object response from input
       this.side.voicingObjectResponseDirty = true;
 
-      this.quadrilateralShapeModel.setVertexPositions( [
-        { vertex: this.side.vertex1, proposedPosition: proposedVertex1Position },
-        { vertex: this.side.vertex2, proposedPosition: proposedVertex2Position }
-      ] );
+      this.quadrilateralShapeModel.setVertexPositions( scratchLabelToPositionMap );
     }
 
     this.updateBlockedState( !isShapeAllowed, !inBounds );
@@ -387,7 +385,10 @@ class QuadrilateralSideNode extends QuadrilateralMovableNode {
   private rotateVertexAroundOther( anchorVertex: QuadrilateralVertex, armVertex: QuadrilateralVertex, modelDelta: Vector2 ): void {
     const modelPosition = armVertex.positionProperty.get().plus( modelDelta );
     const proposedPosition = this.quadrilateralModel.getClosestGridPosition( modelPosition );
-    if ( this.quadrilateralModel.areVertexPositionsAllowed( [ { vertex: armVertex, proposedPosition: proposedPosition } ] ) ) {
+
+    scratchLabelToPositionMap.clear();
+    scratchLabelToPositionMap.set( armVertex.vertexLabel, proposedPosition );
+    if ( this.quadrilateralModel.areVertexPositionsAllowed( scratchLabelToPositionMap ) ) {
       armVertex.positionProperty.value = proposedPosition;
     }
   }

js/quadrilateral/view/QuadrilateralVertexNode.ts

@@ -24,13 +24,17 @@ import QuadrilateralMovableNode, { QuadrilateralMovableNodeOptions } from './Qua
 import StrictOmit from '../../../../phet-core/js/types/StrictOmit.js';
 import optionize, { EmptySelfOptions } from '../../../../phet-core/js/optionize.js';
 import TReadOnlyProperty from '../../../../axon/js/TReadOnlyProperty.js';
+import { VertexLabelToProposedPositionMap } from '../model/QuadrilateralShapeModel.js';
 
 // constants
 const LABEL_TEXT_FONT = new PhetFont( { size: 16, weight: 'bold' } );
 
 type SelfOptions = EmptySelfOptions;
 type VertexNodeOptions = SelfOptions & StrictOmit<QuadrilateralMovableNodeOptions, 'grabbedSound'>;
 
+// Reusable map that saves proposed vertex positions, to avoid excessive garbage.
+const scratchLabelToPositionMap: VertexLabelToProposedPositionMap = new Map();
+
 export default class QuadrilateralVertexNode extends QuadrilateralMovableNode {
   private readonly quadrilateralModel: QuadrilateralModel;
   private readonly vertex: QuadrilateralVertex;
@@ -104,7 +108,8 @@ export default class QuadrilateralVertexNode extends QuadrilateralMovableNode {
         const inBoundsPosition = quadrilateralModel.vertexDragBounds.closestPointTo( proposedPosition );
         const isAgainstBounds = !inBoundsPosition.equals( proposedPosition );
 
-        const isPositionAllowed = quadrilateralModel.areVertexPositionsAllowed( [ { vertex: vertex, proposedPosition: inBoundsPosition } ] );
+        scratchLabelToPositionMap.set( vertex.vertexLabel, inBoundsPosition );
+        const isPositionAllowed = quadrilateralModel.areVertexPositionsAllowed( scratchLabelToPositionMap );
         if ( isPositionAllowed ) {
 
           // only update and trigger a new Voicing response if the position has changed.
@@ -148,7 +153,9 @@ export default class QuadrilateralVertexNode extends QuadrilateralMovableNode {
         // constrain to the allowable positions in the model along the grid
         const constrainedPosition = quadrilateralModel.getClosestGridPosition( inBoundsPosition );
 
-        const isPositionAllowed = quadrilateralModel.areVertexPositionsAllowed( [ { vertex: vertex, proposedPosition: constrainedPosition } ] );
+        scratchLabelToPositionMap.clear();
+        scratchLabelToPositionMap.set( vertex.vertexLabel, constrainedPosition );
+        const isPositionAllowed = quadrilateralModel.areVertexPositionsAllowed( scratchLabelToPositionMap );
 
         if ( isPositionAllowed ) {
           vertex.positionProperty.value = constrainedPosition;

js/quadrilateral/view/prototype/QuadrilateralMediaPipe.ts

@@ -21,9 +21,10 @@ import quadrilateral from '../../../quadrilateral.js';
 import MediaPipe, { HandLandmarks } from '../../../../../tangible/js/mediaPipe/MediaPipe.js';
 import QuadrilateralModel from '../../model/QuadrilateralModel.js';
 import Vector2 from '../../../../../dot/js/Vector2.js';
-import QuadrilateralShapeModel from '../../model/QuadrilateralShapeModel.js';
+import QuadrilateralShapeModel, { VertexLabelToProposedPositionMap } from '../../model/QuadrilateralShapeModel.js';
 import MediaPipeQueryParameters from '../../../../../tangible/js/mediaPipe/MediaPipeQueryParameters.js';
 import QuadrilateralTangibleController from './QuadrilateralTangibleController.js';
+import VertexLabel from '../../model/VertexLabel.js';
 
 // aspect ratio of the video stream to map camera coordinates to sim model coordinates
 const streamDimension2 = MediaPipe.videoStreamDimension2;
@@ -43,6 +44,9 @@ if ( MediaPipeQueryParameters.cameraInput === 'hands' ) {
   MediaPipe.initialize();
 }
 
+// A reusable map that has proposed vertex positions, to avoid lots of garbage.
+const labelToProposedPositionMap: VertexLabelToProposedPositionMap = new Map();
+
 export default class QuadrilateralMediaPipe extends MediaPipe {
   private readonly quadrilateralShapeModel: QuadrilateralShapeModel;
   private readonly tangibleController: QuadrilateralTangibleController;
@@ -84,23 +88,12 @@ export default class QuadrilateralMediaPipe extends MediaPipe {
         const rightHandPositions = sortedPositions[ 1 ];
 
         // package and attempt to update shape
-        const firstPositionProposal = {
-          vertex: this.quadrilateralShapeModel.vertexA,
-          proposedPosition: leftHandPositions.indexPosition
-        };
-        const secondPositionProposal = {
-          vertex: this.quadrilateralShapeModel.vertexB,
-          proposedPosition: rightHandPositions.indexPosition
-        };
-        const thirdPositionProposal = {
-          vertex: this.quadrilateralShapeModel.vertexC,
-          proposedPosition: rightHandPositions.thumbPosition
-        };
-        const fourthPositionProposal = {
-          vertex: this.quadrilateralShapeModel.vertexD,
-          proposedPosition: leftHandPositions.thumbPosition
-        };
-        this.tangibleController.setPositionsFromAbsolutePositionData( [ firstPositionProposal, secondPositionProposal, thirdPositionProposal, fourthPositionProposal ] );
+        labelToProposedPositionMap.set( VertexLabel.VERTEX_A, leftHandPositions.indexPosition );
+        labelToProposedPositionMap.set( VertexLabel.VERTEX_B, rightHandPositions.indexPosition );
+        labelToProposedPositionMap.set( VertexLabel.VERTEX_C, rightHandPositions.thumbPosition );
+        labelToProposedPositionMap.set( VertexLabel.VERTEX_D, leftHandPositions.thumbPosition );
+
+        this.tangibleController.setPositionsFromAbsolutePositionData( labelToProposedPositionMap );
       }
     }
   }