search

phetsims / build-a-nucleus

"Build a Nucleus" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 5 forks source link

Sim uses default layoutBounds, shred uses non-default layoutBounds. #81

Closed pixelzoom closed 1 year ago

pixelzoom commented 1 year ago

This sim uses default layoutBounds for ScreenView. But are you aware that one of its dependencies (shred) required non-default layoutBounds? In ShredConstants.js:

  LAYOUT_BOUNDS: new Bounds2( 0, 0, 768, 464 ),

I don't know whether/how this might be affecting this sim. But other sims (e.g. build-an-atom) that use shred have been forced to use these ShredConstants.LAYOUT_BOUNDS for their ScreenViews.

See also https://github.com/phetsims/shred/issues/37 and https://github.com/phetsims/phet-io/issues/1939.

zepumph commented 1 year ago

Thanks for checking in. BAN is using default layout bounds and has been developed with the default bounds from the beginning. Closing

pixelzoom commented 1 year ago

Right. But in case I wasn't clear... The issue was that BAN and shred are using different layoutBounds. It sounds like that's not causing any problems in BAN. But I can imagine that it will cause problems in some sim, and it would be better if shred were parametrized so that it could use the same layoutBounds as the sim that is using shred.

zepumph commented 1 year ago

I see, yes that makes sense. I am not sure about how BAN is using the shred view components and if there is some hackary in that department to get things to mesh well. We should investigate.

pixelzoom commented 1 year ago

The danger with such hackery (if it exists) is that BAN is likely to break if/when https://github.com/phetsims/shred/issues/37 is addressed.

zepumph commented 1 year ago

After doing some investigation. ShredConstants.LAYOUT_BOUNDS isn't used by BAN at all. This is used as the layout bounds in other sims that use shred, but wasn't done here. Changing ShredConstants.LAYOUT_BOUNDS to the default value didn't change or effect the snapshot comparison of BAN at all. Basically, when it comes time to change the shred constant, it will effect how the screen views that use this (BAA and IAAM) convert to use default bounds.

It is possible that as part of that work, those sim devs will want to change details of shred components (change layout of shred nodes) to make their lives easier, but since BAN is now in the mix, the solution is going to be much easier to just handle things on their sim side instead. I'm now ready to close. Thanks for the ping.