Impact

This forum is read only and just serves as an archive. If you have any questions, please post them on github.com/phoboslab/impact

6 years ago by Joncom

It would be nice if the engine saw a fix for this issue which has been mentioned already (in the private forum):

The way in which erase and random functions have been added to the Array prototype mess with enumeration and make it difficult to use other JavaScript libraries in tandem with Impact.

For example, MIDI.js cannot be used with Impact so long as this is not fixed, because the necessary MIDI.loadPlugin function eventually works its way to these lines:

for (var key in conf.items) {
	self.queue.push(conf.items[key]);
}

The result is that the erase and random functions are pushed into the queue array, breaking the plugin.

The fix appears to be a simple one (courtesy of vincentpiel):

/* impact.js */

Object.defineProperty(Array.prototype,'erase', { value : eraseItemFromArray } );

function eraseItemFromArray(item) {
    var i=this.indexOf(item);
    if (i>=0) this.splice(i, 1);
    return this;
};

Object.defineProperty(Array.prototype,'random', { value : randomItem } );

function randomItem() {
    return this[ 0 | (Math.random() * this.length) ];
};

6 years ago by FragOnly

Hello,

I am not arguing against a fix, but until it is fixed the loop should probably be written such as:
for (var key in conf.items) {
    if(conf.items.hasOwnProperty(key)) {
        self.queue.push(conf.items[key]);
    }
}

See http://yuiblog.com/blog/2006/09/26/for-in-intrigue/ and http://jslint.com/lint.html section 'for in' for more details.

I also created a fiddle to illustrate the problem: http://jsfiddle.net/37Rxd/1/

6 years ago by dominic

Fixed in the git repo on your download page.

For the record: I always considered iterating over Arrays with for...in loops "wrong". I remember an older Safari at the time I wrote the first Impact version not supporting it at all and it still has some other pitfalls. I'm not arguing against the fix either, I'm arguing against using for...in for Arrays :)

6 years ago by Joncom

Awesome, thanks!

6 years ago by vincentpiel

Let's be a little serious, here :

The reasons quoted in the link about not using for...in are just not relevant.

for ... in behaves differently from for( ; ; ), one willing to use it has to live with it and open some book or browse MDN.

Adding an issue -by polluting the prototype- could anyway not help at all 'fixing' 'this perfectly legal construct.

...

6 years ago by gxxaxx

Could someone post a snippet of the Array fix from the git repo, or perhaps point me in the direction of a tutorial or documentation on how to use the link provide on our download page for cloning.
Thanks

6 years ago by Joncom

@gxxaxx: Replace these lines in impact.js:
Array.prototype.erase = function(item) {
	for( var i = this.length; i--; ) {
		if( this[i] === item ) {
			this.splice(i, 1);
		}
	}
	return this;
};

Array.prototype.random = function() {
	return this[ Math.floor(Math.random() * this.length) ];
};

...with these lines:
Object.defineProperty(Array.prototype,'erase', { value : eraseItemFromArray } );

function eraseItemFromArray(item) {
    var i=this.indexOf(item);
    if (i>=0) this.splice(i, 1);
    return this;
};

Object.defineProperty(Array.prototype,'random', { value : randomItem } );

function randomItem() {
    return this[ 0 | (Math.random() * this.length) ];
};
Page 1 of 1
« first « previous next › last »