phetsims / color-vision

"Color Vision" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/color-vision
GNU General Public License v3.0
1 stars 7 forks source link

view of the head needs to be better encapsulated #46

Closed pixelzoom closed 10 years ago

pixelzoom commented 10 years ago

RGBScreenView creates 4 nodes for the head:

51 var headBackNode = new HeadNode( headBack, this.layoutBounds.bottom ); 54 var headBackNoBrainNode = new HeadNode( headNoBrain, this.layoutBounds.bottom ); 86 var headFrontNode = new HeadNode( headFront, this.layoutBounds.bottom ); 89 var headFrontNoBrainNode = new HeadNode( headFrontNoBrain, this.layoutBounds.bottom );

And then it has to manage those nodes:

93 model.headModeProperty.link( function( mode ) { headBackNode.visible = ( mode === 'brain' ); headBackNoBrainNode.visible = ( mode === 'no-brain' ); headFrontNode.visible = ( mode === 'brain' ); headFrontNoBrainNode.visible = ( mode === 'no-brain' ); } );

This is complicated enough that there should be 1 view component that is responsible for creating the head and keeping it synchronized with the model. Recommended to move all of this into one node (HeadNode?) that can be used in both screens. Then creation in RGBScreenView would look like:

var headNode = new HeadNode( model.headModeProperty, ... );

pixelzoom commented 10 years ago

These images would also move into HeadNode:

var headBack = require( 'image!COLOR_VISION/head-with-brain.png' ); var headFront = require( 'image!COLOR_VISION/head-front-with-brain.png' ); var headNoBrain = require( 'image!COLOR_VISION/head-no-brain.png' ); var headFrontNoBrain = require( 'image!COLOR_VISION/head-front-no-brain.png' );

aaronsamuel137 commented 10 years ago

I have encapsulated the headNodes in SingleBulbHeadNode and RGBHeadNode. See #59 for reasons why they need to be different. Assigning to @pixelzoom for review

aaronsamuel137 commented 10 years ago

I have since consolidated both head node files into HeadNode.js. See #59

pixelzoom commented 10 years ago

Looks good, closing.