CreateJS / SoundJS

A Javascript library for working with Audio. It provides a consistent API for loading and playing audio on different browsers and devices. Currently supports WebAudio, HTML5 Audio, Cordova / PhoneGap, and a Flash fallback.
http://createjs.com/
MIT License
4.42k stars 838 forks source link

SoundJS's AbstractPlugin.js had a set method problem, At version 1.0.0 #281

Closed Akimyou closed 6 years ago

Akimyou commented 6 years ago

https://github.com/CreateJS/SoundJS/blob/master/src/soundjs/AbstractPlugin.js

    /**
     * Handles internal preload completion.
     * @method _handlePreloadComplete
     * @param event
     * @protected
     */
    p._handlePreloadComplete = function (event) {
        var src = event.target.getItem().src;
        this._audioSources[src] = event.result;
        for (var i = 0, l = this._soundInstances[src].length; i < l; i++) {
            var item = this._soundInstances[src][i];
            item.setPlaybackResource(this._audioSources[src]);
            // ToDo consider adding play call here if playstate == playfailed
            this._soundInstances[src] = null;
        }
    };

This method call a set method named setPlaybackResource, then the method is replaced with a setter but without a deprecated wrapper.

It may cause bug as below.

01

Thanks you !

Akimyou commented 6 years ago

It may have another bug. When get length without check.

02

lannymcnie commented 6 years ago

@MIKUScallion Thanks, don't know how we missed that one!

The two issues don't seem related (even though they are in the same method)

lannymcnie commented 6 years ago

Regarding the second issue -- what are the circumstances that cause this? Although we aren't adequately checking for the error condition, it isn't clear how to reproduce that particular use-case. Can you post more info?

Akimyou commented 6 years ago

Haha, I will try to extract use-case demo about the second issue. &_&

lannymcnie commented 6 years ago

The second issue is documented here: https://github.com/CreateJS/SoundJS/issues/283

I am closing this one for now.

k-may commented 6 years ago

I don't understand how this is closed? I'm experiencing the first issue.

lannymcnie commented 6 years ago

@k-may What version are you using? This is just fixed in NEXT, the tagged version (1.0.2) does not contain the fix.

k-may commented 6 years ago

Okay, that explains it, I'm using the latest download, 1.0.0