Allow functions to take parameters object

Feature requests
1

Issue:

The current way of using comma-separated parameters for functions is error-prone and harder to use:

The user has to remember the order of the parameters. This problem might be solved for TypeScript users by the editor but it is still a problem for JS developers.

When not using all of the parameters, still needs to enter a value, which makes it harder to read the code.

 const importResult = await SceneLoader.ImportMeshAsync(
            "",
            "",
            controllerModel,
            scene,
            undefined,
            ".glb"
        );

If the developer wants to use a configuration object the implementation would be harder.

Solution:

If as an option, we can give an object as a parameter with named values, no one needs to remember the order of parameters and would be easier to read code examples.

const importResult = await SceneLoader.ImportMeshAsync({
        sceneFilename: controllerModel, 
        scene: scene, 
        pluginExtension: ".glb"
});

I am happy to help to refactor.

Thanks!

1 Like
2

There’s quite a few places in the API where options objects are used e.g. the static MeshBuilder methods. But even here, required parameters like name and scene etc are separate. I’ve grown to appreciate this hybrid approach and use it in my own code.

I’m not a core contributor so can’t really comment on the rationale and I suspect there might be issues with ‘refactoring’ existing code as a key goal of Babylon.js is maintaining backwards compatibility as much as possible. But I’ll leave it to the core devs to comment as I’m just speculating :slight_smile:

3

Hey @Yonet and thanks for your post!

I’m totally ok to add a new SceneLoader.ImportMeshWithOptionsAsync that will technically just uses the params and dispatch them

We try to use options when it makes sense but talking about that specific function: It was created long time ago and we kept adding new params to it

I would obviously agree with you if we were about recreating it now but one of our foundation is to protect backward compatibility so customers and users can easily migrate their code to newer versions

4

For the SceneLoader methods could this be possible?
You have an Loader method added to Scene so that you had

Scene.Loader.ImportMesh(options);
Scene.Loader.ImportMeshWithAsync(options)
Scene.Loader.Append(options)

Then internally within SceneLoader.ts something along the lines of

public static ImportMesh(lots of parameter)
  //sort parameters into options;
  return Loader.ImportMesh(options)

OK probably SceneLoader and Scene.Loader would perhaps be confusing but I cannot think of a better name for Loader, - none of Opener, Fetcher, Uploader sound correct.

5

I understand and appreciate your concern of the backward compatibility. We can still update it in a backward compatible way. I can send a pull request. I do see that in other functions you are using options and it would be nice to update the old ones to have consistency in APIs.

Thanks!

2 Likes
6

Fantastic! Thanks a ton

1 Like