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

Overlapping hydrogen or others #190

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/506. I noticed that, when atoms would need to be in the same spot, though linked to different atoms, the sim does not allow this. However, with hydrogen atoms it is allowed and this causes an overlap of the atoms. This can also occur to an extent with very large atoms that are placed next to each other. This may likely be the best solution. If so, please close.

Visuals butcanoverlap overlappingP

Troubleshooting information:

!!!!! DO NOT EDIT !!!!! Name: ‪Build a Molecule‬ URL: https://phet-dev.colorado.edu/html/build-a-molecule/1.0.0-dev.75/phet/build-a-molecule_all_phet.html Version: 1.0.0-dev.75 2020-06-01 13:20:12 UTC Features missing: touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/83.0.4103.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

I would vote to pass on fixing this one. There would most likely be an intrusive patch to the model that determines bonding locations based on molecules that aren't neighbors to an atom. For instance, the bottom-most Phosphorus isn't aware of the position/bounds of the uppermost Phosphorus. The bottom-most Phosphorus only knows about its neighbor, bottom-most carbon.

Also note, this is present in the java version and I don't think it was reported as an issue. image

I'll leave to @arouinfar to decide. Raising priority because this will require model changes.

arouinfar commented 4 years ago

Thanks for reporting @KatieWoe. I agree with @Denz1994's decision to leave this as-is.