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

CT phetioElement not found #319

Closed
KatieWoe opened this issue Jan 27, 2021 · 7 comments
Closed

CT phetioElement not found #319

KatieWoe opened this issue Jan 27, 2021 · 7 comments

Comments

@KatieWoe
Copy link
Contributor

energy-skate-park : phet-io-studio-fuzz : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611779185807/studio/?sim=energy-skate-park&phetioDebug&fuzz&wrapperContinuousTest=%7B%22test%22%3A%5B%22energy-skate-park%22%2C%22phet-io-studio-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1611779185807%22%2C%22timestamp%22%3A1611783126661%7D
Uncaught Error: phetioElement not found: energySkatePark.playgroundScreen.playgroundScreenView.trackNodeGroup.archetype.controlPointNode1.controlPointUI
Error: phetioElement not found: energySkatePark.playgroundScreen.playgroundScreenView.trackNodeGroup.archetype.controlPointNode1.controlPointUI
at window.assert (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611779185807/phet-io-wrappers/common/js/assert.js:32:13)
at Studio.onPhetioElementAdded (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611779185807/studio/js/studio.js:252:19)
at phetioElementAdded (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611779185807/studio/js/studio-main.js:42:10)
at Client.dispatch (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611779185807/phet-io-wrappers/common/js/Client.js:456:64)
at https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611779185807/phet-io-wrappers/common/js/Client.js:323:26
at Array.forEach (<anonymous>)
at windowMessageListener (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1611779185807/phet-io-wrappers/common/js/Client.js:305:29)
id: Bayes Chrome
Snapshot from 1/27/2021, 1:26:25 PM
@jessegreenberg
Copy link
Contributor

I think this may be related to other PhET-iO issues that have been worked on recently, #123 and #318. Reassigning, but let me know if I can help.

@samreid
Copy link
Member

samreid commented Jan 29, 2021

I made progress in this patch:

Index: js/common/view/TrackNode.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/TrackNode.js b/js/common/view/TrackNode.js
--- a/js/common/view/TrackNode.js	(revision de18ba9771162cfce6bf0bece84a014a9e723d74)
+++ b/js/common/view/TrackNode.js	(date 1611942616377)
@@ -32,7 +32,7 @@
    * @param {Tandem} tandem
    * @param {Object} [options]
    */
-  constructor( track, modelViewTransform, availableBoundsProperty, tandem, options ) {
+  constructor( track, modelViewTransform, availableBoundsProperty, controlPointNodeGroup, tandem, options ) {
     const model = track.model;
 
     options = merge( {
@@ -105,7 +105,9 @@
 
         if ( controlPoint.visible ) {
           const isEndPoint = i === 0 || i === track.controlPoints.length - 1;
-          const controlPointNode = new ControlPointNode( this, this.trackDragHandler, i, isEndPoint, tandem.createTandem( 'controlPointNode' + i ) );
+          const controlPointNode = controlPointNodeGroup ?
+                                   controlPointNodeGroup.createNextElement( this, i, isEndPoint ) :
+                                   new ControlPointNode( this, this.trackDragHandler, i, isEndPoint, tandem.createTandem( 'controlPointNode' + i ) );
           this.addChild( controlPointNode );
 
           if ( controlPoint.limitBounds ) {
Index: js/common/view/TrackToolboxPanel.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/TrackToolboxPanel.js b/js/common/view/TrackToolboxPanel.js
--- a/js/common/view/TrackToolboxPanel.js	(revision de18ba9771162cfce6bf0bece84a014a9e723d74)
+++ b/js/common/view/TrackToolboxPanel.js	(date 1611942558539)
@@ -35,7 +35,7 @@
         interactive: false
       }
     } );
-    const iconNode = new TrackNode( iconTrack, view.modelViewTransform, model.availableModelBoundsProperty, tandem.createTandem( 'iconNode' ), {
+    const iconNode = new TrackNode( iconTrack, view.modelViewTransform, model.availableModelBoundsProperty, null, tandem.createTandem( 'iconNode' ), {
 
       // want the icon to look pickable, even though it isn't really draggable (forwarding listener makes the new
       // TrackNode draggable)
Index: js/common/view/SceneSelectionRadioButtonGroup.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/SceneSelectionRadioButtonGroup.js b/js/common/view/SceneSelectionRadioButtonGroup.js
--- a/js/common/view/SceneSelectionRadioButtonGroup.js	(revision de18ba9771162cfce6bf0bece84a014a9e723d74)
+++ b/js/common/view/SceneSelectionRadioButtonGroup.js	(date 1611942558536)
@@ -115,7 +115,7 @@
       }
 
       const track = createIconTrack( model.trackTypes[ index ] );
-      const trackNode = new TrackNode( track, view.modelViewTransform, new Property(), tandem.createTandem( 'trackNode' + index ), {
+      const trackNode = new TrackNode( track, view.modelViewTransform, new Property(), null,tandem.createTandem( 'trackNode' + index ), {
         isIcon: true
       } );
 
Index: js/common/view/EnergySkateParkScreenView.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/common/view/EnergySkateParkScreenView.js b/js/common/view/EnergySkateParkScreenView.js
--- a/js/common/view/EnergySkateParkScreenView.js	(revision de18ba9771162cfce6bf0bece84a014a9e723d74)
+++ b/js/common/view/EnergySkateParkScreenView.js	(date 1611942558547)
@@ -36,6 +36,7 @@
 import EnergySkateParkConstants from '../EnergySkateParkConstants.js';
 import AttachDetachToggleButtons from './AttachDetachToggleButtons.js';
 import BackgroundNode from './BackgroundNode.js';
+import ControlPointNode from './ControlPointNode.js';
 import EnergyBarGraphAccordionBox from './EnergyBarGraphAccordionBox.js';
 import EnergySkateParkColorScheme from './EnergySkateParkColorScheme.js';
 import EnergySkateParkControlPanel from './EnergySkateParkControlPanel.js';
@@ -133,12 +134,24 @@
     this.trackNodeGroup = new PhetioGroup( ( tandem, track, modelViewTransform, availableBoundsProperty, options ) => {
       assert && options && assert( !options.hasOwnProperty( 'tandem' ), 'tandem is managed by the PhetioGroup' );
       options = merge( { phetioDynamicElement: true }, options );
-      return new TrackNode( track, modelViewTransform, availableBoundsProperty, tandem, options );
+      return new TrackNode( track, modelViewTransform, availableBoundsProperty, this.controlPointNodeGroup,tandem, options );
     }, [ model.trackGroup.archetype, modelViewTransform, this.availableModelBoundsProperty, {} ], {
       tandem: tandem.createTandem( 'trackNodeGroup' ),
       phetioType: PhetioGroup.PhetioGroupIO( Node.NodeIO ),
       phetioDynamicElementName: 'trackNode',
 
+      // These elements are not created by the PhET-IO state engine, they can just listen to the model for supporting
+      // state in the same way they do for sim logic.
+      supportsDynamicState: false
+    } );
+
+    this.controlPointNodeGroup = new PhetioGroup( ( tandem, trackNode, index, isEndPoint ) => {
+      return new ControlPointNode( trackNode, trackNode.trackDragHandler, index, isEndPoint, tandem );
+    }, [ this.trackNodeGroup.archetype, 0, true ], {
+      tandem: tandem.createTandem( 'controlPointNodeGroup' ),
+      phetioType: PhetioGroup.PhetioGroupIO( Node.NodeIO ),
+      phetioDynamicElementName: 'controlPointNode',
+
       // These elements are not created by the PhET-IO state engine, they can just listen to the model for supporting
       // state in the same way they do for sim logic.
       supportsDynamicState: false

However, I ran into this cycle:

The controlPointNodeGroup requires a reference to the trackNodeGroup archetype before it can be created. But the TrackNodes created by trackNodeGroup need the controlPointNodeGroup so they can create points. We will need to unravel this somehow.

I'm not sure whether to (a) ask @zepumph for help on it now or (b) temporarily uninstrument until we give more focus to this sim instrumentation. @zepumph can you please advise?

@samreid samreid assigned zepumph and unassigned samreid Jan 29, 2021
@zepumph
Copy link
Member

zepumph commented Jan 30, 2021

Why do these view elements need to be instrumented as dynamic elements? Aren't only the models stateful (trident)? Could we see how far we get by first opting out of TrackDragHandler's instrumentation, and others like it in the view?

That said, it wouldn't bother me if we remove ESP from PhET-iO testing until you get back to working on that sim specifically.

@zepumph zepumph assigned samreid and unassigned zepumph Jan 30, 2021
@samreid
Copy link
Member

samreid commented Jan 30, 2021

I think I understand the trident and that only the models are stateful, but since the view nodes need to be instrumented, don't they also need to be in a PhetioGroup, since there is an arbitrary number of them?

@samreid samreid assigned zepumph and unassigned samreid Jan 30, 2021
@zepumph
Copy link
Member

zepumph commented Jan 30, 2021

What needs to be instrumented about them? I saw the input listener, and perhaps we can punt on that for now, but is there anything else?

@zepumph zepumph assigned samreid and unassigned zepumph Jan 30, 2021
@samreid
Copy link
Member

samreid commented Jan 31, 2021

Yes, I thought they should be instrumented because they have input listeners. But uninstrumenting them seems like a good temporary solution until we focus on this sim.

@samreid
Copy link
Member

samreid commented Feb 1, 2021

OK I've opted out of the TrackNode instrumentation. Fuzzing has been going for a relatively long time on my machine without error. Long term solution will be done in #123. Closing for now (CT has the final say).

@samreid samreid closed this as completed Feb 1, 2021
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

4 participants