Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate events #188

Open
clabe45 opened this issue Jan 7, 2023 · 0 comments
Open

Deprecate events #188

clabe45 opened this issue Jan 7, 2023 · 0 comments
Assignees
Labels
priority:medium type:feature New feature or request

Comments

@clabe45
Copy link
Collaborator

clabe45 commented Jan 7, 2023

TL;DR Events can be replaced with more user-friendly alternatives, such as callbacks and public layer and effect methods.

Events provide a pull data pattern, where the event listener pulls the event from the event emitter. All events are emitted by the movie.

In Etro, events often lead to spaghetti code. A cleaner alternative to listening for events outside the movie is to add callback methods to async functions. For instance, the 'movie.play' event can be replaced with an onStart option for Movie#play(). Now, the subscriber no longer needs to unsubscribe from the 'movie.play' event when the movie is done playing.

Before:

function startedPlaying() {
  // Started playing (all resources loaded, etc.)
}

movie.play().then(() => {
  // Done playing
  etro.event.unsubscribe(movie, 'movie.play', startedPlaying)
})

etro.event.subscribe(movie, 'movie.play', startedPlaying)

After:

await movie.play({
  onStart: () => {
    // Started playing (all resources loaded, etc.)
  },
})

// Done playing

We can replace events that layers and effects subscribe to with public methods on the layers and effects. The movie can call these methods instead of emitting the events.

@clabe45 clabe45 added type:feature New feature or request priority:medium labels Jan 7, 2023
@clabe45 clabe45 self-assigned this Jan 7, 2023
clabe45 added a commit that referenced this issue Jan 7, 2023
This is a much cleaner and safer way to wait for all resources to load.

Before, the layers and effects pushed their ready states to the movie.
To do this, they emitted the 'ready' event every time their ready states
changed. Since the ready state is accessed with a getter (the boolean
`ready` getter), every place in the code that *could* change `ready`
needed to emit the event (when needed).

Now, the movie pulls the ready state from its layers and effects. This
is as simple as wrapping the DOm events in promises and waiting for them
to load.

See #188
clabe45 added a commit that referenced this issue Jan 7, 2023
Previously, layers pulled `currentTime` updates from the movie by
listening for the 'timeupdate' and 'seek' events. In an effort to
replace the pull-data pattern provided by events with a simplified
push-data approach, the following changes were made:

- The 'seek' event was replaced by a `seek()` method that any layer can
  implement. Instead of layers listening for an event from the movie,
  the movie simply calls this method on every active layer with the
  layer's time. `seek()` may also be added to effects in the future.
- Similarly, the `timeupdate' event was replaced by a `progress()`
  method that any layer can implement.

See #188
clabe45 added a commit that referenced this issue Jan 7, 2023
The 'pause' event isn't too useful. Instead of subscribing to it, users
can wait for the promises returned by `play()`, `stream()` or `record()`
to resolve.

See #188
clabe45 added a commit that referenced this issue Jan 7, 2023
clabe45 added a commit that referenced this issue Jan 7, 2023
clabe45 added a commit that referenced this issue Jan 7, 2023
Since all events will be deprecated soon, it is misleading to add a new
way to use the event system.

Reverts 6ac91e1
See #188
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:medium type:feature New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant