Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

any updates?

The team is focused on 0.15.1, i'm in wait until 0.16 becames the target, then i will merge and do the comments

there were too many changes on bevy_gltf since my last commit, will restart from main, will reopen when i commit something

@alice-i-cecile can we get this moving now? who would be SME on this topic to do the reviews?

@andriyDev see if this solution is satisfactory

@mockersf This PR makes more small files than I personally would, but it's definitely better than the current monster load_gltf function.

I see following code by jumping between different files easier that jumping between different locations on the same file

I see following code by jumping between different files easier that jumping between different locations on the same file

There's a balance to reach, and this PR takes it too far in the other direction in my opinion

Throwing in my 2 cents as someone who's looking to do work on gltf in future:

• I like the overall direction of the change.
• I agree that some of the files seem too small.
• On the other hand, there's still chunks of loader/mod.rs that could be refactored into some of the smaller files?
  • E.g. gltf_ext/mesh.rs is small now, but there's still 100+ lines in the loader that could move there in a follow up PR.
• Speaking selfishly, I'd much prefer a slightly overdone refactor to no refactor at all.

E.g. gltf_ext/mesh.rs is small now, but there's still 100+ lines in the loader that could move there in a follow up PR.

my first go into this at the tail end of 0.14 was a lot more aggressive with the changes, including breaking the contents of load_gltf, load_material, etc, methods into smaller methods, but there is already pushback with the current changes, so my idea was to leave to a different PR

I've finally taken a proper look at this, and I'm broadly on board with this. Definitely a bit aggressive, but it's at least clear. It's much better than the status quo, so I'm going to merge this now.

1. I'm ok with merging the types on assets/ into a single file
2. I could also merge the types on extensions/ into a single file, but for this one I would vote against, as to have a clearer view of which extensions we already have
3. I could also merge the methods on gltf_ext/ into a single file, but I would prefer not to, my reasoning for the split was, if the main type of the method is present in gltf::N or gltf::json::N, I put the method in gltf_ext/N, so methods that use gltf::Gltf or gltf::Document would go on the root, and gltf::texture::Texture and gltf::json::texture::Info would go into gltf_ext/texture.rs, etc
4. gltf_ext/extras.rs I can merge with the module root or implement as From for the components

5. ImageOrPath and DataUri are in separate modules because I couldn't think of a name for the module that would work

Let's go with 1 and 4, and then resolve 5 by moving them back into lib.rs for now? I like the extensions being separate too.

4 by moving to mod.rs or impl From?

Let's go with the From route. Although if I understand correctly, the conversion is fallible, in which case it should be TryFrom.

is my solution for 4 satisfactory?

I quite like your final solution for 4. I think this is a nicer balance now; thanks for your patience. These sorts of reorganization PRs are quite painful due to merge conflicts and bikeshedding, but it's important to do them every now and then.