Skip to content

Fixed invalid texture names#22

Open
MrGaribaldi wants to merge 1 commit intoarindam-m:masterfrom
MrGaribaldi:master
Open

Fixed invalid texture names#22
MrGaribaldi wants to merge 1 commit intoarindam-m:masterfrom
MrGaribaldi:master

Conversation

@MrGaribaldi
Copy link

The sketchup api doesn't ensure there is a valid filename in texture.name, sometimes it only contains the filename extension.

Blender handles this fine, except when such materials are exported to other formats like fbx, then the missing filenames creates trouble.

I've solved this issue by checking if the texture.name starts with a '.' and the length is less than 5 characters. In this case we know that there will only be an image extension in the filename, and should combine it with the material name.

This simple fix ensures you can export the imported model.

Fixed issue when tex.name is only an image extension.
Now combines texture name with material name if texture name is 4 characters or less, and starts with a '.'
@arindam-m
Copy link
Owner

@MrGaribaldi
Thank you for your contribution, Erik. I am sorry for my much-delayed response, I was on a long hiatus.

I also couldn't merge your PR at the time because I have moved on with a few more changes and commits on my end already. It's always nice to create the PR for the most active branch, here develop.

Now since I have pulled all my changes to the main branch, would you like to resolve the current conflicts here and make a fresh PR?

  • I think you would just need to move your code-block from line#340 to line#428

I have one more question related to the logic you have used. But first, let us resolve the current conflict and from there we can move ahead. Also, please provide me with a sample file to reproduce the bug you have faced (if possible), that would be super nice.

@arindam-m arindam-m self-requested a review July 30, 2021 17:57
@arindam-m arindam-m added bug Something isn't working and removed bug Something isn't working labels Jul 30, 2021
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