Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PhET-iO State: support UserMovableModelElement.supportingSurface #424

Open
zepumph opened this issue Sep 12, 2023 · 3 comments
Open

PhET-iO State: support UserMovableModelElement.supportingSurface #424

zepumph opened this issue Sep 12, 2023 · 3 comments

Comments

@zepumph
Copy link
Member

zepumph commented Sep 12, 2023

From #423, HorizontalSurface likely needs to support serialization in some form. We were able to bypass a CT error over there with deep comprison of Vector2s, but we will want supportingSurface to maintain the right pointer through PhET-iO state, even if a lack of this doesn't currently cause behavioral or assertion problems. I am not planning to work on this right now. Marking deferred until next iO release of this sim.

@jbphet
Copy link
Contributor

jbphet commented Sep 13, 2023

There is a new CT error that is now popping up occasionally. The trace is below. It seems like a distinct possibility that the cause of this is that the supporting surface isn't being cleared, so things can essentially get recursively stacked on top of one another.

energy-forms-and-changes : phet-io-state-fuzz : unbuilt
http://128.138.93.172/continuous-testing/ct-snapshots/1694583921792/phet-io-wrappers/state/?sim=energy-forms-and-changes&phetioDebug=true&phetioWrapperDebug=true&fuzz&wrapperContinuousTest=%7B%22test%22%3A%5B%22energy-forms-and-changes%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1694583921792%22%2C%22timestamp%22%3A1694591327260%7D
Uncaught RangeError: Maximum call stack size exceeded
RangeError: Maximum call stack size exceeded
at (UserMovableModelElement.js:1:0)
at (UserMovableModelElement.js:1:0)
at (UserMovableModelElement.js:1:0)
at (UserMovableModelElement.js:1:0)
at (UserMovableModelElement.js:1:0)
at (UserMovableModelElement.js:1:0)
at (UserMovableModelElement.js:1:0)
at (UserMovableModelElement.js:1:0)
at (UserMovableModelElement.js:1:0)
at (UserMovableModelElement.js:1:0)
[URL] http://128.138.93.172/continuous-testing/aqua/html/wrapper-test.html?url=..%2F..%2Fct-snapshots%2F1694583921792%2Fphet-io-wrappers%2Fstate%2F%3Fsim%3Denergy-forms-and-changes%26phetioDebug%3Dtrue%26phetioWrapperDebug%3Dtrue%26fuzz&testInfo=%7B%22test%22%3A%5B%22energy-forms-and-changes%22%2C%22phet-io-state-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1694583921792%22%2C%22timestamp%22%3A1694591327260%7D

@zepumph
Copy link
Member Author

zepumph commented Oct 24, 2023

@jbphet and I worked on this today and got to a commit point for instrumenting HorizontalSurface and supportingSurface, but it didn't solve all the state issues, so we removed the sim from PhET-iO state testing.

When we get to this next time, we will want to instrument RectangularThermalMovableModelElement, which holds some bounds entities that update as movables translation around. This is likely some of the cause of the state issues.

Also noting though that all actual assertions we hit were because the supportSurface was out of sync, so an audit on the above commit will be worthwhile too.

At one point this was helpful debug code:

   // Debug code.  To use, set phet.preloads.phetio.jb to either 'brick' or 'iron' from the console.
    if ( phet.preloads.phetio.queryParameters.frameTitle === 'destination' ) {
      if ( window.parent.phet.jb === undefined ) {
        window.parent.phet.jb = '';
      }
      if ( ( window.parent.phet.jb === 'brick' && this.blockType.toString() === 'BRICK' ) ||
           ( window.parent.phet.jb === 'iron' && this.blockType.toString() === 'IRON' ) ) {
        console.log( '-----------------------' );
        console.log( `this.id = ${this.id}` );
        console.log( `this.blockType = ${this.blockType}` );
        const elementOnTopSurface = this.topSurface.elementOnSurfaceProperty.value;
        if ( elementOnTopSurface ) {
          console.log( `elementOnTopSurface.id = ${elementOnTopSurface.id}` );
          console.log( `elementOnTopSurface.blockType = ${elementOnTopSurface.blockType}` );
        }
        else {
          console.log( 'Nothing on top surface.' );
        }
        window.parent.phet.jb = '';
      }
    }

@zepumph zepumph removed their assignment Oct 24, 2023
@jbphet
Copy link
Contributor

jbphet commented Oct 24, 2023

This is being marked as deferred and we will give it more attention when we decide to upgrade this sim to the Hydrogen level of phet-io.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants