Skip to content
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

Add what trajectory the DataProbe is on #243

Open
zepumph opened this issue Feb 8, 2021 · 5 comments
Open

Add what trajectory the DataProbe is on #243

zepumph opened this issue Feb 8, 2021 · 5 comments

Comments

@zepumph
Copy link
Member

zepumph commented Feb 8, 2021

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
Copy link
Member Author

zepumph commented Jul 26, 2021

@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.

@zepumph zepumph assigned arouinfar and unassigned zepumph Jul 26, 2021
@arouinfar
Copy link
Contributor

@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?

@arouinfar arouinfar assigned zepumph and unassigned arouinfar Aug 19, 2021
@zepumph
Copy link
Member Author

zepumph commented Aug 23, 2021

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
Copy link
Member Author

zepumph commented Sep 2, 2021

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
Copy link
Contributor

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.

@arouinfar arouinfar removed their assignment Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants