Non-existent bricks crash script

[le717]

If a .dat does not exist, the script crashes with a FileNotFoundError rather than skipping it and moving on. Currently, the line it stems from is wrapped in a try… except Exception block. I anticipated this type of error but did not know what exactly would be raised, hence Exception. I can fix this by either changing the exception or (if possible) doing a os.path.exists() check on the variables before opening them for reading.

Image

[le717]

I got this partly fixed. It no longer crashes , but it kills the import process rather than skipping it (which is best). Even continue (which is supposed to skip the current loop and go on) doesn't work. Anything I do either kills it or gets stuck in an endless loop.

This may be something that can only be fixed by redoing the script to kill it's recursiveness, but I'm leaving the issue open because if it has to stop, it needs to say why. See around lines 180-184 for references to what I'm thinking.

[JoshTheDerf]

I'll take a look at this, it seems like it would be simple to fix.

[le717]

Go right ahead. I have already fixed the crash (as I said), but if you can get it to skip them, that's the best fix of them all.

[JoshTheDerf]

~~Can I have a file with missing bricks to test on?~~ "Made" one myself >:D

I've tried several things, and it ultimately looks as if the locate function won't return False or all conditionals are passing it through O_o

How exactly does that loop even know when to move to the next brick?

UPDATE: Okay wow.... Umm... I'd replace the while True with recursive calls to parse() and have parse return something, but I cannot tell what on earth tells the loop to move to the next brick. The code is very cryptic, and I have no idea what tmpdate means. It is definitely not a date.

UPDATE2: Ah, I see what is happening here. The variable names and comments are VERY misleading. XD Now the trick is to cache the next line of the main file for reading in case the current part fails.

UPDATE3: Progress made: Now the main .ldr file is cached in memory while the process goes. Whenever a new part is started, the line in that ldr file is cached as well. If a part fails, the importer tries to resume working from wherever it left off on the main file.

In practice, more of the file gets loaded, but it's still pretty bad.

UPDATE 4: Okay, I think this entire thing needs to be redone. Period. If only I could understand it enough to get all the recursion right.

Currently the structure appears to be:

1. Create a new object - >
2. Parse the main file, then all sub files, and append to the sub file list - >
3. go through the subfile list and create a new object for each subpart - > Step 1

It really needs to use functions, not while recursions, lists, and objects. Going to go work on something else now, to refresh my brain.

[le717]

Wow, how did I miss all this?! Will comment tomorrow, time for sleep now + on GitHub android app, so commenting is not the best.

[le717]

@Banbury, do you see anything in Tribex's summaries that might enable skipping of nonexistent bricks, or is he right on the button that it may not be possible without the rewrite you said it badly needs (which I agree with).

[le717]

Ah, I see what is happening here. The variable names and comments are VERY misleading. XD Then if you know what it really is, go right ahead and fix them! Or you can tell me the misleading variables and I'll change them.

Okay, I think this entire thing needs to be redone. Period. If only I could understand it enough to get all the recursion right.

Currently the structure appears to be:

1. Create a new object - >
2. Parse the main file, then all sub files, and append to the sub file list - >
3. go through the subfile list and create a new object for each subpart - > Step 1

It really needs to use functions, not while recursions, lists, and objects.

Banbury and I have already noted that. It needs to be rewritten to kill the (bad) recursion. It might still need a class, but there is a much better way to do all this, and the recursion is clearly hampering progress. I know we already have a few targets for v1.2, but I am going to suggest holding off on those for a rewrite in v1.2 (but I'll speak more on that later. No need to comment on this idea right now :wink:).

[le717]

Since this can't seem to be fully fixed right now, at least we could have some sort of error message thrown if this happens. I tried adding self.report to the area in an attempt to trigger the pop-up error box, but I was getting an AttributeError.

[JoshTheDerf]

Thats because self.report has to be called inside a class that extends bpy.types.Operator.

[le717]

Hm. I should be able to fix that. Will try either this afternoon or tonight, whenever I get the chance.

[le717]

I tried this, and was unable to do it. In the mean time, the import will simply stop with a message in the readme starting this bug.

[JoshTheDerf]

Still have this in my sights, been on vacation that past week.

[le717]

Very well, Tribex. If you think you can pull off skipping bricks with all these loops, make a branch off the master and attempt it. However, I still think a script rewrite is in order to pull this off and to allow us to add new features (like MPD and Bricksmith models, the latter of I get asked about a lot).

Hope you had a good vacation!

[JoshTheDerf]

I'm working on a script rewrite (and failing horribly), but thank you!

[le717]

We will all work on the rewrite, Tribex. I'll try to get a proposition for doing now rather then later posted within a few days. :wink: