Open samreid opened 4 years ago
Great idea! I mocked this up for the first screen and it read like so:
"data": {
"phetioID": "balancingAct.introScreen.model.smallTrashCan",
"mass": 10,
"distance": 0.5,
"fullState": [
{
"name": "balancingAct.introScreen.model.fireExtinguisher1",
"mass": 5,
"distance": 1.25
},
{
"name": "balancingAct.introScreen.model.smallTrashCan",
"mass": 10,
"distance": 0.5
}
]
}
However, I realized to get this working for the creator pattern on the 2nd screen, we would likely need to introduce PhetioGroup for the elements to set their names. That will be a good long term plan but I don't know if I have enough time to implement that by tomorrow.
Here's the patch:
@samreid - Tomorrow will definitely not include data collection, so I don't think this is a "by tomorrow" sort of question. Logging is a week away - at a minimum - and could be more depending on how the testing of the logging goes this week.
I tried out @samreid's suggestion of implementing PhetioGroups for the brick stacks and mystery masses.
It looks nice in the data stream:
"data": {
"phetioID": "balancingAct.balanceLabScreen.model.mysteryMassGroup.mysteryMass_0",
"mass": 15,
"distance": -0.5,
"fullState": [
{
"name": "balancingAct.balanceLabScreen.model.brickStackGroup.brickStack_0",
"mass": 5,
"distance": 0.5
},
{
"name": "balancingAct.balanceLabScreen.model.mysteryMassGroup.mysteryMass_0",
"mass": 15,
"distance": -0.5
}
]
}
However, it was difficult and not pretty to get this working. Most of the trouble I ran into was because I was trying to make the conversion for just these two types (many more "mass" types are used in the sim). I think it will be a project to implement this generally for the whole sim.
Working patch (including @samreid's changes from above):
@chrisklus your changes look great. I made a few improvements in this patch.
I think this is the right direction to move in, and I'm ready to commit, but I wanted to hear from @kathy-phet whether this initial version should have object IDs.
We reviewed the current status with @kathy-phet, and we discussed the following 3 options:
We decided to go with the (b) because it helps disambiguate which mass the user is using. In the future we will probably pursue (c) but we will need to take time to make sure it's well designed.
When this sim gets fully instrumented we'll proceed with (c) from https://github.com/phetsims/balancing-act/issues/99#issuecomment-579870712, but are going to close for now since this is a limited instrumentation.
There are still TODOs marked for this issue, discovered during https://github.com/phetsims/chipper/issues/946
Unassigning until we return focus to this sim.
Im marking this as deferred since phet-io brand will not be part of the upcoming publication.
For https://github.com/phetsims/balancing-act/issues/96
@kathy-phet said: