editor: Pick proper dir and filename in Save Map/Scenario As dialog

[soliton-]

Describe the desired feature

1. Enter editor
2. New Scenario
3. Save Scenario As
4. Type in some name and save
5. Save Map As
   Now you get the previous scenario filename in the dialog and if you do save you overwrite your scenario.

Similarly you get the map file name if you save as map first.

This is confusing and can lead to data loss if the user is not careful. Even if you change the filename you're saving to the wrong dir.

The save dialog should pick the proper dir to start with and even then it might be good to give an explicit warning if someone saves a map as .cfg or a scenario as .map and/or when they save them in the wrong dir (as much as the editor can tell, so just if the dir is named maps/scenarios). The wrong extension could perhaps even be completely forbidden.

[Wedge009]

On the directory aspect, #2807 seems related.

[babaissarkar]

@soliton- Please check if the latest commit in #8903 alleviates this somewhat?

The save dialog should pick the proper dir to start with and even then it might be good to give an explicit warning if someone saves a map as .cfg or a scenario as .map and/or when they save them in the wrong dir (as much as the editor can tell, so just if the dir is named maps/scenarios). The wrong extension could perhaps even be completely forbidden.

Working on this part. Forbidding the extension completely should be much easier. Plus I can use my validation textbox idea (a textbox which shows check/cross marks based on whether the input is valid or not).

[babaissarkar]

@soliton- Is b1d0a53 in #8903 enough to resolve this?

[soliton-]

Is input_name the whole path? I think safer would be to just check the directory that is saved into. Not that we get confused by a user with "map" somewhere in their name or something.

Does the user see some feedback on what is wrong? Can they override it?

Using the correct starting dir to save to is also important. I forgot if you already added that in a different commit.

[babaissarkar]

Is input_name the whole path? I think safer would be to just check the directory that is saved into. Not that we get confused by a user with "map" somewhere in their name or something.

input_name is the absolute, username-santized, cached path to the savefile.
Example : /home/USER/.local/share/wesnoth/1.19/data/add-ons/TestAddon/scenarios/test_scenario.cfg

I'll check the last directory.

Does the user see some feedback on what is wrong? Can they override it?

No. What the code does is check the (cached) path and if it is empty or has 'maps'/'scenarios', it opens the file dialog into the default directory (which is the addon's scenario/map directory) with a blank filename.

Using the correct starting dir to save to is also important. I forgot if you already added that in a different commit.

Save Map As opens into the addon's map directory (addon's scenarios directory for Save Scenario As) unless the user has already saved it somewhere else, in which case that directory is opened.

[soliton-]

Ok, to pick the proper dir to start with I was just thinking to use <addon dir>/maps for Save Map As and <addon dir>/scenarios for Save Scenario As.

The other logic would be to warn the user when they pick something that seems to make no sense. Like when they navigate to the scenarios dir from the Save Map As dialog, type in the file name and click Save. Or when they try to save a scenario in any dir with a .map extension. At that point there could be a warning dialog where they can save anyway though if they want.

[babaissarkar]

Ok, to pick the proper dir to start with I was just thinking to use <addon dir>/maps for Save Map As and <addon dir>/scenarios for Save Scenario As.

This already exists in my PR.

The other logic would be to warn the user when they pick something that seems to make no sense. Like when they navigate to the scenarios dir from the Save Map As dialog, type in the file name and click Save. Or when they try to save a scenario in any dir with a .map extension. At that point there could be a warning dialog where they can save anyway though if they want.

Okay, will work on this one.

[soliton-]

Ok, to pick the proper dir to start with I was just thinking to use <addon dir>/maps for Save Map As and <addon dir>/scenarios for Save Scenario As.

This already exists.

I assume you mean in your PR.

[CelticMinstrel]

Saving in the wrong directory should be allowed. Saving with the wrong extension should be forbidden.

[babaissarkar]

This is what I added in 9c65d1e :

The example is from Save Map As, but it also appears for Save Scenario As with the corresponding extension.

[babaissarkar]

692924b gives the user a warning the first they try to save a scenario in maps folder or vice versa. Pressing "Yes" saves anyway, "No" closes dialog.

[babaissarkar]

How do I deal with empty filenames? I see two approaches:

1. Disable the "Save" button until the user enters a non-empty filename.
2. Use auto generated filename, something like "UntitledMapDDMMYY.map"

file dialog seems to use a different design pattern than other dialogs which is confusing to me. It could have used post_show(), but it does it in a way that makes it really hard to tweak things there (at least for me).

@irydacea Any thoughts on why you used a different pattern in file_dialog that is different than other dialogs AFAICS?

[soliton-]

Disabling saving until a name is given sounds good to me. I don't see a reason to auto generate a name.

[babaissarkar]

Disabling saving until a name is given sounds good to me. I don't see a reason to auto generate a name.

I'm thinking about making the filename textbox something like this:

Is this fine?

[babaissarkar]

Commits 8eac378 and da83c9a :

Extension checking is now file dialog's responsibility (since it already had extension info), so I'll remove that from the editor. Editor will now only be responsible for checking map/scenario folders.

[babaissarkar]

Anything else needs to be done as part of this issue?

[soliton-]

The extension is enforced now or can be overridden by the user? I think besides .map also .mask should be allowed.

Otherwise I think this looks to be resolved if the code gets merged.

[babaissarkar]

The extension is enforced now or can be overridden by the user?

Cannot be overridden, so file cannot be saved without correct extension. (The code calling the file_dialog can choose to turn this on or off. If an extension is set on the file_dialog using its set_extension method, the validation turns on.)

I think besides .map also .mask should be allowed.

For which dialog? Save Map As only, or somewhere else?

[soliton-]

Yes, when saving maps .mask would be same as .map.

[babaissarkar]

Yes, when saving maps .mask would be same as .map.

Implemented in b3128ff. File Dialog now has support for multiple extensions (previous commit). Extension validation only done when saving files.