StateSchema's default toStateObject and applyState should support IOType supertypes

phetsims / tandem

Simulation-side code for PhET-iO

MIT License

0 stars 5 forks source link

StateSchema's default toStateObject and applyState should support IOType supertypes #295

Closed zepumph closed 1 year ago

zepumph commented 1 year ago

These methods support a solid implementation, but they don't cascade up to parent supertypes. It would be ideal that this could occur, so that a StateSchema only needs to provide its own keys, and not manually add in its parents.

zepumph commented 1 year ago

[x] @jbphet and I ran into one other problem with the applyState function where we want to use the defaultApplyState function, but don't want to overwrite the array reference for emEnergyPackets:

https://github.com/phetsims/greenhouse-effect/blob/55b7a4e401dc1765d905676a2a0eafe090a95e57/js/common/model/LayersModel.ts#L349-L352

zepumph commented 1 year ago

https://github.com/phetsims/tandem/issues/295#issuecomment-1568873424 was fixed with a new ReferenceArrayIO. Yay!

https://github.com/phetsims/greenhouse-effect/blob/bad4c13fa9263dcf96f93bc94fca84ef10ac5b82/js/common/model/LayersModel.ts#L350

zepumph commented 1 year ago

Alright. I worked with @samreid on this, and state seems to be working well! @samreid do you think there is anything else here? Sorry for the broad review, but I don't know quite how to write up some of the larger portions of state schema and IOType work that we may want to work on to make things better and easier. Perhaps the key is trying to move towards a StateSchema-centric usage.

samreid commented 1 year ago

@zepumph and I finished this up, nice work! Closing.