phetsims / natural-selection

"Natural Selection" is an educational simulation in HTML5, by PhET Interactive Simulations
GNU General Public License v3.0
3 stars 7 forks source link

Environment radio buttons are re-enabled when you start sim and don't carry over when loaded #296

Closed KatieWoe closed 3 years ago

KatieWoe commented 3 years ago

Device Mac and Dell OS MacOS 11 and Win 10 Browser Firefox and Chrome Problem Description For https://github.com/phetsims/QA/issues/662. Seen with this element, but may happen with other elements as well. Seems similar to https://github.com/phetsims/gravity-and-orbits/issues/394. naturalSelection.introScreen.view.environmentPanel.environmentRadioButtonGroup.enabledProperty When you disable the above element, pressing "Add a Mate" reenables the buttons, both in studio or a launched sim. In addition, if you save a sim with that element disabled, reenable it in studio, then "Load" that saved file, the property will not be disabled. Visuals reenable2 reenable

pixelzoom commented 3 years ago

The relevant code is in EnvironmentPanel.js:

    // Simulation mode determines which UI components are enabled. unlink is not necessary.
    model.simulationModeProperty.link( simulationMode => {
      environmentRadioButtonGroup.enabledProperty.value = ( simulationMode !== SimulationMode.COMPLETED );
    } );

The problem is that the sim controls environmentRadioButtonGroup.enabledProperty. When a simulation has "completed" (bunnies have taken over the world, or all bunnies have died), the environmentRadioButtonGroup.enabledProperty is set to false, so that the user can review results, but not change the environment. Any change made to environmentRadioButtonGroup.enabledProperty via Studio/PhET-iO will be overridden by the sim. And there is no way for the sim to know that Studio/PhET-iO changed this Property. Ether the sim can control a Property, or Studio/PhET-iO can control it, not both.

So the options are:

(1) Set environmentRadioButtonGroup.enabledProperty to read-only, so that it can't be changed by Studio/PhET-iO. Instructional designers will not be able to disabled the buttons, but they can hide them.

(2) Remove the above code, so that the sim never changes environmentRadioButtonGroup.enabledProperty. The student will be able to change the environment when reviewing results, which is not ideal, but not tragic.

If the instructional designer doesn't want the student to change the environment, then hiding the radio buttons seems more appropriate than disabling them. So my recommendation is option (1).

This is a PhET-iO design issue, and the associated QA issue is top priority. So assigning to @amanda-phet with top priority.

amanda-phet commented 3 years ago

I agree with @pixelzoom . Option (1) is my preference.

pixelzoom commented 3 years ago

Option (1) has been implemented, patched in master and 1.4 branches.

Ready for testing in the next 1.4 RC.

KatieWoe commented 3 years ago

Looks ok in rc.2