search

phetsims / build-a-molecule

"Build a Molecule" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
8 stars 7 forks source link

Atoms float in the bucket #112

Closed KatieWoe closed 4 years ago

KatieWoe commented 4 years ago

Test device Dell Operating System Win 10 Browser Chrome Problem description For https://github.com/phetsims/QA/issues/459 When atoms in a bucket are taken from the bottom layer the atoms above it do not fall, leaving them floating in mid air. Steps to reproduce

  1. Go to the last screen (occurs on any, but this is easier)
  2. Pick a bucket with two layers
  3. Remove the bottom layer of atoms and put them in the play area.

Visuals FloatH

Troubleshooting information:

!!!!! DO NOT EDIT !!!!! Name: ‪Build a Molecule‬ URL: https://phet-dev.colorado.edu/html/build-a-molecule/0.0.0-dev.23/phet/build-a-molecule_all_phet.html Version: 0.0.0-dev.23 2019-11-20 23:29:39 UTC Features missing: touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.97 Safari/537.36 Language: en-US Window: 1536x722 Pixel Ratio: 2.5/1 WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium) GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 4096 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 32767x32767 OES_texture_float: true Dependencies JSON: {}
Denz1994 commented 4 years ago

SphereBucket.removeParticle(particle, skipLayout) indicates that its second argument should determine whether the layout is handled automatically. Currently, this is set to false and layout is handled by BAM code.

This should be refactored, but it requires the atom.destinationProperty be updated accordingly.

Denz1994 commented 4 years ago

This can be fixed by adjusting the skipLayout parameter from above. Also, the Atom2.isSeparatingProperty can be removed because it was restricting stepping the Atom to update the position property based on its destination property.

@KatieWoe Can you confirm that the atoms don't float in the buckets anymore? Also, the remaining atoms should fill the space that a removed/dragged atom was occupying. This dev version can be checked.

KatieWoe commented 4 years ago

The floating issue seems fixed. I did notice some other odd behavior of the remaining atoms in bucket though. Sometimes one will jump to a higher position, move to the side, rather than drop down, and become unable to be stacked back up. randomshift shiftovernodrag shiftup

Denz1994 commented 4 years ago

Note to self: Bucket.addParticleNearestOpen() may need to be used where the restoring bucket state isn't important. Bucket.placeAtom() should distinguish this using an argument.

Denz1994 commented 4 years ago

Adjusting Bucket.placeAtom() worked well for handling the odd behavior described above. @KatieWoe can you verify in this dev version?

KatieWoe commented 4 years ago

Looks much better now. You can't really restack the atoms, like in the second gif, but you can move them around now, so I think this is probably satisfactory.

Denz1994 commented 4 years ago

In the java version, a user could stack the atoms until the exited the carousel and spilled into the play area. I don't think we want this behavior but I'll let @arouinfar decide and close. @arouinfar Could you review and close if this looks good?

Here is the Java version: image

Denz1994 commented 4 years ago

@arouinfar and I went over this and the stacking behavior mentioned above is something we don't want. As it stands we can't intentionally stack atoms anymore. Closing this issue.