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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

FBXLoader: Fix Filename to RelativeFilename in parseImage() #13875

Merged
merged 2 commits into from
Apr 17, 2018

Conversation

acidsound
Copy link
Contributor

@acidsound acidsound commented Apr 16, 2018

The path of "Filename" property is useless. It depends on local directory (ex. C:\somewhere\blahblah\somefile.fbx). RelativeFilename is better for URI.
I made a tiny example in codesandbox ( https://codesandbox.io/s/y2kv8yyw5j ).
enjoy it! 馃拑

The path of "Filename" property is useless. It depends on local directory (ex. C:\somewhere\blahblah\shitty.fbx). RelativeFilename is better for URI.
@Mugen87 Mugen87 changed the title fix Filename to RelativeFilename in parseImage() FBXLoader: Fix Filename to RelativeFilename in parseImage() Apr 16, 2018
@mrdoob mrdoob requested a review from looeee April 16, 2018 22:47
@mrdoob mrdoob added this to the r92 milestone Apr 16, 2018
@looeee
Copy link
Collaborator

looeee commented Apr 17, 2018

You'll need to update the line below as well to prevent breaking embedded images:

blobs[ videoNode.Filename ] = image;

To

blobs[ videoNode.RelativeFilename || videoNode.Filename ] = image;

Copy link
Collaborator

@looeee looeee left a comment

Choose a reason for hiding this comment

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

Looks good now!

@mrdoob mrdoob merged commit bea5222 into mrdoob:dev Apr 17, 2018
@mrdoob
Copy link
Owner

mrdoob commented Apr 17, 2018

Thanks!

@acidsound
Copy link
Contributor Author

Awesome! 馃憤

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.

None yet

3 participants