phetsims / vegas

Reusable game components for PhET simulations.
MIT License
1 stars 4 forks source link

RewardNode seems broken in 0.2 #39

Closed samreid closed 9 years ago

samreid commented 9 years ago

I only saw 1-2 smiling faces go by. It looks like the bounds are too wide. From skype:

@samreid: FractionMatcher needs to get the bounds of the screen to set the size of a reward node canvas. Was using:
this.getUniqueTrail().nodes[ 0 ].bounds

What should I use in 0.2? @jonathanolson: Same thing will work in 0.2 @samreid: [1/28/15, 12:07:38 PM] Sam Reid: I tried it and the bounds are very wide:
Bounds2 {minX: -2000, minY: -2000, maxX: 4000, maxY: 3999.9999999999995, constructor: function…} [1/28/15, 12:07:45 PM] Sam Reid: I only saw one smiling face go by :(

pixelzoom commented 9 years ago

It's also broken in RPAL. Run with ?dev&showReward and you'll see the reward regardless of score. When it gets to where it should display the reward, the final challenge remains displayed. And it looks like it's entering an endless loop, with this error message in the console:

Uncaught TypeError: Cannot read property 'transformed' of undefined Node.js?1422472276879:2096

pixelzoom commented 9 years ago

This is apparently due to a breaking API change in scenery 0.2. RewardNode uses getScene and scene.sceneBounds, but Scene no longer exists in scenery 0.2.

pixelzoom commented 9 years ago

Sims that are broken: balancing-chemical-equations build-an-atom fraction-matcher graphing-lines reactants-products-and-leftovers

pixelzoom commented 9 years ago

After a bunch of Skype discussion with @jonathanolson and @samreid, here's the plan:

Scenery changes:

getTrails( [predicate] ), getUniqueTrail( [predicate] ), brackets indicating optional getTrails returns {[Trail]} getUniqueTrail will return a single {Trail}, or fail

RewardNode fix:

var trail = node.getUniqueTrailTo( function( node ) { return node instanceof ScreenView; } ); var screenView = trail.rootNode(); var localBounds = trail.getTransform().inverseBounds2( screenView.layoutBounds );

pixelzoom commented 9 years ago

This is still broken, see https://github.com/phetsims/reactants-products-and-leftovers/issues/33. Reopening and assigning to @jonathanolson. High priority.

pixelzoom commented 9 years ago

Fraction Matcher is also still broken, with the same console error as RPAL:

Uncaught TypeError: Cannot read property 'transformed' of undefined Node.js?1422472276879:2096

I'm guessing that all sims that use RewardNode are similarly broken.

pixelzoom commented 9 years ago

The following sims are broken:

balancing-chemical-equations fraction-matcher graphing-lines reactants-products-and-leftovers

jonathanolson commented 9 years ago

It's likely I broke it again recently, looking into it. Sorry!

jonathanolson commented 9 years ago

Looks like it was an unchanged window.simDisplay reference that caused the issue (change now to phet.joist.sim.display). Reassigning for testing/review.

pixelzoom commented 9 years ago

Appears to be fixed, thanks! Closing.