[BUG][EDITOR] Repeated dialog when style piece missing | Behaviour when opening?

[WillLem]

Note for NeoLemmix users: this bug is also present in the NeoLemmix Editor, and has been for a while. If I can get it resolved in the SLX Editor, I'll absolutely provide the fix for the NL Editor as well.



The problem

Steps to reproduce:

1 Find a level that has some pieces in it that are definitely not present in the styles folder (perhaps temporarily remove the level's style from the styles folder for testing)

2 Open this level in the Editor

A dialog similar to this one should pop up:



Any attempt to move past this dialog (either by clicking OK or [X]) results in the same dialog popping up again. You either have to reach for Windows Task Manager (), or if you have enough patience (and a rapid-fire button on your mouse!), you can repeatedly click it up to 40-50 times and it will eventually close, and the level will load.

This should clearly not happen. Ideally, the dialog disappears after a single click.

With that said - if the level is missing multiple pieces, this dialog appears per piece. So, even if a single click closes the dialog, this is still very inconvenient if you have to click through many missing pieces.

The fix

It's a very easy fix, thankfully. If we remove the code that causes the dialog to open and silently log the exception instead, it turns out that the Editor code already features a much better way of handling missing pieces; it shows a much more user-friendly dialog with a list of all missing pieces, and explains that the image files cannot be found:



Please note that I have already modified this dialog in the latest working copy of the SLX Editor, but the original version was essentially the same; I've simply added some line breaks for readability, and a Cancel button. The reason being, clicking "OK" closes the dialog and loads the level, albeit with the missing pieces; the user should also be able to cancel the loading of the level altogether at this point.

I do wonder, though - what is the most desirable behaviour here? Should the level load with missing pieces? This is not necessarily a good result; if the level is saved at that point, the missing pieces will have disappeared unrecoverably.

I suggest it would be far better to simply not load the level, instead preserving it as it whilst encouraging the user to gather the missing style pieces so that the level can be opened properly.

Offering the choice (as the working copy currently aims to do - I haven't quite finished it yet) seems like a happy medium, but maybe preserving the level file should be top priority here. I feel like an extra step should be necessary to open the level with missing pieces (an "are you sure?" dialog with a warning, perhaps), and cancelling loading the level should be an easier default.

Thoughts on this? Should it be a choice? Should the level never load with missing pieces? Should it always load with missing pieces?

One thing is for sure - the repeatedly-opening dialog is GONE, so the bug is fixed. We just need to figure out what is the best thing to do with the "missing pieces level" from that point.

[nin10doadict]

I was unaware that the level being loaded after the whole "missing piece" rigmarole was dealt with could result in you saving the level and having the missing pieces removed. I feel like that shouldn't happen. A missing piece should be displayed using some placeholder graphic (like a box with a red X or something). It could still be moved and saved that way, though if the level was loaded in the player and the piece was missing then it wouldn't actually be in the level (and the player should also throw out a warning that pieces are missing, which I think it does; it won't even boot the level if I recall).

In any case, making the warning only appear once in the player is definitely good. I didn't know you could even get past it without aborting the program at that point.

[WillLem]

I was unaware that the level being loaded after the whole "missing piece" rigmarole was dealt with could result in you saving the level and having the missing pieces removed. I feel like that shouldn't happen.

Glad to have some agreement to this

A missing piece should be displayed using some placeholder graphic (like a box with a red X or something). It could still be moved and saved that way

Not a bad idea, the only issue though is that if that the placeholder is moved, there's no way to know whether it's being moved correctly (i.e. the trigger area could end up in the wrong place, or the terrain could be offset by an important number of pixels).

Another idea may be to display generic placeholders, but prohibit editing of those pieces. This would at least allow the level to be opened for viewing purposes (and other pieces could be edited as normal).

Ultimately though, the simplest and most elegant solution IMHO is simply not to load the level. Perhaps a text file could be generated listing the missing pieces: that way, at least it makes it easier for the user to locate them.

[WillLem]

Added a poll.

Please note that loading the level with (a) placeholder piece(s) would be a lot more problematic and complicated than simply displaying an image of the level with placeholder pieces, due to the information that the Editor requires to load style pieces properly.

I'm not saying it's impossible, but it's not an option at present.

If you have any other suggestions, please share.

Thanks!

[WillLem]

OK, it's been agreed that refusing to load the level at all is a bit drastic.

Meanwhile, I've opted not to pursue placeholder graphics for 2 main reasons: 1) it defeats the purpose of opening the level in the Editor, since a generic placeholder cannot accurately represent the missing tile, 2) it's probably better practice to encourage users to obtain copies of the relevant styles, that way the level can be loaded properly.

So, the original bug is fixed - no repeating error dialogs, and no need to close the Editor using Windows Task Manager!

Instead, we get a single dialog listing all missing pieces, plus a handy MissingStylePieces.txt file which also lists them for easy reference.

Users are given the choice to open the level anyway (e.g. for incidental viewing purposes), but are warned that opening the level will delete the missing pieces. If they choose to go ahead and open anyway, the file name is automatically appended with "_MissingPieces" to prevent accidental overwriting of the original file in the event of a save.

After much consideration, I believe this to be the best way to handle loading and saving levels with missing tiles.

See commits 7c99170 and 425ba43

[mobius]

thank you for.finally adressing amd fixing this. This plagued me sooo much way back. I think thats the best option as well.

[WillLem]

thank you for.finally adressing amd fixing this. This plagued me sooo much way back. I think thats the best option as well.

Thanks mobius, glad you approve

...

The fix has introduced a new bug (which I'm in the process of fixing).

Since the check for missing pieces currently occurs after the level loading process has begun (and necessarily has to in order so that it knows which level to check), cancelling the loading at that point doesn't quite work.

Possible fix: check for missing pieces before loading. This will involve knowing which level to check, but not actually setting it as the value of CurLevel. Should be doable.

[WillLem]

OK, the Editor now checks for missing pieces before setting the value of CurLevel; this allows the loading method to be cancelled effectively.

See commit 8d0820a

[WillLem]

Debating whether or not to use dialogs at all for this...

A simple "This level is missing some pieces. See MissingPieces.txt for details. [OK]" Is probably all that's actually needed here. Users can click past this quickly - we can even add a bool for tracking if they ever want to see the popup again for subsequent levels with missing pieces.

After reading Simon's Blog on the subject of programmer-user communication, I'm wondering whether the Editor can support a status bar - much less intrusive and more likely to be read properly than a popup message. Will definitely be investigating this.

[WillLem]

OK, brilliant - we now have a status bar in the Editor!



We'll still show a popup listing the missing pieces as this is important information that the user might be able to fix quickly if they have access to the relevant styles. But, there's certainly no longer any need for an "are you sure?", and we might even want a "don't show this again" bool for the missing pieces dialog (maybe not, though, since the list will most likely be different for each level).

EDIT: This is remarkably versatile! I've now added a button to show a list of the missing pieces - with this in place, it may be possible to bypass dialogs entirely.

It's also probably a good idea to have a menu item & hotkey dedicated to showing missing pieces, then we can also bypass the need to generate a separate text file.

[Pieuw]

This is great! I encountered this problem a lot and I think it was the best possible way to fix it

[WillLem]

Commits b0ccdbff - a31aaa3 implement the following

The level validation procedure now generates a list of missing pieces to pass to a dialog which can be called up at any time by pressing F8 - this removes the need to display a popup or write the list to a separate text file.

The missing pieces are then deleted from the level file as usual. A unique filename is created to prevent overwriting, and a message is displayed in the new status bar informing the user. The status bar also includes a button to show the "missing pieces" dialog, and another to remove the status bar message if the user is finding it distracting.

We no longer need to cancel loading, so levels are always successfully loaded regardless of having missing items (no more popups!)

I believe this to be the best overall solution to this issue - with the added bonus that the status bar can potentially be used for other features (such as tooltips, etc), if needed in the future.