LegendaryExplorer icon indicating copy to clipboard operation
LegendaryExplorer copied to clipboard

Conditionals Editor Add New Conditional dialog is globally modal

Open levicki opened this issue 1 year ago • 8 comments

I tested this in the latest nightly of Legendary Explorer.

When you click on Add New Conditional option in Conditionals Editor it shows a modal dialog to enter plot ID. While that dialog is visible all other Legendary Explorer windows regardless of the tool (i.e. Plot Database, Package Editor, etc) are impossible to interact with so you cannot for example go to plot database to copy the ID.

I hope that's not by design and that it can be fixed.

levicki avatar Sep 02 '22 12:09 levicki

If I recall, in Java swing land at least, you could set up modality settings for global /window chain / document. We use .ShowDialog() in C# which I am pretty sure uses global. I am curious if it supports window, it may be a good idea to review them as we have a lot of modal dialogs.

On Fri, Sep 2, 2022, 6:42 AM Igor Levicki @.***> wrote:

I tested this in the latest nightly of Legendary Explorer.

When you click on Add New Conditional option in Conditionals Editor it shows a modal dialog to enter plot ID. While that dialog is visible all other Legendary Explorer windows regardless of the tool (i.e. Plot Database, Package Editor, etc) are impossible to interact with so you cannot for example go to plot database to copy the ID.

I hope that's not by design and that it can be fixed.

— Reply to this email directly, view it on GitHub https://github.com/ME3Tweaks/LegendaryExplorer/issues/337, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAU4VFF4ZKJFQKGZ5M6QL3LV4HY4HANCNFSM6AAAAAAQDHY4II . You are receiving this because you are subscribed to this thread.Message ID: @.***>

Mgamerz avatar Sep 02 '22 14:09 Mgamerz

It looks like to make it not global it must set a parent property. We should review the toolset and set parents on modals dialogs (MessageBox.Show() and uses of .ShowDialog(), where appropriate). I'll fix this one in conditionals editor.

Mgamerz avatar Sep 02 '22 14:09 Mgamerz

Actually that doesn't seem to actually work. https://docs.microsoft.com/en-us/dotnet/desktop/wpf/windows/dialog-boxes-overview?view=netdesktop-6.0

It is due to PromptDialog using .ShowDialog(). For this to be non-app modal we would probably need to use a listener for the closing event and disable the calling window. Not exactly sure how that'd work but wouldn't be a bad idea for some of our dialogs like listdialog.

Mgamerz avatar Sep 02 '22 15:09 Mgamerz

@Mgamerz

Generally modal dialogs should be avoided whenever possible in application design, especially when you are not using them for confirmation of potentially destructive operations.

In this particular case it is used to display a dialog for data entry and being modal is absolutely not necessary — the only thing needed is to disable the menu item that invokes the dialog before showing it and re-enable it afterwards so it can't be launched twice.

Frankly, I was surprised when I first discovered that LegendaryExplorer is a single application with multiple windows and that closing the Legendary Explorer also closed say the Package Editor which I had open.

I was expecting that all those tools would be separate executables which are launched (and updated) from a main application which could then be closed to reduce the clutter while keeping individual tools open.

In my opinion, separate executables for each tool would work better and it would also allow for those executables to register file types they handle and get launched when you click those files and to have command line options so they can be used from command line (or from a batch file) to accomplish some simpler tasks.

As for the Conditionals Editor, I think that its functionality should be streamlined into Plot Database Editor. For example, user could select (using checkboxes which should be added to each row in the right pane) all plot variables they are interested in overriding and then click "Create new CND file from selected" which would be shown in a window similar to Conditionals Editor where you could change the values, add new ids not in the database if necessary or delete ones you mistakenly selected, and save the resulting CND file.

levicki avatar Sep 02 '22 17:09 levicki

Tools would not work as separate executables as we use an interop system to share files in memory across multiple tools to keep them in sync. Splitting to executables would add a ton of overhead for development.

Mgamerz avatar Sep 02 '22 18:09 Mgamerz

Tools would not work as separate executables as we use an interop system to share files in memory across multiple tools to keep them in sync. Splitting to executables would add a ton of overhead for development.

I see... would it be too inconvenient to share them using named shared memory or perhaps even named pipes (.net implementation)? Or would the be too OS-specific?

Could you point me to the source code files with current implementation so I can take a look?

levicki avatar Sep 02 '22 19:09 levicki

This is a windows x64 only tool, so OS-specificity is not a concern. But yes, it would be an enormous increase in complexity to have every single tool be its own process, and in my opinion, would offer no discernable benefit.

allow for those executables to register file types they handle and get launched when you click those files and to have command line options so they can be used from command line (or from a batch file) to accomplish some simpler tasks.

This is already possible and does not require separate executables. In LEX's settings you can associate all the filetypes that LEX can open (see attached image), and they will open in the appropriate tool. image LEX also already has command line arguments. There's not very many implemented at the moment, but it would be fairly trivial to add more if there was a compelling use case.

Could you point me to the source code files with current implementation so I can take a look?

The basic idea is that all tools that have the same pcc open are all using the same IMEPackage object. All IMEPackages are opened through MEPackageHandler, which keeps a dictionary of open packages, keyed on the filePath. When you try to open a new package, it either opens it from disk, or returns the already open one. The package itself keeps track of what users it has, and notifies them all any time changes are made to it. In this way, edits made in one tool are immediately reflected in other tools.

SirCxyrtyx avatar Sep 02 '22 19:09 SirCxyrtyx

@SirCxyrtyx Thanks for explaining, I skimmed the code but it is a lot to take at once, I will certainly study it in more detail. From a quick glance I take it that this MEPackageHandler does not apply for tools that work with file types other than the .pcc, right?

Well I guess you are right, the out-of-process package server (i.e. a COM server in an .exe file) with MEPackageHandler class which is used by creating an instance through its interface from the client processes would be an overkill for this.

The separate apps / processes (regardless of the exact implementation of inter-process communication) would solve the problem with windows and dialogs as well as with accidentally closing everything without warning if you close Legendary Explorer itself though.

Edit: Sorry @SirCxyrtyx, I mistakenly tagged @Mgamerz instead of you.

levicki avatar Sep 03 '22 03:09 levicki