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

Undefined reference in Loader videoLoadEvent (after game destroy) #742

Closed
Akurn opened this issue Sep 27, 2024 · 1 comment · Fixed by #743
Closed

Undefined reference in Loader videoLoadEvent (after game destroy) #742

Akurn opened this issue Sep 27, 2024 · 1 comment · Fixed by #743

Comments

@Akurn
Copy link
Contributor

Akurn commented Sep 27, 2024

Hey folks, posting what looks like a small code oversight/error due to legacy bugs and recent refactors.
But I wanted to first confirm some early behaviours before making a patch.


This Issue is about A bug in the API.

  • Phaser version(s): Phaser-CE 2.20.0

  • Live example: https://blake.pp.ua/loadVideoTag/

  • What steps produce the bug: (See below)

  • What's the error message?
    Chrome:

    Uncaught TypeError: Cannot read properties of null (reading 'fileComplete')

    Firefox:

    Uncaught TypeError: _this.game.load is null

  • What's the error trace (expand the error message)?
    from videoLoadEvent
    Screenshot 2024-09-27 at 4 02 01 PM


Cause

The affected line errors when the game instance is destroyed before any video preloading has completed.
The demo link has more detailed setup code, but its basically this:

game.state.add('Video', {
    preload: function () {
        this.load.video('space', 'wormhole.mp4');
    },
    create: function () {
        const video = game.add.video('space');
        video.play(true);
        video.addToWorld();
    }
});
game.state.start('Video', true, true);
TweenMax.delayedCall(0.01, () => {
    game.destroy();
});

Since the game is destroyed before the video's videoLoadEvent callback is fired, when it eventually runs, the _this.game.load reference no longer exists, and errors.

History

I was snooping through the git history, as it looked odd that the video callback would reference it like _this.game.load.fileComplete instead of _this.fileComplete (like the audio tag callback).

  1. It appears to be from an early bug (added in this commit):

    phaser-ce/src/loader/Loader.js

    Lines 2053 to 2054 in 77468e7

    // Why does this cycle through games?
    Phaser.GAMES[_this.game.id].load.fileComplete(file);

  2. That comment was then removed here: 7d308a2

  3. And in a more recent update was refactored here (10ef4b2) to be:

    _this.game.load.fileComplete(file);

    (which is the current 2.20.0 version)

Potential fix

The original code and comment suggests that there was an early browser bug which required the latest GAMES instance to be used instead of just the current Loader instance (this). Since the code is now this.game.load.fileComplete, isn't that the same as this.fileComplete? i.e. the original bug may no longer exist (but I wonder if @photonstorm can recall why that was needed)

The recent refactor to change it from Phaser.GAMES[_this.game.id] to _this.game looks like we can potentially now just match the audio callbacks and use _this.fileComplete directly?

i.e. this line could now be:

_this.fileComplete(file);

This prevents the error since fileComplete method already handles if the game is destroyed.
Demo link: https://blake.pp.ua/loadVideoTag2/

If this isn't ideal, or we don't want to potentially break it, could we just add a guard?

if (_this.game.load) {
    _this.game.load.fileComplete(file);
}
@samme
Copy link
Collaborator

samme commented Sep 27, 2024

i.e. this line could now be:

_this.fileComplete(file);

This prevents the error since fileComplete method already handles if the game is destroyed.

This should be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants