Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce valid frame rate cap values #4732

Conversation

kwvanderlinde
Copy link
Collaborator

@kwvanderlinde kwvanderlinde commented Mar 28, 2024

Identify the Bug or Feature request

Fixes #4730

Description of the Change

We no longer allow frame rates that aren't positive integers. This is enforced in two ways:

  • In the Preferences dialog, by validating that the value is positive. This is mainly done in the hopes that validation errors will one day be signaled to the user somehow, though today it is currently a silent failure.
  • In AppPreferences, by using the default value wherever an invalid value is encountered. This lets all components be able to rely on AppPreferences.getFrameRateCap() to return a useable value.

Possible Drawbacks

This can lead to confusion since invalid values are silently ignored in the Preferences dialog (this is already the case for invalid values in this field and others). The Preferences dialog deserves some TLC regarding validation.

Documentation Notes

N/A

Release Notes

  • Fixed a bug where it was possible to set invalid frame rates of zero or less.

This change is Reviewable

In `AppPreferences`, use the default value whenever an invalid frame rate cap is detected.

In the Preferences dialog, reject non-positive values with a `ParseException`. Though silent today, this is done in the
hopes that it will one day be signalled to the user somehow.
@Baaaaaz
Copy link

Baaaaaz commented Mar 28, 2024

https://wiki.rptools.info/index.php/MapTool_Preferences documentation could include a general note about ignoring invalid preference values perhaps? This may mitigate potential confusion noted under drawbacks above, well while we are awaiting the aspirational user validation error signaling feature!

Similarly https://wiki.rptools.info/index.php/MapTool_Preferences#Performance documentation could include a specific note about +ve integer valid values for Frame Rate Cap.

@cwisniew cwisniew added this pull request to the merge queue Mar 30, 2024
@cwisniew cwisniew added the bug label Mar 30, 2024
Merged via the queue into RPTools:develop with commit d6ffeea Mar 30, 2024
4 checks passed
@kwvanderlinde kwvanderlinde deleted the bugfix/4730-handle-non-positive-frame-rate branch March 30, 2024 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

[Bug]: Setting Frame Rate Cap to 0 frazzles MapTool
3 participants