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

cannot drag and drop onto the third layer of the particleNucleus #157

Closed zepumph closed 1 year ago

zepumph commented 1 year ago

Most likely from changes to https://github.com/phetsims/build-a-nucleus/blob/a17c5f304303539f78f64ad1a4c273638a9d8811/js/common/BANConstants.ts#L91-L95

The bounding box is just a bit too low, you can add to the nucleus quite below the bottom (first) layer of 2.

zepumph commented 1 year ago

Taking a look now.

zepumph commented 1 year ago

Using this patch I was able to draw the rectangle for the MVT bounds, but this doesn't seem correlated to how the Particle drop is computed into "in" or "out" of the NucleonShellView. I will need @Luisav1 to help investigate.

(at the end of the NucleonShellView constructor):


// have one less space than there are particles
    const NUMBER_OF_RADII_SPACES_BETWEEN_PARTICLES = SECOND_LEVEL_CAPACITY - 1;
    const PARTICLE_RADIUS = ShredConstants.NUCLEON_RADIUS;
    const PARTICLE_DIAMETER = PARTICLE_RADIUS * 2;
    const PARTICLE_X_SPACING = PARTICLE_RADIUS;
    const NUMBER_OF_ENERGY_LEVELS = 3;
    const NUMBER_OF_Y_SPACINGS = NUMBER_OF_ENERGY_LEVELS - 1;
    const PARTICLE_Y_SPACING = PARTICLE_DIAMETER * 4;

    console.log( 'hi' );
    // this.addChild( Rectangle.bounds( new Bounds2( 0, 0, ( PARTICLE_DIAMETER + PARTICLE_X_SPACING ) * NUMBER_OF_RADII_SPACES_BETWEEN_PARTICLES,
    //   ( PARTICLE_DIAMETER + PARTICLE_Y_SPACING ) * NUMBER_OF_Y_SPACINGS ), { stroke: 'red' } ) );
    this.addChild( Rectangle.bounds( this.modelViewTransform.modelToViewBounds(
      new Bounds2( 0, 0, NUMBER_OF_RADII_SPACES_BETWEEN_PARTICLES, 2 ) ), { stroke: 'red' } ) );
zepumph commented 1 year ago

@Luisav1 helped greatly with the above commits. Not only did we fix the bug, but we went further in basing all the model and view logic on the atomViewPosition option, instead of sprinkling in offsets and position changes at each level. The main change there was in positioning the NucleonShellView nodes with setTranslation instead of set leftTop(). @Luisav1 should take one more look at the commits though since I was driving, and certainly not the expert here.

Luisav1 commented 1 year ago

These changes are looking great and the bug now fixed. Closing.