Number display incorrect when using array to set balls

[Nancy-Salpepi]

Test device
MacBook Air M1 chip

Operating System
14.5

Browser
Safari 17.5

Problem description
For phetsims/qa#1121, on the Balance Point screen only, the number display doesn't change to show the number of balls on the field when using the array method to add balls.

Steps to reproduce

1. In Studio, go to the Balance Point Screen
2. In the console paste: await phetioClient.invokeAsync( 'meanShareAndBalance.balancePointScreen.model', 'setDataPoints', [ [4, 9, 10, 1, 10, 6,] ] );

Visuals

[marlitas]

Addressed above. I would like to talk with over with @jbphet though.

[jbphet]

This change is a little tricky for me to review. I can see what you're after (I think) in BalancePointSceneModel.setDataPoints with the lines:

    this.targetNumberOfBallsProperty.value = Math.min( dataPoints.length, this.maxKicksProperty.value );
    this.numberOfQueuedKicksProperty.value = 0;

Reading through other parts of the code, I can see that changing targetNumberOfBallsProperty could queue up some balls for kicking, thus changing the value of numberOfQueuedKicksProperty, so you then need to prevent that from actually happening by setting numberOfQueuedKicksProperty to 0. It sure feels fragile though, since it relies on an understanding of how these two Properties interact within the context of the model. I looked at the handler for changes to targetNumberOfBallsProperty, and I think this is safe, but I didn't scan through the code for listeners to numberOfQueuedKicksProperty that could potentially be messed up by all of this.

Since this is a phet-io-specific feature, I think it's probably okay, but I'd suggest elaborating the existing comment a bit to describe a bit more fully what's going on.

[marlitas]

Committed more documentation. I believe this is ready for cherry-pick. @jbphet let's connect synchronously to make sure we are indeed good to go on this. I agree that it feels fragile... it was not my favorite overall. I would feel better if the method could be private or protected, but that does not appear to be possible either.

[marlitas]

Met with @jbphet and we both agreed it was a necessary evil. @pixelzoom helped us by pointing out that moving the override code to the BalancePointModel IOType would increase the safety of the code. We feel much better now.

[Nancy-Salpepi]

Unable to verify this issue is fixed due to #345

[jbphet]

#345 is now, I believe, fixed. I verified that this issue (i.e. #335) is fixed on main, and it's lookin' good, so I think this will be ready to verify in the next RC.

[KatieWoe]

Looks good in rc.3