Skip to content

AWTSettingsDialog: javadoc + add AppSettings Preference logging - #2478

Merged
yaRnMcDonuts merged 8 commits into
jMonkeyEngine:masterfrom
capdevon:capdevon-AppSettings
Jul 29, 2025
Merged

AWTSettingsDialog: javadoc + add AppSettings Preference logging#2478
yaRnMcDonuts merged 8 commits into
jMonkeyEngine:masterfrom
capdevon:capdevon-AppSettings

Conversation

@capdevon

@capdevon capdevon commented Jun 6, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

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
yaRnMcDonuts
yaRnMcDonuts reviewed Jun 7, 2025
View reviewed changes
Comment thread jme3-awt-dialogs/src/main/java/com/jme3/awt/AWTSettingsDialog.java
@capdevon

capdevon commented Jun 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

yaRnMcDonuts commented Jul 28, 2025

Copy link
Copy Markdown
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

github-actions Bot commented Jul 29, 2025

Copy link
Copy Markdown

🖼️ 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

yaRnMcDonuts commented Jul 29, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

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

Reviewers

@yaRnMcDonuts yaRnMcDonuts yaRnMcDonuts approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v3.9.0

Development

Successfully merging this pull request may close these issues.

2 participants

@capdevon @yaRnMcDonuts