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

Tolerance on bump-away vs connect #171

Closed kathy-phet closed 4 years ago

kathy-phet commented 4 years ago

@arouinfar - When I was playing with the posted prototype version, it seemed like the tolerance on creating a bond was pretty small. I had to be more precise about placement then I expected. It seems like the usability would be improved by lowering the tolerance, so if you are a bit overlapping an atom and drop say a H, it will attach at the nearby open spot if it is an allowed attachment spot.

Can you take a look at this and see what you think?

arouinfar commented 4 years ago

@kathy-phet there was a lot of back-and-forth with this in #154. Initially the threshold distance for atoms to bond was so large that atoms cut off of a molecule would automatically reconnect to the molecule when clicked.

I think the underlying issue here is that atoms will repel each other if the approach is diagonal. This behavior is different from Java. javaDiagonal

@Denz1994 can you take a look?

kathy-phet commented 4 years ago

Yes, it is related to this, but also to the fact that it doesn't "jump to the nearest allowable position" when it is actually overlapping the atom you want to add it too.

This is what the current HTML behavior is: BAM_html_atomsdoesnotstick

This is what the old Java sim did: BAM_Java_atomsticks

It makes it very usable for it to jump to the nearest open position that will work when it is overlapping the atom you are trying to add it too.

kathy-phet commented 4 years ago

@Denz1994 - Can you take a look at this issue? It would really help the usability of the sim to address this in the short term if possible. Perhaps we can have a shortish meeting tomorrow during design time (@arouinfar)?

Denz1994 commented 4 years ago

Kit.bondDistanceThreshold determines how far from the bonding position an atom has to be before bonding. Along with this, Kit.attemptToBondMolecule() would need to proceed with bonding if atomA.destinationBounds.intersectsBounds( atomB.destinationBounds ) === true. After checking if the atoms are within the bonding threshold we can check if their bounds are overlapping.

The best location is already determined by BondOption so I think an additional conditional is the only thing required here.

kathy-phet commented 4 years ago

@Denz1994 - I think it might be preferable UI to first check if atoms are overlapping and if so "always bond to the closest bond", and only use Distance Threshold if the atoms are not overlapping. That way DistanceThreshold only applies for atoms that are physically separated. ... Perhaps this is the behavior that was added, and if so, great.

Denz1994 commented 4 years ago

This dev version will bond overlapping atoms to the atom's nearest bonding option (North, South, East, or West).

(Re diagonal bonding) Because we are now considering the bounds of the full atom, the threshold for diagonal bonding is a bit more lenient in the diagonal directions. This is because the bounds are essentially a rectangle around the circular atom, so a diagonal approach for bonding will intersect bonds at the edge of the bounding rectangle.

@arouinfar Could you please review the above dev version and make sure this doesn't conflict with atoms being bonded too easily as mentioned here https://github.com/phetsims/build-a-molecule/issues/154#issuecomment-588516817.

arouinfar commented 4 years ago

Thanks @Denz1994, I was just reading through this issue, and I thought the proposals all sounded good. Excited to test it out!

kathy-phet commented 4 years ago

@Denz1994 - Amy and I reviewed this latest version. She will post a request for a slight improvement - but this version is working a lot better re this UI interaction.

@mattpen - Please post this prototype version over the current one on our website. https://phet-dev.colorado.edu/html/build-a-molecule/1.0.0-dev.67/phet/build-a-molecule_en_phet.html

mattpen commented 4 years ago

https://phet.colorado.edu/sims/html/build-a-molecule/latest/build-a-molecule_en.html is now available.

image

Denz1994 commented 4 years ago

@arouinfar Mentioned via slack, that she will write up a slight improvement. I'll remove the high-priority label because this is currently available as mentioned above.

Denz1994 commented 4 years ago

While meeting with @arouinfar, she mentioned that sometimes the molecules bump away when they are diagonally near another molecule. Molecules are bumped away when they are not within the range to bond (kit.bondDistanceThreshold) but are still overlapping the padded molecule bounds (kit.interMoleculePadding). Because the java version had this same behavior, we decided to only slightly increase the separation distance and bonding threshold so the separation looks more intentional.

This separation also handles cases like the image below. Here the H2 separates further than expected from the larger molecule, so the molecule buttons and name don't overlap each other. The topmost blue-button belongs to the larger molecule. image

This dev version has these changes @kathy-phet and @arouinfar.

arouinfar commented 4 years ago

@Denz1994 I think the bonding behavior is looking pretty good, but @kathy-phet should also take it for a spin.

Denz1994 commented 4 years ago

Latest on this issue: @kathy-phet just needs to look at over the latest dev version to sign off on the bonding distance and separation distance.

Raising priority on this issue because we are getting close to an RC.

ariel-phet commented 4 years ago

I think I feel comfortable "signing off" for @kathy-phet in this case. The behavior feels very natural and seamless to me currently in dev.79

Closing.

kathy-phet commented 4 years ago

Just for the record, it does feel good to me too!