New crate: bevy_user_prefs

[viridia]

Objective

• Add framework for managing user preferences on both desktop and WASM platforms.

Design doc: https://hackmd.io/@dreamertalin/rkhljFM7R

Testing

• Unit tests, manual testing on both desktop and wasm

Additional Information

This PR is likely to be highly controversial, it incorporates a number of design decisions which are likely to be contentious.

[Person-93]

Should we track changes per group (or even per setting) ? An App that doesn't autosave settings might want to indicate each unsaved change in its settings page.

[viridia]

This use case seems speculative. I try to avoid writing code based on hypothetical requirements, as I find this to be a major source of scope creep.

[Person-93]

This use case seems speculative.

It seems pretty standard to me. It's needed for games that offer players lots of knobs to twiddle.

[Person-93]

Logging might not be initialized when preferences are read. Perhaps this should return a result instead.

[viridia]

Does this mean that I can't call warn() in main? That would be very surprising to me.

[Person-93]

Does this mean that I can't call warn() in main? That would be very surprising to me.

That's correct. Logging events might be missed if they're emitted before the log plugin is added. I usually do something like this.

fn main() -> AppExit {
  let mut app = App::new();

  // setup logging early
  app.add_plugins(LogPlugin::default());

  pre_bevy_setup(); // this can use logging

  app.add_plugins(DefaultPlugins.disable::<LogPlugin>());

  app.run()
}

[viridia]

I'm still thinking about whether to return Result instead of Option on load.

Most of the time, users will not call load directly, but will call get which creates a new preferences file if it does not already exist. However, get will fail if the preferences_dir cannot be located - signaling that a future attempt to save preferences will also fail.

Applications will likely call get in many places: each individual subsystem which has configuration settings may call get individually. Some of these calls may happen in main, while others may happen within ECS systems. If the preference system is not functioning (because the preferences dir cannot be found) we probably don't want each individual subsystem reporting the same error.

I would rather have a failure of the preferences system be reported once; but this is hard to do because it gets called in many places, unless the error is handled within the preference system itself. Unfortunately, we don't have a lot of options here - an app shouldn't panic, because it can still run without preferences.

[Person-93]

What if you find/create the file in Preferences::new ? That seems like the right spot to report if there's something wrong with the whole file. Then Preferences::default can return a preferences table not backed by an actual file.

User code might look like this:

let prefs = Preferences::new("com.example.my-app").unwrap_or_else(|e| {
    // user can log the error, set an AtomicBool, or whatever else they want
    default()
});

If they don't care to do anything about the error, they can call unwrap_or_default.

[cart]

I think my big "problem" with this proposal is that it feels disconnected from Bevy / it is something that you "bind" to it, and that comes at the cost of features and clarity

I also think preferences / settings should be something that ultimately largely replace Plugin settings in their current form, and they should be strongly typed as the default interface into them (as in, they are defined and accessed as Rust types). I think type safety (as in a strong pairing between the "name" of a preference and its expected type) is extremely valuable, especially in cases like the enums. With your proposed system, if a preference is expected to be some enum PlayerTeam { Red, Blue } enum, the consumer of the preference needs to know (and successfully write out) the "player_team" string, and they need to know that it corresponds to the PlayerTeam enum when choosing how to interpret the raw untyped value assigned to "player_team".

I think something like the following API will make our users happier, significantly improve the UX, and feel more "integrated" with Bevy in general. As written, it relies on Resources as Components (currently in review, soon to be merged), Reflection, and a "SettingsPlugin" running immediately after the reflection type registry is initialized (but before plugins that use settings).

#[derive(SettingGroup, Reflect)]
#[settings_group(parent = AppSettings)]
struct PlayerSettings;

// A `Setting` is a Resource that is expressed as an "immutable component" to make change tracking bulletproof
// It also sets up an _on_insert_ component hook that detects when Team has changed in code and kicks off persistence
// to the settings file (if auto save is enabled)
#[derive(Setting, Reflect, Default)]
#[setting_group(PlayerSettings)]
#[reflect(Default)]
enum Team {
    #[default]
    Red,
    Blue,
}

#[derive(Setting, Reflect)]
#[setting_group(PlayerSettings)]
#[reflect(Default)]
struct Name(String);

impl Default for Name {
    fn default() -> Self {
        Name("Anonymous".into())
    }
}

struct PlayerPlugin;

impl Plugin for PlayerPlugin {
    // Note that this happens _after_ the settings have been initialized
    fn build(&self, app: &mut App) {
        match *app.resource::<Team>() {
            Team::Red => {/* do red things here */},
            Team::Blue => {/* do blue things here */},
        }
    }
}

fn main() {
    App::new()
        .insert_resource(AppSettings("my_studio.my_game"))
        .add_plugins((DefaultPlugins, PlayerPlugin))
        .add_systems(Update, read_team_setting)
        .add_observer(on_team_change)
        .run();
}

fn read_team_setting(team: Res<Team>) {
    println!("The currently configured team is {}", team);
}

fn on_team_change(team: On<Insert, Team>) {
    println!("This observer detected a team change: {}", team);
}

The serialized form stored in CONFIG_DIR/my_studio.my_game/app.toml
(only non-default settings are serialized)

[player]
team = "Red"

Representing settings as entities/components means they will be inspectable / integrate with whatever tooling we have there. If we make SettingGroup a resource too, we could then represent settings as relationship hierarchies (making them easily explorable / editable in inspectors).

An interesting side-effect of this approach is that we could in theory represent settings as BSN. (ex: Parent/child hierarchies of settings components). Although I'm not convinced theres a reason to do that.

This structured approach also allows us to detect cases where settings from different authors step on each other's toes. It also allows us to define migrations more effectively (if that is an area we or our users want to pursue).

There are some downsides with my proposal. We need to express each setting individually as a component, which introduces some boilerplate. It also creates "naming" questions, as Team could easily also be a normal component or resource type (decoupled from settings). Theres also the issue of discoverability (but frankly your proposal suffers from that to an even greater degree, by nature of having no source of truth for a given setting). I believe the naming and discoverability problems are largely resolved by using a setting submodule. We could also adopt a naming convention like NameSetting.

Being able to access settings individually by the type is also a double edged sword. It makes it low-boilerplate to read, but it also decouples it from the wider context (ex: in my example, to access Team you don't need to go through the PlayerSettings symbol at all). I think the ability to granularly react to a specific setting change in observers is well worth that price of admission.

[viridia]

There's one other downside to your approach, although maybe not super important: you might want to organize the properties in the settings file differently than they are organized in memory.

Take Minecraft for example: there's a settings page which has several different logical sections, one of which is "graphics". Within this page there are various settings such as "MSAA" and "Chunk Render Distance". From a user ergonomics standpoint, all of these are grouped together under the category of "graphics settings".

However, MSAA and Chunk Render Distance are very different, and are used by different parts of the graphics pipeline. While one could group all of the graphics-related settings together in a single struct, I can also imagine that a developer might not want to.

Again, I don't know how important this is, whether developers would feel significantly constrained by this.

[cart]

you might want to organize the properties in the settings file differently than they are organized in memory.

I'm not sure this critical. I think its worth pointing out that you can group settings in a UI however you want, so this really only applies to the file format, which is likely only going to be accessed by advanced users.

However with the "one resource per setting" approach, I think the "in memory organization" thing barely comes into play (as consumers of the setting consume it directly, rather than accessing it through some organizational hierarchy), so you can treat the "groupings" as the user-facing contract (which is the serialized contract, but it could in theory also be reflected in the UI).

[viridia]

Also, I'm not sure why you say that "resources as components" is necessary for this. I've prototyped this approach before, using reflection annotations (iter over all resources and scoop up the ones that have a prefs annotation). The piece I was missing - why I turned away from that approach - was that I didn't have the "initialize before build()" piece, which would have required mucking around with App::build; at the time I thought that be too controversial to try and get adopted. This resulted in preferences being loaded too late.

[cart]

The piece I was missing - why I turned away from that approach - was that I didn't have the "initialize before build()" piece

This has become a non-issue with the new-ish "automatic Reflect type registration", as plugins no longer need to register their types, and the types are available immediately after the TypeRegistry initializes.

Also, I'm not sure why you say that "resources as components" is necessary for this

I didn't mean to say that it was blocking, only that it was necessary to produce the code snippet I showed. "Resources as components" is what enables writing observers for each setting (and enables making those resources "immutable components" )