phetsims / equality-explorer

"Equality Explorer" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
2 stars 3 forks source link

How to make OopsDialogIO extend DialogIO? #195

Closed pixelzoom closed 1 year ago

pixelzoom commented 1 year ago

To instrument equality-explorer for #192, I'm using a PhetioCapsule to manage scenery-phet.OopsDialog.

Here's the relevant code in EqualityExplorerSceneNode:

    // To manage dynamic instrumented OopsDialog
    const oopsDialogCapsule = new PhetioCapsule( tandem => new OopsDialog( EqualityExplorerStrings.numberTooBigStringProperty, {
      //TODO focusOnHideNode:
      tandem: tandem
    } ), [], {
      tandem: providedOptions.tandem.createTandem( 'oopsDialogCapsule' ),
      phetioType: PhetioCapsule.PhetioCapsuleIO( OopsDialog.OopsDialogIO )
    } );

    // When the maxInteger limit is exceeded, dispose of all terms that are not on the scale, and display a dialog.
    // To test this, see EqualityExplorerQueryParameters.maxInteger.
    const maxIntegerExceededListener = () => {
      phet.log && phet.log( 'maxInteger exceeded' );
      scene.disposeTermsNotOnScale();
      oopsDialogCapsule.getElement().show();
    };

Here's the current definition of OopsDialogIO in OopsDialog.ts:

public static readonly OopsDialogIO = Dialog.DialogIO;

But this definition of OopsDialog doesn't seem quite right to me. OopsDialog is a subclass of Dialog, so OopsDialogIO should be a subclass of DialogIO. So I had originally defined OopsDialogIO like this:

  public static readonly OopsDialogIO = new IOType( 'OopsDialogIO', {
    valueType: OopsDialog,
    supertype: Dialog.DialogIO
  } );

That fails in Studio with:

assert.js:28 Uncaught Error: Assertion failed: dynamic element container expected its created instance's phetioType to match its parameterType. at window.assertions.assertFunction (assert.js:28:13) at PhetioCapsule.createDynamicElement (PhetioDynamicElementContainer.ts:302:17) at PhetioCapsule.create (PhetioCapsule.ts:106:25) at PhetioCapsule.getElement (PhetioCapsule.ts:79:12) at maxIntegerExceededListener (EqualityExplorerSceneNode.ts:78:25) at TinyEmitter.emit (TinyEmitter.ts:95:9) at Emitter.emit (Emitter.ts:60:22) at OperationsScene.applyOperation (OperationsScene.ts:194:45) at UniversalOperationControl.ts:260:17 at TinyProperty.emit (TinyEmitter.ts:95:9)

To reproduce:

  1. Run http://localhost:8080/studio/?sim=equality-explorer&locales=*&keyboardLocaleSwitcher&phetioWrapperDebug=true&phetioElementsDisplay=all&maxInteger=1
  2. Go to the Operations screen
  3. Press the round yellow arrow button twice.

What am I doing wrong?

samreid commented 1 year ago

Do I need to apply a patch to see this assertion error?

zepumph commented 1 year ago

I believe all that is needed here is making sure the core type is providing the right phetioType too:


Index: js/OopsDialog.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/OopsDialog.ts b/js/OopsDialog.ts
--- a/js/OopsDialog.ts  (revision 3de8066bd9f1563ac64bc1fc5fefdd16dd31f834)
+++ b/js/OopsDialog.ts  (date 1666379855031)
@@ -14,6 +14,7 @@
 import { HBox, Image, Node, RichText, RichTextOptions } from '../../scenery/js/imports.js';
 import Dialog, { DialogOptions } from '../../sun/js/Dialog.js';
 import Tandem from '../../tandem/js/Tandem.js';
+import IOType from '../../tandem/js/types/IOType.js';
 import phetGirlWaggingFinger_png from '../images/phetGirlWaggingFinger_png.js';
 import PhetFont from './PhetFont.js';
 import sceneryPhet from './sceneryPhet.js';
@@ -48,7 +49,8 @@
       bottomMargin: 20,

       // phet-io
-      tandem: Tandem.OPTIONAL
+      tandem: Tandem.OPTIONAL,
+      phetioType: OopsDialog.OopsDialogIO
     }, providedOptions );

     const text = new RichText( messageString, optionize<RichTextOptions, EmptySelfOptions, RichTextOptions>()( {
@@ -80,7 +82,10 @@
   }

   //TODO https://github.com/phetsims/equality-explorer/issues/195 should OopsDialogIO extend DialogIO?
-  public static readonly OopsDialogIO = Dialog.DialogIO;
+  public static readonly OopsDialogIO = new IOType( 'OopsDialogIO', {
+    valueType: OopsDialog,
+    supertype: Dialog.DialogIO
+  } );
 }

 sceneryPhet.register( 'OopsDialog', OopsDialog );
\ No newline at end of file
pixelzoom commented 1 year ago

Ah yes, thanks -- the old "missing phetoioType" problem. This is something that I unfortunately do all the time, in many different places. And the error messages are almost always incomprehensible and downstream. Usually it only wastes my time. Sorry to have dragged 2 more devs in this time.

pixelzoom commented 1 year ago

OopsDialogIO has been redefined in the above commit. Closing.