Able to move balls with alt input when enabledProperty=false

[Nancy-Salpepi]

Test device
MacBook Air M1 chip

Operating System
14.5

Browser
Chrome

Problem description
For phetsims/qa#1121, in Studio on the Balance Point screen, setting the enabledProperty to false for the soccer balls still allows them to be moved with alt input.

Steps to reproduce

1. In studio set the enabledProperty for all the soccer balls to false.
2. Kick the balls
3. Tab to the balls, grab a ball, and use arrow keys move it

Visuals

soccerBallsInteractive.mp4

[marlitas]

Hmmm there should be no cueing indicators either. Oh I see you just opened an issue for that as well.

[Nancy-Salpepi]

I didn't realize there was this property: soccerBallsEnabledProperty
That property works correctly.

The bug I saw was when I set the enabledProperty of each ball to false.

[marlitas]

I fixed the alt-input situation with this, but there still are some oddities I'm not quite sure how we want to handle. For example, if I disable input for all of the top soccer balls, but some of the bottom ones are still enabled I still get the mouse cue.

The visibility of the mouse cue right now is only tied to wether the soccerBallsEnabledProperty is true/false, not the individual enabledProperties. Personally, this feels documentation worthy in examples.md. I can toggle the visibility of the mouse cue based on whether all top balls are enabled or not, but it feels unnecessary to complicate the code when we already have a wonderful Property to help clients with these. I don't think it's worth it to provide two ways of doing the same thing.

Thoughts?

Also important to note, the studio tree has now changed a bit since I needed to access the enabledProperty in the model. You can now toggle the enabledProperty for each soccer ball either under model.sceneModel.soccerBalls.soccerBall*.enabledProperty or as a linked Property in view.sceneView.soccerBallNodes.soccerBallNode*.enabledProperty

Over to @amanda-phet to weigh in on the above question and review the final behavior.

[amanda-phet]

It's a little funky... For example, I set meanShareAndBalance.levelOutScreen.model.initialWaterLevelsProperty and the cups didn't come out how I set them, but I clicked reset and Set Value a few times and eventually things worked out. I think if I was more methodical I could figure out the sequence, but my overall question is just if clicking 'Set Value' should immediately set the value of all 7 cups, which I think it should, and that's not happening.

It seems to be working well on the Distribute and Faire Share screens!

[marlitas]

@amanda-phet, this feedback seems related to #334.

I'm going to reassign you to this issue to get more specific feedback on the enabledProperty.

[catherinecarter]

Upon testing CaV, we found two situations, one where the hand appears on the number line with potentially no ball at that tick mark, and one where the hand appears in the upper left.

Situation 1 -- hand appears on number line based on previous situation

1. Kick the max number of balls (at least one needs to be enabled so the hand appears)
2. Hit the orange reset all button
3. Disable each ball individually
4. Kick the max again
5. The cueing hand often appears in strange locations based on some previous situation, shown below:
   

Situation 2 -- hand appears in upper left corner

1. Reload sim so it thinks no balls have ever been kicked
2. Follow only steps from above 3, 4, 5, in order.
   

[amanda-phet]

I'm going to update examples.md to say two things:

1. If you want all balls disabled, don't do them individually. Use meanShareAndBalance.balancePointScreen.model.soccerArea.soccerBallsEnabledProperty
2. If you disable some balls, the cueing arrow may end up in unpredictable locations, so we don't recommend doing that.

[marlitas]

I committed a fix that handles the above scenarios more cleanly than it had been. I think the warnings that @amanda-phet wants to put in examples.md are still warranted, but hopefully we avoid as many of the ultra-buggy scenarios as possible. I think this is ready for cherry-pick for QA to verify and test.

QA: It will be important to check that keyboard input/cues have not regressed with the soccer ball interactions. Especially in conjunction with PhET-iO.

[Nancy-Salpepi]

@amanda-phet @marlitas Since it isn't recommended to set the enabledProperty to false for the individual soccer balls, maybe those elements shouldn't be featured?

[catherinecarter]

@Nancy-Salpepi - being able to set enabledProperty=false for each ball is desirable if an instructional designer want to create a specific situation where certain balls have a fixed value, and others can be moved so the mean/median can change. For example, if I fix the location of two balls to be 3 and 6, and then have two more that are moveable, I could ask the user to move the two enabled balls to create a mean of 4.

I think the undesirable situation is disabling 100% of the balls one by one, and instead, the user should use the soccerBallsEnabledProperty for that screen.

[Nancy-Salpepi]

Ah gotcha! Thanks for explaining CC!

[catherinecarter]

Seems reasonable to note this behavior in examples.md and keep the logic as it is currently (behavior with the cueing hand on disabled balls).

My only real concern is in the situation shown below. Since none of the balls are enabled (which the user shouldn't be doing, per examples.md), the hand appears, but there's no way to remove it since I can't tell the sim that I have recognized the cue and acted on it. Maybe this should added to examples as a consequence of disabling each ball rather than the group? What do you think, @amanda-phet?

[amanda-phet]

Made a slight tweak to examples.md with @catherinecarter that can be used for both MSaB and CaV. I think we are done here!

[Nancy-Salpepi]

Addition to the examples.md looks good.

I do see this one oddity....when disabling some of the kicked balls, if I disable the last ball kicked, in the Standard PhET-iO wrapper, when I grab a ball, the cueing hand/arrows will briefly move to the last ball kicked before disappearing.

cueingArrowJumps.mp4

[KatieWoe]

Found a more general version of #337 (comment)
#350

[marlitas]

I believe #337 (comment) was fixed by the commit in #350

[Nancy-Salpepi]

This looks fixed in rc.4.
Closing.