Default settings changed in game not applied to local settings storage

[MaxCWhitehead]

Description

IWhen updating build on steamdeck, there is Ragdoll control in settings, but it is set to None. It seems that engine does not handle missing fields in settings when new ones are added. Maybe we need to differentiate specifically between missing input in stored settings and write default, and a none input that was set by player. (Explicitly set none in settings?)

Or we could have a default settings file, and user overrides, and overlay it, that way when game runs it always updates default, to make sure changes are applied correctly.

EDIT: I think the most right thing to do is probably some sort of patch function, overlay/merge could work if just adding stuff, but if renaming, wouldn't cut it.

Workaround:

EDIT: I found that we have a handy dandy reset button in settings, player can hit reset to get default controls, slightly inconvenient but ok for now.

To Reproduce

Not entirely sure - but something like install a old build of game pre-ragdoll, run it, then run ragdoll version, see if ragdoll control setting is set to default or set to none.

EDIT: Also seem to be able to delete the ragdoll lines from storage file and the reboot to simulate this.

Expected Behavior

Settings should reflect default unless overridden by user.

Additional Context

No response

Log Messages

No response

[nelson137]

I've been giving this issue some thought,,,

Maybe we need to differentiate specifically between missing input in stored settings and write default, and a none input that was set by player.

I agree, I'm thinking we will need to wrap each leaf InputKind of PlayerControlSetting in Maybe<_> that way any missing value in the user's settings can be easily detected and overwritten with the default from GameMeta. This allows us to differentiate between our 3 possible states of a mapping from the user's settings: Set(_) (mapping explicitly set by user), Set(InputKind::None) (mapping explicitly cleared by user), and Unset (mapping not changed by user, fallback to default). We can even add a "Reset to default" button to the popup for setting a mapping to make it Unset:

Though this makes accessing mappings a lot more cumbersome since everything is now a Maybe which we will have to resolve to a value at some point.

We could use some kind of helper function/proc-macro to resolve a mapping. It would take the optional user value and the optional default (which is also a Maybe because Settings is used for both the user settings and GameMeta.default_settings) and return the resolved InputKind. The default should always have a value but if neither that nor the user mapping has a value it can panic or use InputKind::None. Though this still means we will need the defaults everywhere we access a mapping. That's not difficult, and we don't even access mappings in many places, but is still annoying.

One way we could potentially improve on this is a system param which has references to PlayerControlSetting and the defaults from GameMeta and expose a getter to resolve each mapping. I think a proc-macro would be very helpful here for code generation. Then we can make the fields of PlayerControlSetting private to ensure the getters are used. But again, we don't use mapping in many places so this could be a future improvement.

I also considered having a second version of PlayerControlSetting that has no optional values, only InputKinds (like it is now). We would resolve all of the mappings into this struct once on startup and each time the user changes & saves mappings, rather than resolving on-demand for each mapping when we need it; but I think this would be more difficult and I'm not sure if there's much advantage over the proposal above.

[MaxCWhitehead]

@nelson137 What if we had two PlayerControlSetting types? One is the serialized state, and one that is used in gameplay.

The thought is: The serialized one uses Maybe like you mentioned, so that if is Some(InputKind::None), the binding was explicitly cleared by user, and must be serialized that way to override default. If None, the player never mutated it, should load from default. So to the file, we only serialize the Some values, the player changes.

On load, we start by loading default, and overlay the Some values from serialized settings. This would load into a type that does not use Maybe, as after this load/overlay, we know have concrete settings for all bindings, either from default or user change. Then we use this type in gameplay?

PlayerControlMapping/PlayerControlSettings are currently a resource, would need to split out a true settings type that can be loaded and converted into this?

This might be tricky/annoying to do - but are some thoughts.

[nelson137]

Yeah, totally. I kept flip-flopping between that idea -- resolving mappings up front into a struct -- and the one I fleshed out more in the comment above -- resolving mappings on-demand with a helper. I like the former because accessing mappings stays easy. I think my one issue with that idea was having to maintain 2 struct definitions (even with the help of a proc macro), but I'm now realizing that making PlayerControlMapping generic (as well as PlayerControlSetting and VirtualDPad) could fix that:

//  For (de)serializing the user config
type UserConfigControlMappings = PlayerControlMapping<Maybe<InputKind>>;

// For the resolved config
type ResolvedControlMappings = PlayerControlMapping<InputKind>;

(open to ideas on names lol). Luckily InputKind is the only leaf value type we use so we will only need the 1 generic param for everything, otherwise this could get messy.

It sounds like we're on the same page, at least in terms of how to reconcile user mappings and their defaults, so I'll start working in that direction.

[MaxCWhitehead]

gotcha - yeah I think the ideas are good. My only real concern is making sure that when using the mapping in gameplay code that it still feels similar / doesn't increase in complexity. I think it is sounding like that isn't a concern. Thanks for looking at this!