search

phetsims / density-buoyancy-common

Common code for the Density and Buoyancy simulations
GNU General Public License v3.0
0 stars 2 forks source link

Min and Max mass listed don't take material into consideration #407

Closed KatieWoe closed 1 month ago

KatieWoe commented 1 month ago

Device Samsung OS Win 11 Browser Chrome Problem Description For https://github.com/phetsims/qa/issues/1141 In studio, the mass ranges given for blocks may not take into account the material the block is currently made of. For example, the range for buoyancy.exploreScreen.view.blockAPanel.massNumberControl.massProperty is .1-27, but depending on the material is usually smaller than that.

Visuals setvaluegivenwrong

KatieWoe commented 1 month ago

Actually, this looks like it's already addressed in the examples doc, so it can probably be closed.

Nancy-Salpepi commented 1 month ago

I asked about this same thing in the Buoyancy slack channel! Here is the response:

Sam Reid My understanding is that the range shown there (at the bottom) is the total composite range for any material. It is static and unchanging, whereas the enabledRange is able to change. But studio (at the bottom) is supposed to show the static range, not the dynamic enabledRange.

Kathy Perkins @arouinfar should weigh in on this question as well.

Amy Rouinfar

Yes, that behavior is fine. The examples clarifies that resultant volume needs to be 1-10L and recommends clients first refer to enabledRangeProperty before setting mass.

Nancy Salpepi

Wondering if that statement in the examples should come before the massProperty phetio IDs or do you think it is OK as is @arouinfar?

KatieWoe commented 1 month ago

I noticed this discrepancy with the mystery fluids, which doesn't make sense to me. fluidcbounds

Nancy-Salpepi commented 1 month ago

@KatieWoe your last comment is part of issue #405

KatieWoe commented 1 month ago

Ahh, thanks

zepumph commented 1 month ago

Ok great. Yes I believe all the actionables are in https://github.com/phetsims/density-buoyancy-common/issues/405 and examples.md. Closing.

zepumph commented 1 month ago

Oh, we need to update examples. @arouinfar can you add this sentiment from the above comment:

Wondering if that statement in the examples should come before the massProperty phetio IDs or do you think it is OK as is @arouinfar?

arouinfar commented 1 month ago

In the above commits, I made minor wording changes and combined the phetioIDs into a single list so folks encounter massProperty and enabledRangeProperty together.

Since these are the same commits as the changes in #400, we can close this issue and handle the cherry-picks over there.