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

Going back to previously completed collections do not display said different collection #121

Closed loganbraywork closed 4 years ago

loganbraywork commented 4 years ago

Test device Windows 10 Laptop Operating System Windows 10 Browser Chrome Problem description From https://github.com/phetsims/QA/issues/459

After a molecule collection is completed, it is possible to navigate back to said completed collection using the arrow button near the top of the collection box. Navigating back to previous collections does not alter the box in any way and the box remains on the current collection while the collection number changes. Steps to reproduce

  1. Navigate to Multiple screen
  2. Complete the first collection
  3. Attempt to Navigate back to the first collection from the second collection using the yellow arrow at the top of the collection box. Visuals 2019-11-22ColctnBldMlcle

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: generatedcontent, 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: 1536x754 Pixel Ratio: 1.25/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: 4095 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 32767x32767 OES_texture_float: true Dependencies JSON: {}

loganbraywork commented 4 years ago

If a molecule in the second collection is completed before moving back to the first then back to the second, the atoms used to complete said model will return and can be manipulated.

KatieWoe commented 4 years ago

Example of https://github.com/phetsims/build-a-molecule/issues/121#issuecomment-557682447 as well as graphical issues that occur as a result. example

loganbraywork commented 4 years ago

If an atom is released in the basket while on a previously completed collection screen the sim crashes. The console error is:

build-a-molecule_all_phet.html:1068 Uncaught TypeError: Cannot read property '_parents' of undefined
    at C.removeChildWithIndex (build-a-molecule_all_phet.html:1068)
    at C.removeChild (build-a-molecule_all_phet.html:1068)
    at t.value (build-a-molecule_all_phet.html:1068)
    at Array.<anonymous> (build-a-molecule_all_phet.html:1068)
    at C._fireItemRemoved (build-a-molecule_all_phet.html:1068)
    at C.remove (build-a-molecule_all_phet.html:1068)
    at C.recycleAtomIntoBuckets (build-a-molecule_all_phet.html:1068)
    at build-a-molecule_all_phet.html:1068
    at Array.forEach (<anonymous>)
    at C.recycleMoleculeIntoBuckets (build-a-molecule_all_phet.html:1068)
Denz1994 commented 4 years ago

Commit dc895aa57224af1e128248abfe662262fae237e5 might have fixed this. Can you confirm this is fixed in this dev version @KatieWoe ?

Note: You can use the query parameter ?easyMode. It will allow you to go to the next collection after one completed molecule. You will need to refresh the sim if you don't go to next collection though.

KatieWoe commented 4 years ago

The issue and https://github.com/phetsims/build-a-molecule/issues/121#issuecomment-557689146 both seem fixed. https://github.com/phetsims/build-a-molecule/issues/121#issuecomment-557682447 still occurs and seems odd, but it does not cause issues anymore that I can see.

Denz1994 commented 4 years ago

@arouinfar What do you think about this behavior in https://github.com/phetsims/build-a-molecule/issues/121#issuecomment-557682447? Do we want to keep the atoms in the play area as the user swaps between collections?

I personally vote to reset the play area an return the atoms to their buckets when the user swaps collections (the current behavior). I think of the collections as different scenes on a screen and refer back to how the masses are treated in MAS and MASB. Molecule and Shapes also reset molecules when swapping between molecules on the Real Molecules screen.

arouinfar commented 4 years ago

Do we want to keep the atoms in the play area as the user swaps between collections?

In Java the atoms remain in the play area when scrolling through the collections. It would be nice if we could keep that behavior, as it seems more natural.

That said, if it would take significant time investment or would make the code less robust/maintainable to do, I would be fine with resetting the play area when moving between collections.

Denz1994 commented 4 years ago

The current approach was partly done because we don't have a limit on the number of possible collections for the collection screens.

This wouldn't be a straight-forward change because we are disposing of the listeners when the collections are being changed. We would need to track the listeners for each kit and reassign them, which is possible.

This will increase the memory profile as more collections are completed and then added, but I would need to do a headshot comparison to determine by how much.

Denz1994 commented 4 years ago

During an impromptu meeting, @arouinfar and I thought that we should leave the behavior the way it is. Aside from the complexity described above (https://github.com/phetsims/build-a-molecule/issues/121#issuecomment-572631705), @arouinfar mentioned that limiting the number of collections would require a separate UI for completing the last collection.

Denz1994 commented 4 years ago

From meeting on 02/07/20 @ariel-phet mentioned:

This seems fine to me.

We'll proceed with the current behavior, closing this issue.