Skip to content

Commit

Permalink
✨ Replace 'ready event with whenReady`
Browse files Browse the repository at this point in the history
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
  • Loading branch information
clabe45 committed Jan 7, 2023
1 parent 712b6a4 commit e3beece
Show file tree
Hide file tree
Showing 22 changed files with 130 additions and 215 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Event constants (e.g. `Movie.Event.PLAY`).
- Image and video layers' `source` properties now accept urls ([#153](https://github.com/etro-js/etro/pull/153)).
- Movies, layers and effects have a new `ready` getter, indicating if they are ready to play.
- `'ready'` event, published when the movie, layer or effect is ready to play.
- Layers and effects now have an async `whenReady` method.
- `once` option for `subscribe`.

### Changed
Expand All @@ -21,7 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Deprecated
- `etro.applyOptions()` and `EtroObject#getDefaultOptions()` are deprecated. Instead, set each option in the constructor ([#131](https://github.com/etro-js/etro/issues/131)).
- Events starting with `'movie.'`, `'layer.'` or `'effect.'` are deprecated. Use the unprefixed event names or the new event constants instead.
- The `'movie.loadeddata'` event is deprecated. Consider using `Movie.Event.READY` instead.
- The `'movie.loadeddata'` event is deprecated.
- `'movie.record'` and `'movie.recordended'` events are deprecated. Use `Movie.Event.PLAY` and `Movie.Event.PAUSE` instead.
- The `'movie.ended'` event is deprecated. Use `Movie.Event.END` instead.

Expand Down
18 changes: 13 additions & 5 deletions spec/integration/layer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@ describe('Integration Tests ->', function () {
etro.event.publish(this, 'ready', {})
}

async whenReady (): Promise<void> {
if (this._ready)
return

await new Promise<void>(resolve => {
etro.event.subscribe(this, 'ready', () => {
resolve()
})
})
}

get ready () {
return this._ready
}
Expand Down Expand Up @@ -92,20 +103,17 @@ describe('Integration Tests ->', function () {
}
})

it('should publish a ready event when its effects all become ready', function () {
it('should resolve any calls to `whenReady` when all effects are ready', function (done) {
const effect1 = new CustomEffect()
layer.effects.push(effect1)

const effect2 = new CustomEffect()
layer.effects.push(effect2)

const layerReady = jasmine.createSpy('layerReady')
etro.event.subscribe(layer, 'ready', layerReady)
layer.whenReady().then(done)

effect1.makeReady()
effect2.makeReady()

expect(layerReady).toHaveBeenCalledTimes(1)
})
})

Expand Down
30 changes: 25 additions & 5 deletions spec/integration/movie.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,17 @@ describe('Integration Tests ->', function () {
etro.event.publish(this, 'ready', {})
}

async whenReady (): Promise<void> {
if (this._ready)
return

await new Promise<void>(resolve => {
etro.event.subscribe(this, 'ready', () => {
resolve()
})
})
}

get ready () {
return this._ready
}
Expand All @@ -218,6 +229,17 @@ describe('Integration Tests ->', function () {
etro.event.publish(this, 'ready', {})
}

async whenReady (): Promise<void> {
if (this._ready)
return

await new Promise<void>(resolve => {
etro.event.subscribe(this, 'ready', () => {
resolve()
})
})
}

get ready () {
return this._ready
}
Expand Down Expand Up @@ -315,7 +337,7 @@ describe('Integration Tests ->', function () {
expect(firedOnce).toBe(true)
})

it("should publish 'ready' when its layers and effects become ready", function (done) {
it('should be ready when all layers and effects are ready', function (done) {
// Remove all layers and effects
movie.layers.length = 0
movie.effects.length = 0
Expand All @@ -331,10 +353,8 @@ describe('Integration Tests ->', function () {
const effect = new CustomEffect()
movie.effects.push(effect)

// Subscribe to the event
etro.event.subscribe(movie, 'ready', () => {
done()
})
// `play` should not resolve until the movie is ready
movie.play().then(done)

// Make the layer and effect ready
layer.makeReady()
Expand Down
6 changes: 5 additions & 1 deletion spec/unit/effect/base.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@ describe('Unit Tests ->', function () {
expect(effect.type).toBe('effect')
})

it('should always be ready', function () {
it('should be ready by default', async function () {
// Make sure that `ready` is true
expect(effect.ready).toBe(true)

// Make sure that whenReady() resolves
await effect.whenReady()
})

it('should set _target when attached', function () {
Expand Down
3 changes: 2 additions & 1 deletion spec/unit/layer/audio-source.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ describe('Unit Tests ->', function () {
movie.duration = 4
})

it('should be ready when source is ready', function () {
it('should be ready when source is ready', async function () {
expect(layer.ready).toBe(true)
await layer.whenReady()
})

it('should not be ready when source is not ready', function () {
Expand Down
6 changes: 5 additions & 1 deletion spec/unit/layer/base.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@ describe('Unit Tests ->', function () {
layer = new etro.layer.Base({ startTime: 0, duration: 4 })
})

it('should always be ready', function () {
it('should be ready by default', async function () {
// Make sure that `ready` is true
expect(layer.ready).toBe(true)

// Make sure that whenReady() resolves
await layer.whenReady()
})

it("should be of type 'layer'", function () {
Expand Down
3 changes: 2 additions & 1 deletion spec/unit/layer/visual-source.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ describe('Unit Tests ->', function () {
movie.duration = 4
})

it('should be ready when source is ready', function () {
it('should be ready when source is ready', async function () {
expect(layer.ready).toBe(true)
await layer.whenReady()
})

it('should not be ready when source is not ready', function () {
Expand Down
4 changes: 3 additions & 1 deletion spec/unit/mocks/effect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@ export function mockBaseEffect () {
const effect = jasmine.createSpyObj('effect', [
'getDefaultOptions',
'tryAttach',
'tryDetach'
'tryDetach',
'whenReady'
])
effect.getDefaultOptions.and.returnValue({})
effect.tryAttach.and.callFake(movie => {
// Manually attach layer to movie, because `attach` is stubbed.
// Otherwise, auto-refresh will cause errors.
effect.movie = movie
})
effect.whenReady.and.resolveTo()

effect.type = 'effect'
effect.enabled = true
Expand Down
4 changes: 3 additions & 1 deletion spec/unit/mocks/layer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,16 @@ export function mockBaseLayer () {
'tryDetach',
'start',
'stop',
'render'
'render',
'whenReady'
])
layer.getDefaultOptions.and.returnValue({})
layer.tryAttach.and.callFake(movie => {
// Manually attach layer to movie, because `attach` is stubbed.
// Otherwise, auto-refresh will cause errors.
layer.movie = movie
})
layer.whenReady.and.resolveTo()

layer.type = 'layer'
layer.active = false
Expand Down
23 changes: 1 addition & 22 deletions spec/unit/movie/effects.spec.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
import etro from '../../../src'
import { MovieEffects } from '../../../src/movie/effects'
import { mockBaseEffect } from '../mocks/effect'
import { mockMovie } from '../mocks/movie'

describe('Unit Tests ->', function () {
describe('MovieEffects', function () {
let effects: MovieEffects
let checkReady: () => void

beforeEach(function () {
const movie = mockMovie()
checkReady = jasmine.createSpy('checkReady')
effects = new MovieEffects([mockBaseEffect()], movie, checkReady)
effects = new MovieEffects([mockBaseEffect()], movie)
})

it('should call `tryAttach` when an effect is added', function () {
Expand All @@ -25,23 +22,5 @@ describe('Unit Tests ->', function () {
effects.pop()
expect(effect.tryDetach).toHaveBeenCalled()
})

it('should call `checkReady` when an effect is added', function () {
const effect = mockBaseEffect()
effects.push(effect)
expect(checkReady).toHaveBeenCalled()
})

it('should call `checkReady` when an effect is removed', function () {
effects.pop()
expect(checkReady).toHaveBeenCalled()
})

it('should subscribe to `ready` changes on effects', function () {
spyOn(etro.event, 'subscribe')
const effect = mockBaseEffect()
effects.push(effect)
expect(etro.event.subscribe).toHaveBeenCalledWith(effect, 'ready', jasmine.any(Function))
})
})
})
23 changes: 1 addition & 22 deletions spec/unit/movie/layers.spec.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
import etro from '../../../src'
import { MovieLayers } from '../../../src/movie/layers'
import { mockBaseLayer } from '../mocks/layer'
import { mockMovie } from '../mocks/movie'

describe('Unit Tests ->', function () {
describe('MovieLayers', function () {
let layers: MovieLayers
let checkReady: () => void

beforeEach(function () {
const movie = mockMovie()
checkReady = jasmine.createSpy('checkReady')
layers = new MovieLayers([mockBaseLayer()], movie, checkReady)
layers = new MovieLayers([mockBaseLayer()], movie)
})

it('should call `tryAttach` when an layer is added', function () {
Expand All @@ -25,23 +22,5 @@ describe('Unit Tests ->', function () {
layers.pop()
expect(layer.tryDetach).toHaveBeenCalled()
})

it('should call `checkReady` when an layer is added', function () {
const layer = mockBaseLayer()
layers.push(layer)
expect(checkReady).toHaveBeenCalled()
})

it('should call `checkReady` when an layer is removed', function () {
layers.pop()
expect(checkReady).toHaveBeenCalled()
})

it('should subscribe to `ready` changes on layers', function () {
spyOn(etro.event, 'subscribe')
const layer = mockBaseLayer()
layers.push(layer)
expect(etro.event.subscribe).toHaveBeenCalledWith(layer, 'ready', jasmine.any(Function))
})
})
})
14 changes: 5 additions & 9 deletions src/effect/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,6 @@ import BaseObject from '../object'
* @deprecated All visual effects now inherit from `Visual` instead
*/
export class Base implements BaseObject {
static readonly Event = {
/**
* Fired when the effect is ready to be applied
*
* @event
*/
READY: 'ready'
}

type: string
publicExcludes: string[]
propertyFilters: Record<string, <T>(value: T) => T>
Expand All @@ -35,6 +26,11 @@ export class Base implements BaseObject {
this._target = null
}

/**
* Wait until this effect is ready to be applied
*/
async whenReady (): Promise<void> {} // eslint-disable-line @typescript-eslint/no-empty-function

/**
* Attaches this effect to `target` if not already attached.
* @ignore
Expand Down
8 changes: 8 additions & 0 deletions src/layer/audio-source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,14 @@ class AudioSource extends Base {
})
}

async whenReady (): Promise<void> {
await super.whenReady()
if (this.source.readyState < 2)
await new Promise(resolve => {
this.source.addEventListener('loadeddata', resolve)
})
}

attach (movie: Movie) {
super.attach(movie)

Expand Down
9 changes: 0 additions & 9 deletions src/layer/audio.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// TODO: rename to something more consistent with the naming convention of Visual and VisualSourceMixin

import { AudioSource, AudioSourceOptions } from './audio-source'
import { publish } from '../event'

type AudioOptions = AudioSourceOptions

Expand All @@ -16,14 +15,6 @@ class Audio extends AudioSource {
constructor (options: AudioOptions) {
super(options)

// Emit ready event when the audio is ready to play
// TODO: Change to 'canplay'
this.source.addEventListener('loadeddata', () => {
// Make sure all superclasses are ready
if (this.ready)
publish(this, Audio.Event.READY, {})
})

if (this.duration === undefined)
this.duration = (this).source.duration - this.sourceStartTime
}
Expand Down
14 changes: 5 additions & 9 deletions src/layer/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,6 @@ interface BaseOptions {
* A layer outputs content for the movie
*/
class Base implements EtroObject {
static readonly Event = {
/**
* Fired when the layer is ready to render
*
* @event
*/
READY: 'ready'
}

type: string
publicExcludes: string[]
propertyFilters: Record<string, <T>(value: T) => T>
Expand Down Expand Up @@ -71,6 +62,11 @@ class Base implements EtroObject {
this._movie = null
}

/**
* Wait until this layer is ready to render
*/
async whenReady (): Promise<void> {} // eslint-disable-line @typescript-eslint/no-empty-function

/**
* Attaches this layer to `movie` if not already attached.
* @ignore
Expand Down
Loading

0 comments on commit e3beece

Please sign in to comment.