Fixes for Quads and Smoothing.

[Banbury]

I have created a fix for quads with vertices in the wrong order. Without this some meshes will have unsightly holes.

Additionally I have made some small changes, to smooth the meshes.

[le717]

/cc @rioforce

[rioforce]

When I use model cleanup on the High-quality bricks, it gives an error. Here's what it says:

[Banbury]

Could you attach the LDraw file?

[le717]

Hey! Thanks for the PR! While I am a novice Python programmer, the Blender API is confusing to me, and the documentation is not very well written either. The quads fix work well! I never would have been able to write that! The only issues I see would be

1. The edge split code is a bit iffy. I cc'd rioforce so he could take a look at the results, and it seems 0.523599 (30 degrees as shown in the Blender GUI) works just as well, and 0.78539 (44 degrees in GUI) is unnecessarily too much.
2. The remove doubles lines: I had it the way it was because I wanted it to use the default value (0.0001), so as not to skew it toward personal preferences for how the bricks look. Again, 0.01 makes no visible difference, but, again, I was going for standards over preference, as every one has a slightly different view on how it should look. Plus, this type of cleanup in the original code created more doubles than the original brick had!
   You don't completely have to do anything on these, but
3. Would you please comment your new code? This is originally a Blender 2.5 script that had very little documentation on how it worked, and I do my best to ensure all new changes are fully documented.

The model in the screenshot: it's car.dat from the models folder in the standard LDraw installation. It's what I usually use to test any changes.

Again, thanks for the PR! 👍 😃

[Banbury]

Hi,
re 1. The edge split angle comes from experience. There are a lot of parts where 30 deg is not enough.
re 2. It does make a difference for the creation of normals. With the higher value more normals point in the right direction. But I will retest it.
re 3. Sorry for the lack of comments. But I'm doing this on the quick. I'm afraid it's either that or nothing.

Another pull request is coming up shortly. That one fixes materials for sub-parts and multicolored bricks.

[Banbury]

I have tested importing the car model and everything works. Please retry with the patch in the new pull request.

[le717]

It works correctly, but this error comes up when I import a model not using HQ bricks with cleanup, then import the same using HQ bricks. Importing using HQ bricks and cleanup on the first import after loading Blender works.

That's fine on all three points. If you know by experience those settings work best, than that better than standards. No comments are good too. I might be able to add them after thoughtfully reading the code.

Again, thanks for the PRs! I've had this on here for a while, but never had anyone really take an interest in it aside from a few users. I'll close this one and merge the other one when it's ready. 😉

[Banbury]

This seems to be a different problem entirely, Right now it's not possible to load the same model twice in a row. I'm restarting Blender every time.

Again sorry for the lack of comments. As a programmer I really should know better :). But I like to spend my scarce time coding rather than writing comments.

[le717]

I reopened the PR for the sake of discussing this bug, although this works best in an Issue (which I still may make).

Ah, so I assumed. Usually users won't restart Blender for every import, so this will need to be fixed. In the mean time, I can go ahead and merge the changes and make any small cleanup I've been seeing. That way, you can focus on this bug and the new changes can already be prepared for a future release.

Yea, I know how that feels. For me, I usually try to comment it right after I get it working. Like I said, it's fine.

[Banbury]

I guess the crash has to do with all those nasty global variables. But without debugging Blender itself, that's hard to tell. Global variables are evil and should be avoided at all cost. Also the recursive constructor is probably not a good idea.
This looks like a major rewrite is necessary. Fun :(.

[le717]

Eek. That's not good in both areas. :( Well, if/when it is rewritten, hopefully whoever does it and write it to also support BrickSmith models (the format for those .ldr models is way different). I did figure out David wrote this in the Blender 2.5 days, and it is almost a clean port of a 2.4 script (I'll link it if needed), and this script was way from finished, if that explains anything.

Alright, I'm gonna merge #2, even though it has that error (since it still imports and cleans up correctly despite the error), and later I'll port this bug to a proper issue. ;)

[le717]

Er, I just noticed the last commit for global variables (Mobile GitHub site didn't show it). Can you make a new PR with those changes?

[rioforce]

Nice new features in the script! :D One question, why did you use 0.502 for transparency? Don't you think 0.5 would be better, because it's a rounder number? ;)