search

phetsims / axon

Axon provides powerful and concise models for interactive simulations, based on observable Properties and related patterns.
MIT License
11 stars 8 forks source link

Create TReadOnlyEmitter #450

Closed zepumph closed 6 months ago

zepumph commented 7 months ago

I think it would be valuable to have a type that you can only add listeners to, but not emit/dispose. Should be an easy addition. I know a couple spots where it could start being useful immediately. I'll take a look.

zepumph commented 7 months ago

@samreid what do you think about the above?

samreid commented 7 months ago

Looks great! Nice idea. I cleaned up the implementation slightly and committed that. Next I will replace some other occurrences that can now use TReadOnlyEmitter. Here is the report from the precommit hooks for the upcoming commits.

node chipper/js/scripts/precommit-hook-multi.js
detected changed repos: bamboo, center-and-variability, geometric-optics, joist, number-suite-common, phet-io, quadrilateral, scenery, scenery-phet, tambo, tandem, twixt, utterance-queue, vegas
bamboo: Success
center-and-variability: Success
geometric-optics: Success
joist: Success
number-suite-common: Success
phet-io: Success
quadrilateral: Success
scenery: Success
scenery-phet: Success
tambo: Success
tandem: Success
twixt: Success
utterance-queue: Success
vegas: Success
Done in 250584ms
samreid commented 7 months ago

@zepumph can you please review the changes? Anything else to do here? Close if you wish.

zepumph commented 6 months ago

Amazing thanks. Before closing I want to think if there is a way that we could outfit the public APIs of step/animationFrameTimers to be non emitable. It would be buggy to do this outside of Sim (or a couple other scenery specific startup cases). Do you think this is possible or worth it? Feel free to close.

samreid commented 6 months ago

Do you think this is possible or worth it?

This seems like something that is better accomplished through lint rules or documentation rather than via the type system. To accomplish it within the type system would require otherwise undesirable moves, such as making stepTimer and animationTimer more local to Sim.ts. I also think if code erroneously called either of those methods, and passed in a number, then simulation behavior would likely be disrupted and easily caught. So I would be OK to close. But let me know if you want to investigate lint rules, documentation or architecture changes to work on this further.