Skip to content

Conversation

@capdevon
Copy link
Contributor

@capdevon capdevon commented Jun 6, 2025

This PR focuses on a significant overhaul of the AWTSettingsDialog class, aiming to improve its maintainability, user experience, and overall stability.

Key changes include:

  • Comprehensive Javadoc: Vastly improved and expanded Javadoc for the class, its fields, methods, and nested interfaces, ensuring clarity and consistency for developers.
  • Enhanced Preference Debugging:
    Introduced AppSettings.printPreferences() calls during both the loading and saving phases of AppSettings within the dialog. This provides crucial visibility into the exact parameters being read from and written to the application's preference store, greatly aiding in debugging configuration issues.

These changes collectively lead to a more reliable, well-documented, and user-friendly settings dialog for jMonkeyEngine applications, with improved insights into preference management.

Refactor AppSettings: Add Robust Getters and printPreferences Logging

This PR enhances the AppSettings class by introducing more robust getter methods (getInteger, getBoolean, getString, getFloat) that now accept a default value for improved fault tolerance. Additionally, the printPreferences method has been added. These changes improve the usability and reliability of the AppSettings configuration system.

e.g.

com.jme3.awt.AWTSettingsDialog <init>
INFO: Loading AppSettings from PreferenceKey: jMonkeyEngine 3.9.0-SNAPSHOT
com.jme3.system.AppSettings printPreferences
INFO: Preferences for key: jMonkeyEngine 3.9.0-SNAPSHOT
 * I_DepthBits = 24
 * I_WindowWidth = -2147483648
 * I_MinHeight = 0
 * B_DisableJoysticks = true
 * B_SwapBuffers = true
 * B_Resizable = false
 * B_CenterWindow = true
 * I_Height = 480
 * I_BitsPerPixel = 24
 * I_Width = 640
 * B_UseRetinaFrameBuffer = false
 * B_UseInput = true
 * I_FrameRate = -1
 * S_Title = jMonkeyEngine 3.9.0-SNAPSHOT
 * B_GammaCorrection = true
 * S_AudioRenderer = LWJGL
 * I_WindowXPosition = 0
 * I_WindowHeight = -2147483648
 * I_WindowYPosition = 0
 * I_MinWidth = 0
 * B_GraphicsDebug = false
 * B_VSync = true
 * I_Samples = 0
 * I_StencilBits = 0
 * S_SettingsDialogImage = /com/jme3/app/Monkey.png
 * S_Renderer = LWJGL-OpenGL3
 * S_OpenCLPlatformChooser = com.jme3.opencl.DefaultPlatformChooser
 * I_Frequency = -1
 * B_Fullscreen = false
 * B_OpenCL = false
 * I_Display = 0

@yaRnMcDonuts yaRnMcDonuts added this to the v3.9.0 milestone Jun 7, 2025
@capdevon
Copy link
Contributor Author

capdevon commented Jun 7, 2025

Hi @yaRnMcDonuts , I have updated the class with the suggested changes.

Thanks again for your feedback.

@capdevon capdevon changed the title AWTSettingsDialog: Javadoc, UI, & Add AppSettings Preference Logging AWTSettingsDialog: javadoc + add AppSettings Preference logging Jun 13, 2025
@yaRnMcDonuts
Copy link
Member

I plan to merge this within the next few days; let me know if you want me to wait any longer.

Thanks for your work on upgrading the appSetting interface with this PR!

@github-actions
Copy link

🖼️ Screenshot tests have failed.

The purpose of these tests is to ensure that changes introduced in this PR don't break visual features. They are visual unit tests.

📄 Where to find the report:

  • Go to the (failed run) > Summary > Artifacts > screenshot-test-report
  • Download the zip and open jme3-screenshot-tests/build/reports/ScreenshotDiffReport.html

⚠️ If you didn't expect to change anything visual:
Fix your changes so the screenshot tests pass.

If you did mean to change things:
Review the replacement images in jme3-screenshot-tests/build/changed-images to make sure they really are improvements and then replace and commit the replacement images at jme3-screenshot-tests/src/test/resources.

If you are creating entirely new tests:
Find the new images in jme3-screenshot-tests/build/changed-images and commit the new images at jme3-screenshot-tests/src/test/resources.

Note; it is very important that the committed reference images are created on the build pipeline, locally created images are not reliable. Similarly tests will fail locally but you can look at the report to check they are "visually similar".

See https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-screenshot-tests/README.md for more information

Contact @richardTingle (aka richtea) for guidance if required

@yaRnMcDonuts
Copy link
Member

yaRnMcDonuts commented Jul 29, 2025

It looks like github made a mistake when generating the suggested code to solve the conflicts, and I fixed one thing but overlooked the other.

For some reason, github removed 2 lines of code: one with an opening comment character: /** and another line with a closing bracket character: } and I mistakenly didn't put the } back in. (I'm unsure why github's web interface is so bad for resolving even simple conflicts like this, but at least its resolved now)

@yaRnMcDonuts yaRnMcDonuts merged commit 1fc6230 into jMonkeyEngine:master Jul 29, 2025
16 checks passed
@capdevon capdevon deleted the capdevon-AppSettings branch July 29, 2025 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants