Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GltfLoader::new. #9120

Merged
merged 2 commits into from Jul 14, 2023
Merged

Add GltfLoader::new. #9120

merged 2 commits into from Jul 14, 2023

Conversation

pcwalton
Copy link
Contributor

Objective

In my application, I'm manually wrapping the built-in Bevy loaders with a wrapper loader that stores some metadata before calling into the inner Bevy loader. This worked for the glTF loader in Bevy 0.10, but in Bevy 0.11 it became impossible to do this because the glTF loader became unconstructible outside Bevy due to the new private fields within it. It's now in fact impossible to get a reference to a GltfLoader at all from outside Bevy, because the only way to construct a GltfLoader is to add the GltfPlugin to an App, and the GltfPlugin only hands out references to its GltfLoader to the asset server, which provides no public access to the loaders it manages.

Solution

This commit fixes the problem by adding a public new method to allow manual construction of a glTF loader.

In my application, I'm manually wrapping the built-in Bevy loaders with
a wrapper loader that stores some metadata before calling into the inner
Bevy loader. This worked for the glTF loader in Bevy 0.10, but in Bevy
0.11 it became impossible to do this because the glTF loader became
unconstructible outside Bevy due to the new private fields within it.
It's now in fact impossible to get a reference to a GltfLoader at all
from outside Bevy, because the only way to construct a GltfLoader is to
add the GltfPlugin to an App, and the GltfPlugin only hands out
references to its GltfLoader to the asset server, which provides no
public access to the loaders it manages.

This commit fixes the problem by adding a public `new` method to allow
manual construction of a glTF loader.
@github-actions
Copy link
Contributor

Welcome, new contributor!

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

@hymm hymm added this to the 0.11.1 milestone Jul 11, 2023
@hymm hymm added A-Assets Load files from disk to use for things like images, models, and sounds C-Regression Functionality that used to work but no longer does. Add a test for this! labels Jul 11, 2023
@pcwalton
Copy link
Contributor Author

This should be ready to merge, I think.

Copy link
Member

@mockersf mockersf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding a new method, could you make the fields of GltfLoader public?

@alice-i-cecile alice-i-cecile added the C-Usability A simple quality-of-life change that makes Bevy easier to use label Jul 12, 2023
@cart cart enabled auto-merge July 13, 2023 23:37
@cart cart added this pull request to the merge queue Jul 13, 2023
Merged via the queue into bevyengine:main with commit 05a35f6 Jul 14, 2023
20 checks passed
cart added a commit that referenced this pull request Aug 10, 2023
# Objective

In my application, I'm manually wrapping the built-in Bevy loaders with
a wrapper loader that stores some metadata before calling into the inner
Bevy loader. This worked for the glTF loader in Bevy 0.10, but in Bevy
0.11 it became impossible to do this because the glTF loader became
unconstructible outside Bevy due to the new private fields within it.
It's now in fact impossible to get a reference to a GltfLoader at all
from outside Bevy, because the only way to construct a GltfLoader is to
add the GltfPlugin to an App, and the GltfPlugin only hands out
references to its GltfLoader to the asset server, which provides no
public access to the loaders it manages.

## Solution

This commit fixes the problem by adding a public `new` method to allow
manual construction of a glTF loader.

---------

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Regression Functionality that used to work but no longer does. Add a test for this! C-Usability A simple quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants