Load heightfield from file -> Cancel button handled incorrectly

Started by leafspring, May 05, 2010, 06:49:19 am

Previous topic - Next topic

leafspring

Did a search as always and couldn't find anything about it.

When I go to the Terrain tab, click on Add Terrain -> Heightfield (load file) and cancel the dialog, TG2 creates an 'empty' Heightfield shader and an error pops up that it could not locate the file.
Lang lang er vejen for Aslaug
Længe venter lykken på Kraka

Tangled-Universe

"Correct"...this is typical behaviour of every node which can either load imagemaps, models or terrains.
Whether it is desired behaviour or a bug, I don't know.

leafspring

Quote from: Tangled-Universe on May 05, 2010, 07:32:13 am
Whether it is desired behaviour or a bug, I don't know.

At least the thrown error (Could not locate file) cannot be desired since a cancel means that no file was selected.
Lang lang er vejen for Aslaug
Længe venter lykken på Kraka

Oshyan

Technically this is intended/expected behavior. The error message is after all correct and is automatically generated when it can't find the file (because none is specified in this case, but still an error condition for a file loading node). I suppose we could make an exception to the error system for that particular case, but it seems a bit hackish. Do you have a suggestion for better behavior? Perhaps file loading nodes simply shouldn't complain if *no* file is specified, only if an *invalid* file is path is specified?

- Oshyan

Dune

The same goes for loading an object, and after due consideration while doing so, you decide to cancel. There's still an 'empty' thing loaded. Just X the pop up.

---Dune 

leafspring

Quote from: Oshyan on May 05, 2010, 09:33:58 pmDo you have a suggestion for better behavior?

Well, 'better' is a relative term but what I'd consider more intuitive is that simply nothing happens, when the dialog is canceled since the user obviously changed his/her mind and I see no reason why anyone would want to cancel the loading of the necessary file but still keep an empty heightfield.

I think the actual problem is that this operation appears to be a single-step action while in reality it consists of creating a heightfield, creating a loader and loading a file. Canceling a single-step operation induces the expectation that nothing will happen while canceling only the loading operation in the creation process is expected to leave the first two steps untouched.
Lang lang er vejen for Aslaug
Længe venter lykken på Kraka

Tangled-Universe

I'd suggest:

If the user presses cancel, then don't create the node. The node is probably created prior to choosing the file, if so then TG2 would delete the created node.
If there's no file specified I'd suggest to don't give an error, just keep the node "empty".
If there's a wrong path/file, then pop up the error.

Martin

Oshyan

Yes, both your suggestions were my first thoughts too. The thing is A: no other node creation has a "cancel" option, the only reason this one could is because it depends on loading a file, but the cancel there is really for the file loading, not the node creation. So while it might make sense to people, it would not be "technically correct", B: I could see there being some possible reasons to have a Load-type node without actually specifying a file right-off, and certainly wouldn't want to make that impossible.

I think suppressing the error message is the best option, I just don't know how tricky that would be. It seems a simple thing, but it would basically be creating a special case for the error system, which is not always a good idea...

Something to ponder.

- Oshyan

jaf

I run into that usually when I've been working with obj files.  I get one looking like I want it and usually save it out as a tgo.  Later, I'll want to load it in a scene and I'll mistakenly choose obj, realize I want tgo, and get the error "ObjectLoader: Cannot locate file".  Well, of course it couldn't, I didn't select one.

Not sure why the object node needs to be created until a file has been actually selected and verified.  And since I can click on the object reader after the ObjectLoader error and hit delete and it's gone, why did it have to be created and error logged to begin with?

If I do a File|Load and select cancel, nothing happens.  I think that's what most of us expect.  Cancel (to me) doesn't mean the same as "Stop".

It's not a big deal -- just a little strange, at least to me.
(21nov19) Ryzen 1800x, 970 EVO 1TB NVME-M.2 SSD, Corsair Vengeance 64GB DDR4 3200 Mem,  EVGA GeForce GTX 1080 Ti FTW3 Graphics 441.20 (21nov19), Win 10 Pro x64, Terragen Pro 4.4.44, BenchMark 0:10:02

neuspadrin

Happens to me all the time when I go to load an object or tgo and forget which one... 50/50 chance of correctness ;), if wrong => cancel, repeat.

Oshyan

All this has to do with how things work internally. I agree it's not necessarily intuitive that canceling a file open dialog box is sort of equivalent to "stop" rather than acting like canceling a normal file menu Open command (i.e. nothing happens). The reason is that as you noted, the node gets created first (I'm not certain this is a required order of operations, but I would guess yes as the file open dialog needs to be called by something), and then the file loading dialog is triggered. Canceling the file loading dialog only cancels that portion of the process, and the node has already been created, just as any other node would be (and other nodes don't give the opportunity to cancel them). The file load dialog is just a standard dialog, so that's why it says "cancel" and not anything else; it is specifically referring only to the file loading portion of the operation.

Another option would be to remove the automatic file loading entirely and leave it up to the user to load something after node creation. Technically this would be more in keeping with how the vast majority of other node creation works, but it would likely be confusing for new users.

- Oshyan

jaf

Have no idea what the code looks like and there may be threading implications, but from a simplistic view, maybe reversing the order to 1) open and verify file exists and 2) create node and load.  If there's an exception at 1, throw an error message.  If there's a cancel, exit out.

This seems to be in an area of the workflow where there shouldn't be any rendering or speed advantages to get that node created, other than the preview (but that gets interrupted by user changes all the time.)

One other observation:  There is an undo (undo node creation) and it works, so it's difficult to see the necessity of having to create the node in the middle of the file selection process.
(21nov19) Ryzen 1800x, 970 EVO 1TB NVME-M.2 SSD, Corsair Vengeance 64GB DDR4 3200 Mem,  EVGA GeForce GTX 1080 Ti FTW3 Graphics 441.20 (21nov19), Win 10 Pro x64, Terragen Pro 4.4.44, BenchMark 0:10:02

Oshyan

As far as I know, the file selection is an action/property of the node, so theoretically you need to create the node first before you can call its load function. That being said again in theory it should be possible to pass a path to a previously selected file into the node as it's being created. But I'm not on the development side so everything I'm saying here might not be entirely accurate. ;)

I'll have dev take a look at the discussion here and weigh in with more info if possible.

- Oshyan

jaf

It's nice many of these discussions are not real bugs -- maybe they should be called "annoyances" or personal preferences.  The program is quite stable compared to most I use. ;D
(21nov19) Ryzen 1800x, 970 EVO 1TB NVME-M.2 SSD, Corsair Vengeance 64GB DDR4 3200 Mem,  EVGA GeForce GTX 1080 Ti FTW3 Graphics 441.20 (21nov19), Win 10 Pro x64, Terragen Pro 4.4.44, BenchMark 0:10:02

neuspadrin

Seems it wouldnt be too hard to do a

filename = fileChooser();

if (filename !=null || filename != "")
  load the file (with possibility of errors if file doesnt exist etc)
else
  node created, but empty fields

just roughly given various knowledges of how a lot of file pickers work :/  But could be a bit harder then that ;)