Move SettingsDialog and ErrorDialog in jme3-awt-dialogs module

[riccardobl]

This PR removes SettingsDialog and ErrorDialog from the internal code, since they do not work properly in some platforms when AWT is initialized in the same thread of GLFW. ErrorDialog also causes issues with mouse release on fullscreen apps in linux.

SettingsDialog is turned into a standalone class for developers that still wish to use it, by doing so we make it easier to replace with something else, eg. a configurator that takes settings from an external launcher with commandline arguments or environment variables

AppSettings settings=new AppSettings(true);
if(SettingsDialog.showDialog(settings)){
  app.setSettings(settings);
  app.start();
}

ErrorDialog is replaced by an error message handler that can be configured by the developer to show/log/send the error message in the most appropriate way.

Eg. reimplementing the ErrorDialog to reproduce the current behavior will look like this:

JmeSystem.setErrorMessageHandler((message)->{
   ErrorDialog.showDialog(message);
});

By default on desktop ErrorDialog is not used and the error is only printed to System.err. On android the behavior remains the same since the current error reporting is reimplemented using a default error message handler.

[riccardobl]

If there is no objection i think this can be merged

[stephengold]

I'd like more time to study this please.

[stephengold]

This would be much easier to review if it were split into 3 PRs: one for the native libraries, one for the settings dialog, and one for the error dialog. Would you mind splitting it up please?

[stephengold]

Also, the dialog changes have major impact so I think they should be discussed at the Hub/Forum before this is integrated.

[riccardobl]

I'll remove the native patch from this PRs and commit it directly, since it is a trivial change of the build script. Error and Settings dialogs are part of the same problem and prevent the engine from working in MacOS and they are already split in different commits.

I don't see this as a major change, since the same functionality is provided with SettingsDialog.showDialog(settings) and ErrorDialog.showDialog(message), but i'll open a thread

[stephengold]

I notice that the "Show Setting" checkbox in the test chooser no longer works. Please fix that.

I'll remove the native patch from this PRs and commit it directly, since it is a trivial change of the build script.

Thanks.

Error and Settings dialogs are part of the same problem and prevent the engine from working in MacOS and they are already split in different commits.

OK, I'll try viewing them that way.

I don't see this as a major change, since the same functionality is provided with SettingsDialog.showDialog(settings) and ErrorDialog.showDialog(message)

I disagree. If you won't start the discussion, then I will. But I imagine you could explain the changes better than I.

[riccardobl]

I notice that the "Show Setting" checkbox in the test chooser no longer works. Please fix that.

Should be fixed now

I disagree. If you won't start the discussion, then I will. But I imagine you could explain the changes better than I.

I opened a thread here

[stephengold]

Should be fixed now

Tested and confirmed.

I opened a thread here

Looks good. Thank you.

[stephengold]

I got distracted today and didn't have time to study this PR in depth. Hopefully tomorrow...

[stephengold]

I don't understand why the SKIP_SETTINGS variables were removed from TestIssue1120 and TestGImpactShape. How do those changes relate to this PR?

[riccardobl]

Isn't SKIP SETTING used to skip the setting dialog there?

[stephengold]

Isn't SKIP SETTING used to skip the setting dialog there?

Yes. And?

[riccardobl]

If the settings dialog is removed it cannot unskip it

[stephengold]

Okay, the removals make sense then.

I'm trying to run apps in jme3-examples on Linux (this branch of JME) and every test so far has failed. Here's a typical crash log:

> Task :jme3-examples:run
Jan 19, 2022 9:31:30 PM com.jme3.system.JmeDesktopSystem initialize
INFO: Running on jMonkeyEngine 3.1.0-SNAPSHOT
 * Branch: macNatives
 * Git Hash: f6d099e
 * Build Date: 2022-01-19
Jan 19, 2022 9:31:30 PM com.jme3.app.LegacyApplication handleError
SEVERE: Failed to create display
java.lang.RuntimeException: Unable to find fullscreen display mode matching settings
	at com.jme3.system.lwjgl.LwjglDisplay.createContext(LwjglDisplay.java:80)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.initInThread(LwjglAbstractDisplay.java:120)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:221)
	at java.lang.Thread.run(Thread.java:748)

[JME ERROR] Failed to create display
RuntimeException: Unable to find fullscreen display mode matching settings
Jan 19, 2022 9:31:30 PM com.jme3.app.LegacyApplication handleError
SEVERE: Uncaught exception thrown in Thread[jME3 Main,6,main]
java.lang.IllegalMonitorStateException
	at java.lang.Object.notifyAll(Native Method)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.initInThread(LwjglAbstractDisplay.java:135)
	at com.jme3.system.lwjgl.LwjglAbstractDisplay.run(LwjglAbstractDisplay.java:221)
	at java.lang.Thread.run(Thread.java:748)

[JME ERROR] Uncaught exception thrown in Thread[jME3 Main,6,main]
IllegalMonitorStateException

Exception: java.lang.NullPointerException thrown from the UncaughtExceptionHandler in thread "jME3 Main"

[Ali-RS]

Running on jMonkeyEngine 3.1.0-SNAPSHOT

Why 3.1?

[stephengold]

Why 3.1?

I noticed that, too. I suppose that might be because Riccardo's fork lacks any recent build tags:

sgold:~/Git/riccardo/jmonkeyengine$ git tag
3.1-github-tag-test
3.1-snapshot-test
3.1-snapshot-test-2
3.1-snapshot-test-3
3.1-snapshot-test-4
test123
test124
v3.1-alpha1
v3.1.0-alpha2
v3.1.0-alpha3
v3.1.0-alpha4
sgold:~/Git/riccardo/jmonkeyengine$

The branch of "macNatives" from "master" took place less than a week ago.

[riccardobl]

Somehow it seems the window is launcher fullscreen

[stephengold]

I'm interested in writing a new version of Settings Dialog that doesn't use AWT/Swing.

@riccardobl: Instead of removing/disabling the dialogs, would you consider revising this PR so it's not a breaking change? In other words, add the hooks to replace Settings Dialog and Error Dialog with alternatives, but keep the current dialogs as the defaults on all platforms.

[riccardobl]

I am planning to move the dialogs on a jme3-awt-dialogs module that can be replaced with a different module for different platforms or left out for no dialogs. Would that work?

[stephengold]

I think it would.

[riccardobl]

Done. I've moved the awt dialogs into the optional jme3-awt-dialogs module and added a "settings handler" in JmeSystemDelegate to handle the initial configurations.

Both error and settings handlers will search by default the classcom.jme3.system.JmeDialogsFactoryImpl that implements JmeDialogsFactory. If found they will call JmeDialogsFactoryImpl.showErrorDialog() and JmeDialogsFactoryImpl.showSettingsDialog(), if not found the settings handler will do nothing and the error handler will print the error message to System.err.

I've implemented the JmeDialogsFactoryImpl in jme3-awt-dialogs to keep the current behavior when the new module jme3-awt-dialogs is included as dependency. To implement new dialogs the process is to create a new module (eg. jme3-fx-dialogs) and the class com/jme3/system/JmeDialogsFactoryImpl that implements JmeDialogsFactory inside the module.

To reduce the complexity and to have the entire logic selfcontained into the default handlers and the module i made so that a new instance of JmeDialogsFactoryImpl is created everytime a dialog is shown, it shouldn't be a problem, but if proven otherwise i will figure out a different approach.

[riccardobl]

I'm interested in writing a new version of Settings Dialog that doesn't use AWT/Swing.

@stephengold let me know if this meets the requirements for the new dialogs

[stephengold]

I'll study it. Thanks!

[stephengold]

I've begun studying this PR. So many levels of indirection!

I understand correctly, I can create a new library with its own version of com.jme3.system.JmeDialogsFactoryImpl, and applications including that library will automatically use it to generate settings dialogs and error dialogs as needed. That seems sufficient for what I'm interested in doing.

By the way, something I noticed: SimpleApplication.start() is still invoking the deprecated showSettingsDialog() method in JmeSystem. I guess it ought to directly invoke handleSettings() instead?

[riccardobl]

I understand correctly, I can create a new library with its own version of com.jme3.system.JmeDialogsFactoryImpl, and applications including that library will automatically use it to generate settings dialogs and error dialogs as needed. That seems sufficient for what I'm interested in doing.

Yes, that's correct.

By the way, something I noticed: SimpleApplication.start() is still invoking the deprecated showSettingsDialog() method in JmeSystem. I guess it ought to directly invoke handleSettings() instead?

Yes handleSettings is the new method.

If you are happy with this, i will fix the conflicts and remove the deprecated calls so that it can be merged.

[stephengold]

In my opinion, this PR isn't ready to be merged yet. At the very least, it needs javadoc and reformatting.

[riccardobl]

Added the missing documentation. Should i refactor this to use a system similar to what @Ali-RS described in this thread https://hub.jmonkeyengine.org/t/buffer-allocator-implementation-set-by-lwjglcontext-gets-ignored-sometimes/45594/9?u=riccardoblb ? I.e. using config files

[Ali-RS]

I.e. using config files

IMHO, the current implementation is fine, I believe no need to use a config file in this case.

[stephengold]

I'd like to integrate this PR. However I can't figure out how to do so using the mechanisms built into GitHub. My plan is to integrate the diffs into a new branch in the main repo, but I'm open to other suggestions on how to proceed.

[MeFisto94]

The alternative would be to locally rebase the macNatives branch onto master and then force-push (or merge master into that branch and regularly push). Do not copy over the files, though, as that conflicts may be meaningful.