Create MapIO

[samreid]

Greenhouse may need a MapIO

[samreid]

@jessegreenberg and I created this today, but did not have an opportunity to take it for a test drive in any circumstance. Greenhouse is under revision and may not need this feature soon. Still, it seems like the right direction for master, so I've left it committed and added a note about its status. I'm planning to unassign myself until we have a use case for this again, but @zepumph can you take a quick look and see if it seems like the right direction?

[zepumph]

The implementation looks solid to me. I factored out the isValidValue function to remove the todo. I think used this patch to explore it.

Index: js/gravity-and-orbits-main.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/gravity-and-orbits-main.js b/js/gravity-and-orbits-main.js
--- a/js/gravity-and-orbits-main.js	(revision c792b8dabb439d78dba9119602148b1d78ca76dc)
+++ b/js/gravity-and-orbits-main.js	(date 1627568838162)
@@ -6,9 +6,13 @@
  * @author Aaron Davis (PhET Interactive Simulations)
  */
 
+import Property from '../../axon/js/Property.js';
 import Sim from '../../joist/js/Sim.js';
 import simLauncher from '../../joist/js/simLauncher.js';
 import Tandem from '../../tandem/js/Tandem.js';
+import MapIO from '../../tandem/js/types/MapIO.js';
+import NumberIO from '../../tandem/js/types/NumberIO.js';
+import StringIO from '../../tandem/js/types/StringIO.js';
 import GravityAndOrbitsColors from './common/GravityAndOrbitsColors.js';
 import GlobalOptionsNode from './common/view/GlobalOptionsNode.js';
 import gravityAndOrbitsStrings from './gravityAndOrbitsStrings.js';
@@ -46,5 +50,11 @@
     backgroundColorProperty: backgroundColorProperty,
     tandem: Tandem.ROOT.createTandem( 'toScaleScreen' )
   } );
+
+  const x = new Map( [ [ 4, 'hifdsa' ], [ 44, 'hif' ], [ 4432, 'hi' ], [ 4123, 'hi' ], [ 4432, 'hi' ], [ 46888, 'hi' ] ] );
+  modelScreen.xProperty = new Property( x, {
+    tandem: Tandem.GENERAL_VIEW.createTandem( 'mapProperty' ),
+    phetioType: Property.PropertyIO( MapIO( NumberIO, StringIO ) )
+  } );
   new Sim( gravityAndOrbitsTitleString, [ modelScreen, toScaleScreen ], simOptions ).start();
 } );
\ No newline at end of file

It showed me this state in the state wrapper. . . .

 "gravityAndOrbits.general.view.mapProperty": {
        "value": [
            [
                4,
                "hifdsa"
            ],
            [
                44,
                "hif"
            ],
            [
                4432,
                "hi"
            ],
            [
                4123,
                "hi"
            ],
            [
                46888,
                "hi"
            ]
        ],
        "validValues": null,
        "units": null
    },

Then in the downstream sim I see that it is getting reconstituted very nicely too (from the console):

phet.phetio.phetioEngine.phetioObjectMap['gravityAndOrbits.general.view.mapProperty'].value
>Map(5) {4 => "hifdsa", 44 => "hif", 4432 => "hi", 4123 => "hi", 46888 => "hi"}

I'm ready to close this issue, with or without a checked in usage. Good work you two!

[zepumph]

@jbphet and I just spent some time on MapIO, and I realized that these two isValidValue functions should not be the same. One validates the instance, and the other validates the stateObject (object literal). Commit coming soon!

[zepumph]

We were blocking the pairing session on greenhouse that @jbphet and I were having, so I did a quick fix with a couple of TODOs. I'll add myself as an assignee, and remove review. Feel free to take this on if you want though (no need to if you don't).

[zepumph]

I removed a TODO which was caused by a gotcha I put in ValidatorDef about having valueType + arrayElementType together. I updated it to make things more graceful, but still supportive with good error messages. I also updated some tests to be clear.

As for MapIO. . . this has been in Greenhouse effect for over a month now. Here is an example of a usage's toStateObject output for

modelObjectToAttenuatorMap: MapIO( ReferenceIO( IOType.ObjectIO ), WaveAttenuator.WaveAttenuatorIO ):

   "modelObjectToAttenuatorMap": [
            [
                {
                    "phetioID": "greenhouseEffect.wavesScreen.model.atmosphereLayers.layer3"
                },
                {
                    "attenuation": 0.7614250158858502,
                    "distanceFromStart": 17140.253227753477
                }
            ]
        ],

I feel like it is running as expected, and I can't see any more improvements. @samreid, please feel free to close.

[samreid]

That looks reasonable to me, closing.

[samreid]

As discovered in phetsims/chipper#946, there are TODOs in the code referring to this issue. Reopening.