phetsims / sun

User-interface components for PhET simulations, built on top of Scenery.
MIT License
4 stars 12 forks source link

Strange order dependency on start/end callbacks and fire() #844

Open AgustinVallejo opened 1 year ago

AgustinVallejo commented 1 year ago

First noticed in https://github.com/phetsims/my-solar-system/issues/171, it seems that for PushButtonModel and ButtonModel, we're calling startCallback > endCallback > fire (which is when the actual number property changes). There was a Slack discussion with @jbphet @pixelzoom and @zepumph which stated potential solutions or workarounds.

@jbphet:

The problem is that the endcallback and the update of the value by the arrow button are both fired when the downProperty in the button model goes to false, so there is essentially an order dependency, and in this case the endCallback is fired before the update of the value. This seems like incorrect behavior. In my mind, the endcallback should only fire after all of the other results of the interaction have occurred. So, first off, do you concur with that assertion? And second, do we have any sort of facility to say "fire this thing last" on a property? I recall that being discussed at one point, but I don't know that we ever did anything with it.

@zepumph:

Sun buttons are not set up for this at all because the input listener is so obfuscated, but in other spaces I have been adding onInputEmitters that fire after the property change in the input listener so that things like sound, voicing etc have a way to occur after the state change (guaranteed). See other usages in sun and scenery phet. I'm out until about 2 but can check back in later if you have questions.

@pixelzoom:

Yes, I agree. And there is a similar order-dependency problem for startCallback. It’s just not manifesting because it’s the first listener added to downProperty and is therefore being called first as expected.

zepumph commented 1 year ago

Yes. In my opinion it is quite sad that sun buttons are so dependent on downProperty as their only tool to use. For sound, @jbphet noted this challenge and created a sound-specific solution for this called produceSoundEmitter. In https://github.com/phetsims/sun/issues/701 (with linked side issues for specific components), I have had really great luck tying some logic to the interaction instead of the Property listener itself. That said I have never encountered a specific order dependency like this.

I'm happy to have more conversation with anyone that would like.