-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat: Implement storage_creator
for native_options
#7143
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
base: main
Are you sure you want to change the base?
feat: Implement storage_creator
for native_options
#7143
Conversation
- Implement a `storage_creator` field for the user to create their own custom implementation of persistent storage
Few discussion points on the implementation:
|
Marked for review, looking for feedback on the above |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Makes sense to me.
Should persistence_path and storage_creator be merged or kept separate?
I think it makes sense to keep it separate for convenience.
Can we remove the dependency on ron since users implement their own storage logic?
Should we require developers to choose between storage_creator and persistence_path to allow for optional exclusion of file_storage.rs from the binary?
I think we could add a persistence-ron and/or persistence-file feature flag that is enabled by default.
Preview available at https://egui-pr-preview.github.io/pr/7143-custom-persistence |
@lucasmerlin Small road block I need feedback on: eframe uses /// Get and deserialize the [RON](https://github.com/ron-rs/ron) stored at the given key.
#[cfg(feature = "ron")]
pub fn get_value<T: serde::de::DeserializeOwned>(storage: &dyn Storage, key: &str) -> Option<T> {
profiling::function_scope!(key);
storage
.get_string(key)
.and_then(|value| match ron::from_str(&value) {
Ok(value) => Some(value),
Err(err) => {
// This happens on when we break the format, e.g. when updating egui.
log::debug!("Failed to decode RON: {err}");
None
}
})
}
/// Serialize the given value as [RON](https://github.com/ron-rs/ron) and store with the given key.
#[cfg(feature = "ron")]
pub fn set_value<T: serde::Serialize>(storage: &mut dyn Storage, key: &str, value: &T) {
profiling::function_scope!(key);
match ron::ser::to_string(value) {
Ok(string) => storage.set_string(key, string),
Err(err) => log::error!("eframe failed to encode data using ron: {}", err),
}
} Now there are only 2 ways i can think of to solve this for
The 2nd one is more limiting but more efficient and makes the process of If we do the 2nd option we can just add a |
Could we make set_value and get_value part of the storage trait? Then the user can implement this hovever they like |
Good idea is something like this what you mean? pub trait Storage {
/// Get the value for the given key.
fn get_string(&self, key: &str) -> Option<String>;
/// Set the value for the given key.
fn set_string(&mut self, key: &str, value: String);
/// Get and deserialize the value stored at the given key.
fn get_value<T: serde::de::DeserializeOwned>(&self, key: &str) -> Option<T>;
/// Serialize the given value and store it with the given key.
fn set_value<T: serde::Serialize>(&mut self, key: &str, value: &T);
/// write-to-disk or similar
fn flush(&mut self);
} If so then unfortunately this is not possible due the use of generics unless we remove generics
I've also tried using a 2nd trait but then storage itself does not have them thus in impl EpiIntegration {
...
pub fn save(&mut self, _app: &mut dyn epi::App, _window: Option<&winit::window::Window>) {
#[cfg(any(feature = "persistence", feature = "persistence-custom"))]
if let Some(storage) = self.frame.storage_mut() {
..... rest of the code
} |
Must it be E: Of course, the design ship of that trait has sailed, so this is indeed a hypothetical question for a future breaking version. |
Implement a
storage_creator
field to allow users to create a custom implementation ofFileStorage
for custom storage of persistence dataHere is a portable self-contained example of using JSON instead of Ron via
storage_creator
:eframe_template_persistence.zip