josephg / Chipmunk-js

Port of slembcke/Chipmunk-Physics to Javascript
http://dl.dropbox.com/u/2494815/demo/Joints.html
536 stars 59 forks source link

Probably wrong context in SpatialIndex.prototype.collideStatic #24

Open Legich55555 opened 10 years ago

Legich55555 commented 10 years ago

Probably wrong context is used in SpatialIndex.prototype.collideStatic.

SpatialIndex.prototype.collideStatic = function(staticIndex, func) { if(staticIndex.count > 0){ var query = staticIndex.query;

    var self = this;

    this.each(function(obj) {
                    // Wrong call: query(self, obj, new BB(obj.bb_l, obj.bb_b, obj.bb_r, obj.bb_t), func); 
        query.call(self, obj, new BB(obj.bb_l, obj.bb_b, obj.bb_r, obj.bb_t), func);
    });
}

};

josephg commented 10 years ago

What context (this binding) do you think is appropriate for query?

Legich55555 commented 10 years ago

Ups. I pasted wrong code with a fix. This is original one:

SpatialIndex.prototype.collideStatic = function(staticIndex, func) { if(staticIndex.count > 0){ var query = staticIndex.query;

    this.each(function(obj) {
        query(obj, new BB(obj.bb_l, obj.bb_b, obj.bb_r, obj.bb_t), func);
    });
}

};

query is called without any context so it becomes window. I fixed it using caller context (using self) but probably it is wrong and it should be staticIndex.

josephg commented 10 years ago

... I don't think it makes sense to rely on this being bound to anything in particular in the query function. If you want to pass arguments into the function, you should use a closure context. I could arbitrarily pick something, but .call() is slower than calling a method normally as far as I know.

appcrash commented 8 years ago

I'm not a physics engine expert, but the query is referring

BBTree.prototype.query = function(bb, func) { ... }

I think what you want is:

query.call(obj, new BB(obj.bb_l, obj.bb_b, obj.bb_r, obj.bb_t), func);

otherwise this is referring global ...