-
Notifications
You must be signed in to change notification settings - Fork 19
correct file search order to give unofficial files precedence over official ones AND: correct usage of p/8 primitives folder #52
Comments
gh-40 - "(b) speed: the paths[] array construction should be moved out of the locate function. it is expensive and only needs to be done once."
Fixed in f0037a7. I should have already done this, but I'll add you to the list of authors. Do you want to go by Steffen or MinnieTheMoocher? 😉 |
the latter, please. it's my github identity. |
your f0037a7 corrects the unofficial/official files search order, but the error with the p/8 folder is still in it. it needs to be treated like this paths = [] I.e., |
There is no Is this the proper formatting? Python uses indentation levels for paths = []
paths.append(file_directory)
paths.append(os.path.join(LDrawDir, "models"))
paths.append(os.path.join(LDrawDir, "unofficial", "parts"))
if HighRes:
paths.append(os.path.join(LDrawDir, "unofficial", "p", "48"))
elif not HighRes:
paths.append(os.path.join(LDrawDir, "unofficial", "p", "8"))
paths.append(os.path.join(LDrawDir, "unofficial", "p"))
paths.append(os.path.join(LDrawDir, "parts"))
if HighRes:
paths.append(os.path.join(LDrawDir, "p", "48"))
elif not HighRes:
paths.append(os.path.join(LDrawDir, "p", "8"))
paths.append(os.path.join(LDrawDir, "p")) |
no, "elif not HighRes" would be equal to "else" which is wrong here!: the p/8 folder must ONLY be used if the user explicitly requests so. the current implementation is wrong, because it always uses that folder if HighRes is NOT enabled, the fix is very simple!
to
and
to
and
to
and
to
|
in a future refacturing, you can merge these 2 boolean flags to a simple multiple choice:
|
to make is simple, here is the corrected file (untested): |
Yikes the formatting. You might want to paste that in a Gist and edit your comment with the link. |
here it is :) |
Descriptions need reviewing by @MinnieTheMoocher
I tested the patch and it works. However, I did edit it to make the options exclusive so both are not used (and clean up the options a bit) and to work better with your last patch. The layout looks like this now. You can review the changes (and the descriptions) on the |
What about "normal-res" primitives? All you have is low and high. |
I didn't show that option in the screenshots. ;) |
Add patch from #52, rework options display
Fixed by #54, issue solved! |
Currently, import_ldraw.py reads:
This gives official files precedence over unofficial ones.
This is not what you usually want.
Your usual usecase is that you download corrected or updated parts
from the LDRAW parts tracker to that "unofficial files" folder
and want them to take precedence over the official ones.
Thus, the above lines need to be reordered to:
Additionally, the implementation of the p/8 folder is wrong:
(a) it must be searched both in official and unofficial files and
(b) it needs its own "LowRes" flag instead of depending on HighRes
Applying this correction, the above lines become:
Note that in addition to the existing HighRes flag, a new flag LowRes flag needs to be introduced.
The text was updated successfully, but these errors were encountered: