phetsims / projectile-motion

"Projectile Motion" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
15 stars 13 forks source link

Add what trajectory the DataProbe is on #243

Open zepumph opened 3 years ago

zepumph commented 3 years ago

From a design meeting with @arouinfar and @kathy-phet. Please note parent issue in #244.

Not only what dataPoint the probe is on, but what Trajectory that data point belongs to.

zepumph commented 3 years ago

@arouinfar, how important is this request, currently the model is set up that Trajectories own DataPoints, and so the reference in the opposite direction is non existent. When a Probe get's a point, it is sometimes a couple of steps after we have lost the reference to the Trajectory. I would guess a couple of hours for further investigation. And also I'm unsure about the quality of the solution. It doesn't feel like DataPoints should know about the Trajectories they belong to (in terms of Object Oriented modularity). I should maybe brainstorm other solutions.

arouinfar commented 3 years ago

@zepumph is there another way for a client to identify which trajectory is being measured? When measuring a point, dataProbe.dataPointProperty spits out something like:

"position": { "x": 7.5, "y": 5.773750000000007 }

Would those values be an exact match of something that would fall under trajectory_*.dataPoints?

zepumph commented 3 years ago

Yes, nice find! It is totally the exact match. Using the patch below, I saw that the DataPoint instance stored in the probe is the exact same data structure (same instance in memory) of the point stored in the Trajectory.dataPoints list.

Index: js/common/model/DataProbe.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/model/DataProbe.js b/js/common/model/DataProbe.js
--- a/js/common/model/DataProbe.js  (revision 3cab18ce7c800126c205bd9d1c342a9fad4d9b15)
+++ b/js/common/model/DataProbe.js  (date 1629737065752)
@@ -45,6 +45,17 @@
       phetioType: Property.PropertyIO( NullableIO( DataPoint.DataPointIO ) )
     } );

+    this.dataPointProperty.lazyLink( x => {
+      trajectoryGroup.forEach( trajectory => {
+        for ( let i = 0; i < trajectory.dataPoints.length; i++ ) {
+          const dataPoint = trajectory.dataPoints[ i ];
+          if ( dataPoint === x ) {
+            debugger;
+          }
+        }
+      } );
+    } );
+
     // @public
     this.isActiveProperty = new BooleanProperty( false, {
       tandem: tandem.createTandem( 'isActiveProperty' ),

So you could do a simple for loop through each trajectory to find which trajectory the data point goes with. In (what I feel would be) extremely rare cases where position was identical, then you could test every aspect of the data point.

zepumph commented 3 years ago

Sorry I didn't reassign, @arouinfar, what do you think about this approach, do you want me to explain if better or write out the wrapper code to accomplish this?

arouinfar commented 1 year ago

I'm not sure I would request this feature today if we were starting fresh. Let's reassess when work resumes on this sim. For now, I'm going to unassign myself.