ExtrasAsMetadata skips animations

Hi.

I’m working on a viewer for interactive models.
The KHR_interactivity still doesn’t work well, so i create FlowGraphs manually based on metadata in gltf file.
The metadata of animations is crucial for the use case, but it’s not loaded by the extension.

I tried to patch the extension:

export class ExtrasAsMetadata implements IGLTFLoaderExtension {
    // // //
    loadAnimationAsync(context: string, animation: IAnimation): Nullable<Promise<AnimationGroup>> {
        return this._loader.loadAnimationAsync(context, animation).then((babylonAnimation: AnimationGroup) => {
            this._assignExtras(babylonAnimation, animation);
            return babylonAnimation;            
        })
    }
    // // //
}

It works, but I’m totally not sure if it’s right way to do that.

Please, someone, confirm.

ping @srzerbetto @bghgary

That’s fine for now. Ideally, we need an assign function similar to the other extension points so that the extras can be assigned synchronously instead of asynchronously, but we don’t have that at the moment, so this is okay. @qwiglydee Would you like to open a PR for this?

cc @ryantrem

Be aware that the FlowGraph feature is experimental and things can change rapidly.

I could PR that exact change i posted + adjust tests etc

But I’m still a newcomer to all the suff, and having zero understanding how it works.

Also,
adding the assign implies changing extentions API and would break everything.

Yes, it would have to be done carefully.

Apparently, not.

There are no simple unit-tests for the loaders and extensions. Instead there are integration tests that load assets from playground server (either remote or somehow url-mangled to local).
My notebook fails to handle all the integration servers and testing infrastructure.

I’m giving up :unamused_face:

You could simply create a draft PR and everything will run server side for you ?

About:
image

I think the team is planning an update soon.

cc @bghgary to confirm.

I hope so!

But anyway, the extras field is still in glTF spec and it could be useful.
E.g. for timestamp labels or something.