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

Default settings tbl #6491

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

MjnMixael
Copy link
Contributor

@MjnMixael MjnMixael commented Dec 28, 2024

Implements an FR from the FOTG team; specifically it creates a default_settings.tbl that changes the default settings that various options will have before trying to load the player's values from the player file or INI file. Parsing is relatively straightforward.

Many thanks to @wookieejedi for testing and finding the right insert point during game initialization that this can be done safely.

The following example (not recommended for use) will set PostProcessing to default OFF instead of ON. Additionally it will enforce the value to always be false. More on that below.

+Option Key: Graphics.PostProcessing
+Value: false
+Enforce

The idea is to give game creators using FSO the ability to set reasonable default values for settings based on their mod's data. In some cases it may be ideal to entirely enforce an option for the game. The main example I have, though it won't work with this system for reasons described below, is SCHMUPSpace that required a specific resolution to run correctly.

+Enforce causes an option to be set to a specific value. The player's setting for that option, if it exists, will not be read and the option itself will be removed from the options system, effectively hidden from the player. As more and more games made for FSO are not FS and increasingly TC this allows a way for games to specify certain requirements for their games. I don't expect +Enforce to be used much but it's nice to have it there for game creators that really need it.

A couple additional notes:

  • In-Game Options must be enabled for this table to do anything.
  • Retail Built-In Options cannot be +Enforced mostly because they can't be removed from view and that's a poor player experience. A follow-up could get around that by disabling their UI elements in the retail Options UI with a popup warning that the value is enforced if someone really needed... but those options do so little anymore I doubt it's worth the effort.
  • Defaults are set by changing the global value that the option is tied to BEFORE user preferences are read from disk. If a user preference does not exist then the new default will, of course, be used instead. User preferences have higher priority always with the exception of +Enforce.
  • Options that handle hardware either cannot or should not be given a parser. In most cases they cannot because this runs before audio and graphics subsystems are initialized so there's no way to validate any potential values. But also.. I can't see a reason a game creator needs to set a particular monitor, audio device, or window mode. SCHMUPSpace provided a reason to require a specific resolution but there's no way to do that here, since we can't validate the resolution against available resolutions without the graphics subsystem. But also that feels dangerous. If a mod enforces a resolution that the player doesn't support then I'm not sure what happens. So we just won't add that capability since we can't very easily anyway.

Finally

This PR only adds the options in 2d.cpp to the default_settings.tbl parser for now to keep this PR smaller and more about the framework. A follow-up PR after this one is merged will handle the rest of the options. For a bit of documentation the method to do so is explained here.

An option using a parser must use default_func() in place of default_val() because the latter will create an internal func that returns a literal. In other words, with default_val(), the default value can never be changed. default_func() will grab whatever the most recent value is. Most cases this can be a small inline change with a lambda. From there you add a parser with .parser(your_parse_func). The parser assumes optional_string("+Value:") has already succeeded and is ready for stuff_whatever() and should follow up with appropriate value validation before setting the global variable the option is ultimately tied to.

As part of this enhancement, this PR also modernizes the Default Options structs and methods for better type safety and easier calling. I came across this need while creating a parser for lighting settings and will be used for the other settings as well in later PRs.

@wookieejedi wookieejedi added feature A totally new sort of functionality Requested by Active Mod A feature request that has been requested by a mod that is actively in development. labels Dec 28, 2024
@wookieejedi wookieejedi added this to the Release 25.0 milestone Dec 28, 2024

for (int i = 0; i < static_cast<int>(DefaultDetailLevel::Num_detail_levels); i++) {

if (value[i] < 0 || value[i] > static_cast<int>(DefaultDetailLevel::Num_detail_levels)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In mod_table.cpp there is the nearly same detail level check, though in that function the if condition is if (detail_level < 0 || detail_level > static_cast<int>(DefaultDetailLevel::Num_detail_levels) - 1) { with specifically Num_detail_levels) - 1.

I reckon this check should also use Num_detail_levels) - 1 since Num_detail_levels = 4 but the valid values are 0-3. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed in DMs, this revealed that the language between levels and values still isn't clear so I've now clarified between presets, levels, and values in a new commit. That commit also resolves this issue by getting the 4 from the correct define.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A totally new sort of functionality Requested by Active Mod A feature request that has been requested by a mod that is actively in development.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants