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

change model to use 1-based level numbering? #127

Closed
pixelzoom opened this issue Jul 6, 2018 · 2 comments
Closed

change model to use 1-based level numbering? #127

pixelzoom opened this issue Jul 6, 2018 · 2 comments

Comments

@pixelzoom
Copy link
Contributor

In phetsims/vegas#73 and phetsims/vegas#75, the consensus was to make vegas use 1-based level numbering. And that 1-based level numbering in the model may be advantageous for PhET-iO, but should be evaluated on a sim-by-sim basis.

Making the requested change for FiniteStatusBar proved to be complicated. It has option levelProperty, which is typically a model Property. And several sims (including this one) use 0-based level numbering in the model, so that the level number can be used as an index into arrays. I investigated changing the semantics of the model Property to be 1-based, but that turned into a large task, touching dozens of files, potentially error prone.

To avoid changing the model semantics, I added a DerivedProperty that handles the mapping from 0-based to 1-based level numbering. In general, it looks like this:

var statusBar = new FiniteStatusBar( ..., {
  ...
  // FiniteStatusBar uses 1-based level numbering, model is 0-based.
  levelProperty: new DerivedProperty( [ model.levelProperty ], 
    function( level ) { return level + 1; } )
} ); 

At some point, evaluate whether this sim's model should be converted to 1-based level numbering. If so, after converting, the above DerivedProperty can be removed, i.e.:

var statusBar = new FiniteStatusBar( ..., {
  ...
  levelProperty: model.levelProperty
} ); 
pixelzoom added a commit that referenced this issue Jul 6, 2018
@pixelzoom pixelzoom changed the title change model to use 1-based level numbering change model to use 1-based level numbering? Aug 13, 2018
@pixelzoom
Copy link
Contributor Author

The only reason to do this would be for PhET-iO. Deferring until this sim is instrumented.

@pixelzoom
Copy link
Contributor Author

There's no reason to do this, not even for PhET-iO. If 1-based numbering is needed for PhET-iO, then instrument the DerivedProperty that's passed to FiniteStatusBar, it is 1-based. See GamePlayNode line 61:

levelProperty: new DerivedProperty( [ model.levelProperty ], function( level ) { return level + 1; } ),

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

1 participant