three.js: Loaders: Inconsistent implementations of .setPath()

I also prefer setResourcePath over setTexturePath and the plural alternatives. Since resource is a generic term, I do not think we need an additional setTexturePath method for loaders where all of the resources happen to be textures, like MTLLoader. If we’re agreed on that, the tasks are updating (all?) loaders such that:

• .setPath() affects original file (or multiple original files forCubeTextureLoader)
• .setResourcePath() affects dependencies of original file, where applicable

I’m assuming .setAnimationPath() would be for MMDLoader only. I think that is OK.

Any other loaders which is in a similar situation?

Unfortunately not. But I think it should be okay to establish something like .setAnimationPath() and .animationPath. Any opinions on this?

@Mugen87 PR for OBJLoader2 V2.5.0 implementing this agreement, plus nodejs support and hopefully a working fix for minification problems with webpack/angular should be ready this weekend (before R98 is due).

@takahirox I’ve had a look at MMDLoader and I think it’s a bit complicated to implement a .setPath() method. The loader has a method MMDLoader.loadWithAnimation() that accepts two URLs to assets, possibly both in different directories. A single base path defined via .setPath() could not handle a use case like the following. What do you think?

https://github.com/mrdoob/three.js/blob/2f46e2a6ed597423c1aee7789cc1ad0e988a8772/examples/webgl_loader_mmd.html#L124-L133

Yes, I will take care. Thanks for pinging me.

@kaisalmen Can you please check how LoaderSupport and OBJLoader2 are affected by this issue? It would be great if you can provide a PR in order to align the interface if necessary.

I suggest the owners mentioned in this wiki page should migrate their respective loaders.

What is left to do here? Is someone who is familiar with this issue willing to create a new post with a task list?

/ping @takahirox /ping @donmccurdy /ping @Mugen87

.setResourcePath sounds good to you @WestLangley?

Sure.

To be more concrete (I’ll work it out next days; Happy 2018 !!):

<script src='../build/three.js'             ></script>
<script src='js/controls/OrbitControls.js'  ></script>
<script src='js/libs/dat.gui.min.js'        ></script>
<script src='js/loaders/LoaderSupport.js'   ></script>
<script>

    const strLoaders = [
        {name: 'OBJLoader'      , ext: 'obj'    },
        {name: 'OBJLoader2'     , ext: 'obj'    },
        {name: 'GLTFLoader'     , ext: 'gltf'   },
        {name: 'ColladaLoader'  , ext: 'dae'    },
        {name: 'FBXLoader'      , ext: 'fbx'    },
        //TODO: all model-loaders...
    ]

    const models = [
        'Point',
        //TODO: use all these models ...
        'Edge',
        'Cube',
        'Monkey',
    ]

    for (let strLoader of strLoaders){

        const script = document.createElement('script')
        document.head.appendChild( script )
        script.onload = function(e) {
            const loader = new THREE[ this.strLoader.name ]
            loader.load(
                `models/_/Points/${models[0]}.${this.strLoader.ext}`,
                model =>{
                    console.log( model )
                    //TODO: add to scene, render, verify ...
                }
            )
        }
        script.strLoader = strLoader
        script.src= `js/loaders/${ script.strLoader.name }.js`

    }//for

</script>

I’m not sure if it’s the same issue you’re facing, but I’m already aware of that GLTFLoader can’t handle(display) Points correctly.

The reason is GLTFLoader doesn’t create PointsMaterial for Points. Points needs PointsMaterial to be rendered. It can’t be rendered with Mesh*Material (If I’m right).

I already started to work on it in my local branch.

Another issue I’m aware of is glTF doesn’t hold point size information. I’m gonna ask or open an issue to glTF team (if necessary).

As @donmccurdy mentioned, please open an issue if what I mentioned aren’t what you ran into.

I’ve renamed the issue Loaders: Inconsistent implementations of .setPath(), as I think the changes to consider would be:

1. Ensuring that all loaders use setPath() for extra resources but not the original request.
2. Deprecate setPath() on FileLoader and ImageLoader, as they have no extra resources.
3. Consider renaming setPath() in cases where it must behave differently than this, renaming the normal case to setResourcePath(), or similar clarification.