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 missing tutorial models #90

Merged
merged 8 commits into from
Jan 5, 2024
Merged

Conversation

javagl
Copy link
Contributor

@javagl javagl commented Dec 27, 2023

Fixes #70

Some models that are shown in the glTF tutorial had been missing here. From the ones that are listed in the issue, SimpleSkin was already added a while ago. The SimpleTexture and SimpleMeshes samples are added here.

I also cleaned up some of the README.body.md files of the other tutorial models, and added links to the respective sections of the tutorial.

(I think the LICENSE.md files will be created/overwritten during the model index update - but it's all CC0, so there shouldn't be any changes anyhow...)

"written"
],
"screenshot": "screenshot/screenshot.png",
"name": "SimpleMaterial",
Copy link
Member

Choose a reason for hiding this comment

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

I think this name field should have a space. "Simple Material"

Copy link
Member

Choose a reason for hiding this comment

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

Actually this repo is very inconsistent about spaces in proper model names. But it has some, for example:

"written"
],
"screenshot": "screenshot/screenshot.png",
"name": "SimpleTexture",
Copy link
Member

Choose a reason for hiding this comment

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

likewise

@javagl
Copy link
Contributor Author

javagl commented Jan 5, 2024

@emackey It's actually not soooo inconsistent: Nearly every name is just the ModelName with spaces to yield Model Name.

I just did that change for the newly added and previously existing "simple" (tutorial) models.

A few short notes that might be related (also @DRx3D ):

  • For some reasons, I thought that name was supposed to be the name (as in "the directory- and file name"). So the purpose of the name should probably be added at https://github.com/KhronosGroup/glTF-Sample-Assets/blob/main/SubmittingModels.md#example-metadata-file . Maybe roughly something like

    "A short, human-readable name of the model. This will become the link text in the overview page, and the title of the README of the model"

    or so

  • The Box With Spaces model currently is not displayed properly in the lists:
    Khronos - BoxWithSpaces
    Probably because ... it has spaces 😶

  • I'd suggest renaming the glTFPotOfCoals model to PotOfCoals. We don't use "Smurf Naming" prefixes

(I'll note these as internal TODOs, but if desired, they can be escalated into issues, to prevent them sinking to the bottom of my TODO list...)

@emackey
Copy link
Member

emackey commented Jan 5, 2024

I'd suggest renaming the glTFPotOfCoals model to PotOfCoals. We don't use "Smurf Naming" prefixes

Agreed. Also PotOfCoals has a glTF-Animation folder, which none of the other animated models have. Are there GLBs with and without the animation? Why? This needs some cleanup too.

@emackey emackey mentioned this pull request Jan 5, 2024
@emackey emackey merged commit 6ddacf2 into KhronosGroup:main Jan 5, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Several models in glTF-Tutorials do not exist in repository of glTF-Sample-Models
2 participants