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

MMDLoader: Detect model extension from file content. #28266

Merged
merged 1 commit into from
May 9, 2024

Conversation

YusakuNo1
Copy link
Contributor

Description

Before this change, the MMD model extension is detected from URL (code), this approach won't work for blob url because it's like blob://https://domain/uuid. As the author mentioned, it'll be better to detect it by the file content.

How to test
For PMD file testing, please use examples/webgl_loader_mmd.html
For PMX file testing, please use my test branch which includes the asset from Genshin Impact: https://github.com/YusakuNo1/three.js/tree/yusakuno1/detect-mmd-ext-test-pmx

@Mugen87 Mugen87 added this to the r165 milestone May 3, 2024
@Mugen87 Mugen87 requested a review from takahirox May 3, 2024 08:11
@Mugen87 Mugen87 changed the title Detect MMD model extension from file content MMDLoader: Detect model extension from file content. May 3, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented May 8, 2024

@Mugen87
Copy link
Collaborator

Mugen87 commented May 9, 2024

@takahirox I've reviewed the PR as good as possible and everything looks fine to me. The MMD examples work as before. Merging this now but an additional review later on would still be welcome 👍 .

@Mugen87 Mugen87 merged commit b8c4dce into mrdoob:dev May 9, 2024
7 of 11 checks passed
@takahirox
Copy link
Collaborator

Thanks, I will try to review later when I have time.

@takahirox
Copy link
Collaborator

I did quickly review and it looks good to me. I may review more details later if I have time.

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.

3 participants