-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Arbitrary data in Memory
using Any
, fix #255
#257
Conversation
Any
Memory
using Any
Tomorrow I will write more documentation and rewrite all widgets to new API. |
I will test this on the web, improve grammar and vocabulary in the documentation and mark this PR as ready for review. |
Memory
using Any
Memory
using Any
, fix #255
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.
Excellent work!
I left some comments, but beside that I think it would also be good with a couple of unit tests, especially covering serialization/deserialization of AnyMapId
. For instance, does the AnyMapId
handle serializing struct State { foo: i32 }
and deserializing to stuct State { foo: i32, #[serde(default)] bar: f64 }
(simulating adding a new field to an existing state)?
I also think you can make your own crate out of all this if you would like! I think it could become a popular alternative to e.g. anymap
and std::any::Any
for others who needs serialization!
Okay, thank you! Right now I am realized that I have no more time, so I can continue this PR after 6 April. Also, thanks about the idea of moving this out as a crate 🤔 |
* rename `DataElement` -> `AnyMapElement` * make `data` in `Memory` as public field of type with public interface * make interface more rich * transform most unwraps to proper error handling * make `AnyMap` store by `TypeId`, so now individual type can be counted and reset
* replace `serde_json` -> `ron` * move `any_map` to module * separate `AnyMap` into `AnyMapId` and `serializable::AnyMapId` in order to not require `serde` traits in methods * add `AnyMap` and `serializable::AnyMap` that stores elements just by type, without `Id` * write documentation * change tooltips and color picker to use `Memory::data_temp`
Co-authored-by: Emil Ernerfeldt <[email protected]>
* rename `reset` → `remove` * add function `remove_by_type` * format code
7a5eecb
to
aa8c081
Compare
@emilk everything is done. |
I don't like all this copy-pasting in |
No, I believe this is not possible, because this may require an associated trait inside of a trait to generically handle |
* make identical interface for serialized and simple maps * make serialized maps serialized fully, without features by moving this into `Memory` struct with `#[cfg(feature = "persistence")]` under fields * move `serialized::TypeId` into `AnyMapElement` struct
However, copy-pasting between |
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.
Amazing work - thank you! ⭐
|
||
# Only needed if you plan to use the same egui::Context from multiple threads. | ||
single_threaded = ["epaint/single_threaded"] | ||
multi_threaded = ["epaint/multi_threaded"] | ||
|
||
[dev-dependencies] | ||
serde_json = "1" |
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.
we could use ron
for the tests too
}; | ||
|
||
let mut map: TypeMap = serde_json::from_str(&file_string).unwrap(); | ||
assert!(map.get::<StateNew>().is_none()); |
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.
Oh right, because TypeId
changes. In the future it would be nice to not rely on TypeId
, but perhaps use a per-type UUID or similar to handle it. That would make persistence work across rustc versions too.
|
||
/// Stores any object by `Key`. | ||
#[derive(Clone, Debug)] | ||
pub struct AnyMap<Key: Hash + Eq>(HashMap<Key, AnyMapElement>); |
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.
Now that this no longer relies on Id
it would be trivial to move to its own crate - hint, hint, nudge, nudge ;)
This refactors the widget state storage introduced by @optozorax in #257 * Unify the four buckets (`data`, `data_temp`, `id_data` and `id_data_temp`) into a single `data`. * Less complexity, and also less chance of error (storing in one bucket, reading from another). * Store data by `Id` and `TypeId`. * Users can thus reuse the same `Id` to store many types. * Uses a simple xor of id and typeid, which is fast and good since both id and typeid are already high-entropy hashes. * Use different suffixes on the functions to pick if you want the data persisted or not (`get_temp`, `insert_persisted`, etc). * Writing with one suffix and reading with the other works. * To store state not bound to a specific `Id` (i.e. only based on type), use the new `Id::null` as the key.
This refactors the widget state storage introduced by @optozorax in emilk/egui#257 * Unify the four buckets (`data`, `data_temp`, `id_data` and `id_data_temp`) into a single `data`. * Less complexity, and also less chance of error (storing in one bucket, reading from another). * Store data by `Id` and `TypeId`. * Users can thus reuse the same `Id` to store many types. * Uses a simple xor of id and typeid, which is fast and good since both id and typeid are already high-entropy hashes. * Use different suffixes on the functions to pick if you want the data persisted or not (`get_temp`, `insert_persisted`, etc). * Writing with one suffix and reading with the other works. * To store state not bound to a specific `Id` (i.e. only based on type), use the new `Id::null` as the key.
This refactors the widget state storage introduced by @optozorax in emilk/egui#257 * Unify the four buckets (`data`, `data_temp`, `id_data` and `id_data_temp`) into a single `data`. * Less complexity, and also less chance of error (storing in one bucket, reading from another). * Store data by `Id` and `TypeId`. * Users can thus reuse the same `Id` to store many types. * Uses a simple xor of id and typeid, which is fast and good since both id and typeid are already high-entropy hashes. * Use different suffixes on the functions to pick if you want the data persisted or not (`get_temp`, `insert_persisted`, etc). * Writing with one suffix and reading with the other works. * To store state not bound to a specific `Id` (i.e. only based on type), use the new `Id::null` as the key.
CONTRIBUTING.md