What are the position units of this sim?

[zepumph]

From #236, I was asked to document the topBookPositionProperty and wanted to also do distanceBetweenBooksProperty. I found that this was not an easy topic.

I used the inspector to get even more confused.

I am trying to figure out what the distance between the two books are at this time.

If the distance between books was just the pixel coords in the global frame, then the console logged value (of globalBounds differences) would be the same as the distance between books, but that isn't the case.

I will need to come back to this before adding that documentation.

[zepumph]

• As part of this, please update the phetioDocumentation for
  model.bottomOffsetProperty and model.topBookPositionProperty (specify the coordinate frame).

[zepumph]

Some more research looking into this with ?sceneryLog=InputEvent showed this relationship with logging deltas from model.move()

[InputEvent] mouse move at Vector2(784, 349)
Vector2 {x: 0, y: 1.1136583344841995}

[InputEvent] mouse move at Vector2(784, 350)
Vector2 {x: 0, y: 0.5568291672420855}

[InputEvent] mouse move at Vector2(784, 351)
Vector2 {x: 0, y: 0.5568291672420855}

[InputEvent] mouse move at Vector2(784, 353)
Vector2 {x: 0, y: 1.1136583344841995}

[InputEvent] mouse move at Vector2(784, 354)
Vector2 {x: 0, y: 0.5568291672420855}

[InputEvent] mouse move at Vector2(784, 355)
Vector2 {x: 0, y: 0.5568291672420855}

[zepumph]

At each drag event, I printed out what the DragListener.modelPoint was, and also what the value of FrictionModel.topBookPositionProperty.value. I then put them in a spreadsheet and compared the values. On average, the values are off by a bit (.22x, .94y). I am generally confused by this. I would understand if the only discrepancy was when the book touched the other, but there is a difference from the very start of the drag.

topVsDrag.csv

[zepumph]

@samreid and I had an investigation, and we found that having the the DragListener take the model bounds that the frictionKeyboardDragListener was getting was helpful in that it totally removed the discrepancy between the listener's modelPoint and the topBookPositionProperty. This was only true for three directions though. The bottom direction still needed to use the model to determine if book were touching, and to adjust the top book position accordingly. From this work we could simplify the move() function in the model, and I also understand what is occurring in the view and model much better.

I'll update documentation from here.

[zepumph]

5bfcffe

Oops, looks like we need to supply different bounds for the macro view of the book:

I think I can fix that by having a second bounds that takes into account model.bookDraggingScaleFactor, and pass that in for the bookNode's listeners.

[zepumph]

#239 (comment) has not been done yet. I hope to get to it today.

[zepumph]

Index: js/friction/view/book/BookNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/friction/view/book/BookNode.js b/js/friction/view/book/BookNode.js
--- a/js/friction/view/book/BookNode.js	(revision f2b21a8e34f4dbfcd6d057bd746257c0f4c3f716)
+++ b/js/friction/view/book/BookNode.js	(date 1634148117719)
@@ -110,6 +110,7 @@
       soundManager.addSoundGenerator( bookDropSoundClip );
 
       // pdom - add a keyboard drag handler
+      // TODO: why don't you get smaller bounds!?!?!? https://github.com/phetsims/friction/issues/239
       this.keyboardDragHandler = new FrictionKeyboardDragListener( model, temperatureIncreasingAlerter,
         temperatureDecreasingAlerter, bookMovementAlerter );
 
Index: js/friction/view/FrictionKeyboardDragListener.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/friction/view/FrictionKeyboardDragListener.js b/js/friction/view/FrictionKeyboardDragListener.js
--- a/js/friction/view/FrictionKeyboardDragListener.js	(revision f2b21a8e34f4dbfcd6d057bd746257c0f4c3f716)
+++ b/js/friction/view/FrictionKeyboardDragListener.js	(date 1634156952072)
@@ -6,6 +6,7 @@
  * @author Michael Kauzmann (PhET Interactive Simulations)
  */
 
+import merge from '../../../../phet-core/js/merge.js';
 import KeyboardDragListener from '../../../../scenery/js/listeners/KeyboardDragListener.js';
 import friction from '../../friction.js';
 import FrictionModel from '../model/FrictionModel.js';
@@ -17,9 +18,9 @@
    * @param {TemperatureDecreasingAlerter} temperatureDecreasingAlerter
    * @param {BookMovementAlerter} bookMovementAlerter
    */
-  constructor( model, temperatureIncreasingAlerter, temperatureDecreasingAlerter, bookMovementAlerter ) {
+  constructor( model, temperatureIncreasingAlerter, temperatureDecreasingAlerter, bookMovementAlerter, options ) {
 
-    super( {
+    options = merge( {
       start: () => {
         temperatureIncreasingAlerter.startDrag();
         temperatureDecreasingAlerter.startDrag();
@@ -33,7 +34,8 @@
 
       },
       dragBounds: FrictionModel.MAGNIFIED_DRAG_BOUNDS
-    } );
+    }, options );
+    super( options );
   }
 }
 

[zepumph]

After this patch of investigation, I think I would like to mutate/change the bounds as the model changes. This is great opportunity to add support to keyboardDragListener for dragBoundsProperty (in phetsims/scenery#1307).

Patch that is messy and bad. I would rather rework the mode to run on the positionProperty instead of using deltas.

Index: js/friction/view/book/BookNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/friction/view/book/BookNode.js b/js/friction/view/book/BookNode.js
--- a/js/friction/view/book/BookNode.js	(revision 567ffcddd039e6eb84632926d6d626feb8b103f5)
+++ b/js/friction/view/book/BookNode.js	(date 1635436267960)
@@ -22,6 +22,7 @@
 import friction from '../../../friction.js';
 import frictionStrings from '../../../frictionStrings.js';
 import FrictionConstants from '../../FrictionConstants.js';
+import FrictionModel from '../../model/FrictionModel.js';
 import CueArrow from '../CueArrow.js';
 import FrictionDragListener from '../FrictionDragListener.js';
 import FrictionGrabDragInteraction from '../FrictionGrabDragInteraction.js';
@@ -111,7 +112,10 @@
 
       // pdom - add a keyboard drag handler
       this.keyboardDragHandler = new FrictionKeyboardDragListener( model, temperatureIncreasingAlerter,
-        temperatureDecreasingAlerter, bookMovementAlerter );
+        temperatureDecreasingAlerter, bookMovementAlerter, {
+          dragBounds: FrictionModel.MACRO_DRAG_BOUNDS,
+          true: true
+        } );
 
       // highlights must be added prior to adding the grab/drag interaction, this is a constraint of GrabDragInteraction
       this.addChild( focusHighlightRect );
@@ -160,7 +164,8 @@
 
       // add observer
       model.topBookPositionProperty.link( position => {
-        this.setTranslation( options.x + position.x * model.bookDraggingScaleFactor, options.y + position.y * model.bookDraggingScaleFactor );
+        this.setTranslation( options.x + position.x * FrictionModel.BOOK_DRAGGING_SCALE_FACTOR,
+          options.y + position.y * FrictionModel.BOOK_DRAGGING_SCALE_FACTOR );
       } );
 
       this.mutate( {
Index: js/friction/model/FrictionModel.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/friction/model/FrictionModel.js b/js/friction/model/FrictionModel.js
--- a/js/friction/model/FrictionModel.js	(revision 567ffcddd039e6eb84632926d6d626feb8b103f5)
+++ b/js/friction/model/FrictionModel.js	(date 1635436035394)
@@ -250,9 +250,6 @@
                            'overridden by toggling the "hintArrowsNode.visibleProperty" in the view.'
     } );
 
-    // @public {Number} (read-only) - drag and drop book coordinates conversion coefficient
-    this.bookDraggingScaleFactor = 0.025;
-
     // @public (read-only) {Atom[]} - array of atoms that are visible to the user in the magnifier window
     this.atoms = [];
 
@@ -478,12 +475,20 @@
 // pdom - empirically determined value of when the atoms are "pretty much cool and settled"
 FrictionModel.AMPLITUDE_SETTLED_THRESHOLD = VIBRATION_AMPLITUDE_MIN + 0.4;
 
+FrictionModel.BOOK_DRAGGING_SCALE_FACTOR = 0.025;
+
 // The drag bounds for the magnified book view
 FrictionModel.MAGNIFIED_DRAG_BOUNDS = new Bounds2(
   -MAX_X_DISPLACEMENT, // left bound
   MIN_Y_POSITION, // top bound
   MAX_X_DISPLACEMENT, // right bound
   2000 );
+
+FrictionModel.MACRO_DRAG_BOUNDS = new Bounds2(
+  FrictionModel.MAGNIFIED_DRAG_BOUNDS.left * FrictionModel.BOOK_DRAGGING_SCALE_FACTOR, // left bound
+  FrictionModel.MAGNIFIED_DRAG_BOUNDS.top * FrictionModel.BOOK_DRAGGING_SCALE_FACTOR, // top bound
+  FrictionModel.MAGNIFIED_DRAG_BOUNDS.right * FrictionModel.BOOK_DRAGGING_SCALE_FACTOR, // right bound
+  2000 );
 
 FrictionModel.FrictionModelIO = IOType.fromCoreType( 'FrictionModelIO', FrictionModel, {
   documentation: 'model for the simulation'
Index: js/friction/view/FrictionKeyboardDragListener.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/friction/view/FrictionKeyboardDragListener.js b/js/friction/view/FrictionKeyboardDragListener.js
--- a/js/friction/view/FrictionKeyboardDragListener.js	(revision 567ffcddd039e6eb84632926d6d626feb8b103f5)
+++ b/js/friction/view/FrictionKeyboardDragListener.js	(date 1635436621388)
@@ -6,6 +6,7 @@
  * @author Michael Kauzmann (PhET Interactive Simulations)
  */
 
+import merge from '../../../../phet-core/js/merge.js';
 import KeyboardDragListener from '../../../../scenery/js/listeners/KeyboardDragListener.js';
 import friction from '../../friction.js';
 import FrictionModel from '../model/FrictionModel.js';
@@ -16,15 +17,23 @@
    * @param {TemperatureIncreasingAlerter} temperatureIncreasingAlerter
    * @param {TemperatureDecreasingAlerter} temperatureDecreasingAlerter
    * @param {BookMovementAlerter} bookMovementAlerter
+   * @param {Object} [options]
+   *
    */
-  constructor( model, temperatureIncreasingAlerter, temperatureDecreasingAlerter, bookMovementAlerter ) {
+  constructor( model, temperatureIncreasingAlerter, temperatureDecreasingAlerter, bookMovementAlerter, options ) {
 
-    super( {
+    options = merge( {
       start: () => {
         temperatureIncreasingAlerter.startDrag();
         temperatureDecreasingAlerter.startDrag();
       },
-      drag: vectorDelta => model.move( vectorDelta ),
+      drag: vectorDelta => {
+
+        // TODO: only good util the first time the delta goes over
+        if ( this.dragBounds.containsPoint( model.topBookPositionProperty.value ) ) {
+          model.move( vectorDelta );
+        }
+      },
       end: event => {
         model.bottomOffsetProperty.set( 0 );
 
@@ -32,8 +41,12 @@
         bookMovementAlerter.endDrag( event.domEvent );
 
       },
-      dragBounds: FrictionModel.MAGNIFIED_DRAG_BOUNDS
-    } );
+      dragBounds: FrictionModel.MAGNIFIED_DRAG_BOUNDS,
+      true: false
+    }, options );
+    super( options );
+
+    this.true = options.true;
   }
 }
 

[zepumph]

Alright, converting the model to use positionProperty in drag listeners went really well! It is working above.

[zepumph]

From this investigation, I feel confident that this line is accurate documentation for the coordinates of the sim:

friction/js/friction/model/FrictionModel.js

Line 209 in 74bd131

phetioDocumentation: 'The position of the top book. Model and view coordinates are the same in this simulation.',

I am ready to close.