Skip to content

Commit

Permalink
ui: Add inheritance between Settingses.
Browse files Browse the repository at this point in the history
This will help us not persist changes that shouldn’t be, and later
I intend to add per-setting inheritance that will actually be useful to
the user.
  • Loading branch information
kpreid committed Jan 11, 2025
1 parent a5aba5d commit b5eda10
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 14 deletions.
6 changes: 4 additions & 2 deletions all-is-cubes-ui/src/apps/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -882,10 +882,12 @@ impl<I: time::Instant> SessionBuilder<I> {

/// Enable reading and writing user settings.
///
/// The session will get a new [`Settings`] object which inherits the given settings.
///
/// If this is not called, then the session will have all default settings,
/// and they will not be persisted.
pub fn settings(mut self, settings: Settings) -> Self {
self.settings = Some(settings);
pub fn settings_from(mut self, settings: Settings) -> Self {
self.settings = Some(Settings::inherit(settings));
self
}

Expand Down
140 changes: 128 additions & 12 deletions all-is-cubes-ui/src/apps/settings.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use alloc::sync::Arc;
use core::fmt;

use all_is_cubes::listen;
use all_is_cubes::listen::{self, Source as _};
use all_is_cubes_render::camera::GraphicsOptions;

/// Currently, the settings data is *only* graphics options.
Expand All @@ -16,8 +16,7 @@ type Data = Arc<GraphicsOptions>;
///
/// This is separate from [`Session`](super::Session) so that it can be shared among
/// multiple sessions without conflict.
/// All such sessions will edit the same settings.
// TODO: Add settings inheritance for session-specific settings.
/// All such sessions can edit the same settings.
///
/// Having `&` access to a [`Settings`] grants permission to read the settings, follow
/// changes to the settings, and write the settings.
Expand All @@ -27,14 +26,23 @@ type Data = Arc<GraphicsOptions>;
pub struct Settings(Arc<Inner>);

struct Inner {
data: listen::Cell<Data>,
/// If None, then use the hardcoded default.
inherit: Option<Settings>,

/// If `None`, then inherited or default.
/// TODO: should have individual settings be individual cells with individual inheritance.
state: listen::Cell<Option<Data>>,

/// Source which returns the [`Data`] that is stored *or* inherited.
final_data_source: listen::DynSource<Data>,

persister: Arc<dyn Fn(&Data) + Send + Sync>,
}

impl Settings {
/// Creates a new [`Settings`] with the given initial state, and no persistence.
pub fn new(initial_state: Data) -> Self {
Self::with_persistence(initial_state, Arc::new(|_| {}))
Self::new_general(None, Some(initial_state), Arc::new(|_| {}))
}

/// Creates a new [`Settings`] with the given initial state,
Expand All @@ -46,21 +54,54 @@ impl Settings {
initial_state: Data,
persister: Arc<dyn Fn(&Data) + Send + Sync>,
) -> Self {
Self::new_general(None, Some(initial_state), persister)
}

/// Creates a new [`Settings`] which reads and writes the given [`Settings`],
/// until such time as it is explicitly detached.
pub fn inherit(other: Settings) -> Self {
Self::new_general(Some(other), None, Arc::new(|_| {}))
}

fn new_general(
inherit: Option<Settings>,
initial_state: Option<Data>,
persister: Arc<dyn Fn(&Data) + Send + Sync>,
) -> Self {
let state = listen::Cell::new(initial_state);
Self(Arc::new(Inner {
data: listen::Cell::new(initial_state),
final_data_source: Arc::new(
state
.as_source()
.map({
let inherit = inherit.clone();
move |data: Option<Data>| -> listen::DynSource<Data> {
match (data, &inherit) {
(Some(data), _) => listen::constant(data),
(None, Some(settings)) => settings.as_source(),
(None, None) => {
listen::constant(Arc::new(GraphicsOptions::default()))
}
}
}
})
.flatten(),
),
inherit,
state,
persister,
}))
}

/// Returns a [`listen::Source`] of the settings.
/// This may be used to follow changes to the settings.
pub fn as_source(&self) -> listen::DynSource<Data> {
self.0.data.as_source()
self.0.final_data_source.clone()
}

/// Returns the current graphics options.
pub fn get_graphics_options(&self) -> Arc<GraphicsOptions> {
self.0.data.get()
self.as_source().get()
}

/// Overwrites the graphics options.
Expand All @@ -75,21 +116,43 @@ impl Settings {
// TODO: Fix that (will require support from ListenableCell...compare-and-swap?)
#[doc(hidden)]
pub fn mutate_graphics_options(&self, f: impl FnOnce(&mut GraphicsOptions)) {
let mut options: Arc<GraphicsOptions> = self.0.data.get();
let mut options: Arc<GraphicsOptions> = self.get_graphics_options();
f(Arc::make_mut(&mut options));
self.set_state(options);
}

/// If this `Settings` was constructed to share another’s state using
/// [`inherit()`](Self::inherit), make it stop.
/// This means future changes will not be persisted.
pub fn disinherit(&self) {
// TODO: this should be an atomic transaction but is not
self.0
.state
.set(Some(self.0.state.get().unwrap_or_default()));
}

fn set_state(&self, state: Data) {
(self.0.persister)(&state);
self.0.data.set(state);
// TODO: kludge: this condition is only vaguely reasonable because state never transitions
// from Some to None. In reality, we should be using our own mutex instead of depending on
// listen::Cell.
if let Some(parent) = self
.0
.inherit
.as_ref()
.filter(|_| self.0.state.get().is_none())
{
parent.set_state(state);
} else {
(self.0.persister)(&state);
self.0.state.set(Some(state));
}
}
}

impl fmt::Debug for Settings {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("Settings")
.field("data", &self.0.data)
.field("state", &self.0.state)
// can't print persister
.finish_non_exhaustive()
}
Expand All @@ -100,3 +163,56 @@ impl Default for Settings {
Self::new(Default::default())
}
}

#[cfg(test)]
mod tests {
use super::*;
use all_is_cubes::math::ps64;

#[test]
fn disinherit() {
let parent = Settings::new(Arc::new({
let mut g = GraphicsOptions::default();
g.fov_y = ps64(20.0);
g
}));
let child = Settings::inherit(parent.clone());

// Inherited values before child
assert_eq!(
(
parent.get_graphics_options().fov_y,
child.get_graphics_options().fov_y
),
(ps64(20.0), ps64(20.0))
);
assert_eq!(
child.get_graphics_options().show_ui,
true,
"original value for show_ui"
);

// Writing upward and reading downward
child.mutate_graphics_options(|g| g.show_ui = false);
assert_eq!(
(
parent.get_graphics_options().show_ui,
child.get_graphics_options().show_ui
),
(false, false),
"parent and child mutated"
);

// Then it doesn't any more
child.disinherit();
child.mutate_graphics_options(|g| g.fov_y = ps64(10.0));
assert_eq!(
(
parent.get_graphics_options().fov_y,
child.get_graphics_options().fov_y
),
(ps64(20.0), ps64(10.0)),
"after disinherit"
);
}
}

0 comments on commit b5eda10

Please sign in to comment.