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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relax empty selection extraction #114

Closed
TheChocolateOre opened this issue Aug 16, 2023 · 14 comments
Closed

Relax empty selection extraction #114

TheChocolateOre opened this issue Aug 16, 2023 · 14 comments
Assignees
Labels

Comments

@TheChocolateOre
Copy link

Trying to export a selection with no blocks will result in a: Error: no solid blocks found; no file output. ... error

I propose relaxing it, with 2 options on the horizon:

  1. Export an empty Wavefront .obj.
  2. Silently ignore.

The benefit here is that we would preserve the flow of scripts, so they would not crash. Furthermore, I would argue that 1. is better than 2. because it will not break any invariants of the script (i.e. there will always be present an .obj for each export command).

@erich666
Copy link
Owner

Ah, right, for scripting. Could you do an experiment for me? Could you try using this option in your script, at the beginning:

Show error: no

Does this help? You should turn it back on ("yes") at the end of the script. This should suppress error messages and let the script continue. There are also warnings and informational bits you can turn off, see here.

It's not the greatest solution, but maybe is enough? I'm happy to add yet-another-option (probably as a hidden scripting-only option), but would like avoid doing so if I could. New options can add complexity, occasional weird interactions, and bugs.

@erich666 erich666 self-assigned this Aug 29, 2023
@TheChocolateOre
Copy link
Author

Setting the errors to no, effectively implements 2. It's a trick, but I'm happy with that.
The only concern here is that any other errors will pass under our nose.

@TheChocolateOre
Copy link
Author

Another problem with the "show no errors" solution is that it seems that Mineways doesn't close after script ends. I'm spawning Mineways from another script. Having the GUI persist is a minus.

@erich666
Copy link
Owner

erich666 commented Aug 30, 2023

If you want Mineways to close, try:

Close

as the final command in your script (well, you could put more commands after that, but they'll be ignored). That should do it, I hope. Documented here.

@TheChocolateOre
Copy link
Author

Yeah, I already have the Close command.
Anyways, consider this closed for me. As you said it's probably not worth it implemented something here.

@erich666
Copy link
Owner

erich666 commented Aug 31, 2023

OK, thanks for following up.

BTW, if the script itself has syntax errors, you can log these to a file instead of displaying them in a popup. See Save log file: - this isn't really want you want (you want errors that occur due to script execution). Hmmm, maybe I also add script execution errors into the log and suppress otherwise? What do you think? Save log file

@TheChocolateOre
Copy link
Author

Cause we are starting to bikeshed, what I ended up doing is:

  1. Let Mineways crash.
  2. Write down the world layer (height) it did on. (manual)
  3. Set the max height to: crash_layer_height - 1. (manual)
  4. Rerun Mineways.

That "solves" my problem, at the cost of adding a few back and forth manual steps.
That being said, at this point I'm ok with that, it's not a deal breaker, just an inconvenience.

Everything error-hide related solution to this problem is at most superficial, and will be taxing in the long run, at least in the end-site.

Closing, it all boils down to if you consider illegal or not the action of exporting empty space. There are also other "solutions" to this puzzler like, e.g. quering somehow Mineways (is it even possible to query?) the max block height of a selection. One could just then plug that in, and avoid altogether the philosophical dielema of if it's meaningful or not to export empty space.

Anyways, imho there is no need to implement something here, at least for now. If you insist doing so, I would diverge from error handling, and move towards a more fundamental construct. But again, this will move Mineways towards being an API, which might be more that it can chew.

@TheChocolateOre
Copy link
Author

TheChocolateOre commented Aug 31, 2023

After thought, at least for my pipeline, I converged to a solution of silently ignore. But, because other scripts might rely on Mineways throwing on attemps to export empty space, flipping that switch will render those scripts obsolete. So, a more backwards compatible solution here, would be a "Ignore exporting empty selection: yes" solution. But, again that will be 1 more tag to keep track, taxes, taxes...

@erich666
Copy link
Owner

erich666 commented Sep 1, 2023

taxes, taxes...

'xactly. I think you're the rare person who exports empty space on a regular basis. Most people (I think) are doing stuff at ground level, so every exported tile has something in it (even if it's just ocean). But, if this problem gets reported more, I figure something like "if you've suppressed errors AND you have a log file for scripting errors set, I'll assume you want all errors to go into that log file" seems like a reasonable solution. I've put that on my TODO list.

@TheChocolateOre
Copy link
Author

Don't stress here, if I really want this, I'll fork it.

@erich666
Copy link
Owner

erich666 commented Sep 8, 2023

OK, added - empty exports now throw a warning and should let things continue: https://github.com/erich666/Mineways/releases/tag/v11.03

@erich666 erich666 closed this as completed Sep 8, 2023
@TheChocolateOre
Copy link
Author

TheChocolateOre commented Sep 9, 2023

Ok, this just got exponentially worse.
First of all, I apologize in advance for the time I'm consuming here.
I don't want this to be personal tech support, but I will give here more context to understand what I'm doing.

I'm taking a selection of a width (the x axis) and b depth (the z axis) and making 1 block height (y axis) slices of it from the botton -64 all the way up. Therefore, there will be a max height level that will contain 0 blocks (empty space) at some point. From there on it's hell. For the average case, that's around y=120. Now, the max y is 319, do the math 319 - 120, that's how many warning pop-ups I got. Before, I had to set max y to 319, get a single error, just close and reset everything with the error - 1 max y. Now, I can't do that, cause I'm getting 1-2 100s of warnings. So, that blocked my script flow, for good.

Of, course it's mb, I should have given a little bit more context in advance, and as you said, this is just the rare case of me exporting empty space. I think your invariant here was that I only export empty space once, where the reality is I'm exporting more empty space than filled.

In hindsight, we rushed. Still, I do believe that, for the average case, not throwing an error is better for exporting empty space.

What can we do.
Well, I could just "binary search" the warnings by hand and mitigate somehow the problem, but it's a shame, cause the problem is now bigger than the previous one, and all we wanted is to eliminate it.
As for Mineways, probably leave it as is, cause this is just turning into a mirror of my workflow.

As a last note, not a request, going back, I still believe that, ignoring better captures the semantics of exporting empty space. That would immediately solve my problem. In my model, I don't see empty space as an exception or a null to throw or warn. You could see it as a type of block, air. You wouldn't throw trying to export a selection of solely pink wool, so why would you for solely air?

@erich666
Copy link
Owner

Did you try sending your error messages to a script log, e.g.:

Save log file: C:\Users\ehaines\Documents\script_error.log

If you do, and you suppress the warning messages, the script log will get the warnings and tell you which line(s) of your script had them. For example, I made a test where nothing was exported (all air) and in my script_error.log file I see:

Start processing script
  Sun Sep 10 10:03:29 2023

Run-time problem executing line 7: Warning: only air blocks found; no file output. If you see something on the map, you likely need to set the Depth slider near the top to 0, or tap the space bar for a reasonable guess.

With these line numbers you can look at your script and see which exports failed. In my case the bad export is:

Selection location: 0,55,0 to 10,300,10

I do see a small bug, however - I think it should say line 8, not line 7. I'll fix that.

I think it's important to issue a warning. Mineways will not create any files when there's nothing to export - that would be silly. "Air" is a special block, it generates no geometry, so all air means there's nothing to save to a file. Making an empty file for the user doesn't serve much purpose and seems misleading.

@TheChocolateOre
Copy link
Author

Solved.
Show warning: NO did the trick + a few more lines to check which .obj's' got exported.
Shoving warnings under the carpet is way better than burying all errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants