GLTFLoader: Log error when loader extension doesn't have a name.

[marwie]

Description

This logs an error when a new GltfLoader plugin is registered that does not have a name

This contribution is funded by 🌵 Needle

[mrdoob]

Do you mind giving more context?

• When does this happen?
• What happens if we don't this check?

[marwie]

Hi, yes sure

• happened in Needle Engine where we had two GltfLoaderPlugins without a name and didn't notice that one of them wasnt being called anymore, causing bugs / breaking functionality in some cases
• without a name the plugin gets undefined as a key and whichever comes last just overrides the previously registered one in the gltfloader plugins dict and only this last one will then receive callbacks. The others would just silently stop working. So this should help to find those mistakes early

[donmccurdy]

This is OK with me, though I would prefer to just log the error message and not to print the implementation of the plugin.

[marwie]

The idea with logging the implementation was to get an idea which plugin was misbehaving

would you prefer to log e.g. the constructor name if that exists (or something else) ?

[donmccurdy]

I'm hoping it is enough to basically say, a plugin was invalid because it has no name, and let the developer set a breakpoint. Devtools will point you to the source line of the log.

Slightly subjective opinion — but I prefer that libraries log strings and not objects, since they may run in remote environments with logging capabilities that differ from the browser's, and I do know of places GLTFLoader is used like that.

[takahirox]

Some random thoughts from my end

• I prefer logging strings rather than object as Don described
• Three.js doesn't validate input data. So checking whether plugin has a name might be inconsistent with other modules.
• Perhaps what must be checked would be non-unique name rather than non name?

if ( plugins[ plugin.name ] !== undefined ) {
  console.error(`Plugin must have a unique name`, plugin.name);
} else {
  plugins[ plugin.name ] = plugin;
}

[marwie]

I will remove logging the object and keep it simple, no problem.

About the validation: I would prefer to avoid situations where two extension become incompatible (because both miss a name) which could have easily been prevented by warning the authors about it. But in our case a check for non unique would also have helped of course