jeromeetienne / tquery

extension system for three.js
http://jeromeetienne.github.io/tquery/
MIT License
652 stars 119 forks source link

assert for Mesh material check doesn't let THREE.MeshFaceMaterial work #296

Closed alexcasalboni closed 11 years ago

alexcasalboni commented 11 years ago

This check doesn't allow me to use THREE.MeshFaceMaterial with a cube instance:

var red = tQuery.createMeshBasicMaterial().color(0xff0000).get(0);
var green = tQuery.createMeshBasicMaterial().color(0x00ff00).get(0);
var blue = tQuery.createMeshBasicMaterial().color(0x0000ff).get(0);
var multipleMaterial = new THREE.MeshFaceMaterial([red, green, blue, red, green, blue]);
tQuery.createCube().addTo(world).material(multipleMaterial);

And actually the same happens here: without it I could directly call createCube(multipleMaterial).

It looks like instanceof does not work well as a check here. Is it a bug or an intended limitation? Thank you

jeromeetienne commented 11 years ago

i would say the bug is in three.js :) because i think all material should inherit from THREE.Material but it would not help you much.

i will simply change the assert

jeromeetienne commented 11 years ago

there tell me if it fit your need

alexcasalboni commented 11 years ago

Thank you very much, but actually the main benefit would come from editing here so that it will work simply using tQuery.createCube(multipleMaterial).

By the way, you're right: the problem seems to be about THREE.MeshFaceMaterial prototype definition, since there is no prototype inheritance:

THREE.MeshFaceMaterial = function ( materials ) {
    this.materials = materials instanceof Array ? materials : [];
};
THREE.MeshFaceMaterial.prototype.clone = function () {
    return new THREE.MeshFaceMaterial( this.materials.slice( 0 ) );
};

There should be something similar to THREE.MeshFaceMaterial.prototype = Object.create( THREE.Material.prototype );.

I know that adding multiple inheritance checks all around the library is not very elegant, I will report this issue asap.

Thank you again

jeromeetienne commented 11 years ago

i did it this way to reduce the 'spreading bug workaround' effect you talked about

alexcasalboni commented 11 years ago

If you check the issue I've opened, it looks like it is an intended behaviour, since THREE.MeshFaceMaterial is just a container for multiple materials.

I don't think they're going to make it inherit from THREE.Material.