-
-
Notifications
You must be signed in to change notification settings - Fork 20.9k
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
GLTF: Store extensions in GLTFState and get supported extensions from GLTFDocumentExtension #66094
Merged
akien-mga
merged 2 commits into
godotengine:master
from
aaronfranke:gltf-used-extensions
Sep 20, 2022
Merged
GLTF: Store extensions in GLTFState and get supported extensions from GLTFDocumentExtension #66094
akien-mga
merged 2 commits into
godotengine:master
from
aaronfranke:gltf-used-extensions
Sep 20, 2022
+72
−11
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lyuma
reviewed
Sep 19, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything here is minor. I think it's good overall, and should allow for better handling of extensionsRequired
aaronfranke
force-pushed
the
gltf-used-extensions
branch
from
September 19, 2022 18:22
af4b5c5
to
f926eb8
Compare
fire
reviewed
Sep 19, 2022
aaronfranke
force-pushed
the
gltf-used-extensions
branch
2 times, most recently
from
September 19, 2022 19:33
057cb3c
to
b3abd54
Compare
fire
reviewed
Sep 20, 2022
fire
approved these changes
Sep 20, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed.
This allows GLTFDocumentExtension classes to add to the used extensions array.
aaronfranke
force-pushed
the
gltf-used-extensions
branch
from
September 20, 2022 00:40
b3abd54
to
7097e8a
Compare
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes were built with #66026 in mind, in fact this PR is a subset of it. These changes are mostly useful with that PR, but technically changing how the list of GLTF extensions is stored, and adding the ability for GLTFDocumentExtension to say which GLTF extensions it supports, is not dependent on refactoring how GLTFDocumentExtension classes are registered with GLTFDocument. So I split out this PR for easier reviewing.
This work was sponsored by The Mirror.