Change UserState to outside of GraphEditorState

[kkngsm]

Changes

1. Change UserState to outside of GraphEditorState #57
2. add persistence feature flag

[setzer22]

Thanks for the PR! 😄 Everything looks good, sorry it took me a while to review this.

I left a couple of questions just to make sure I understand the changes, but I don't think there's any actionable stuff.

[setzer22]

I'm not an expert with the rules about variance. Usually I've seen fn() -> T used as the PhantomData argument when you want to force the two types to be invariant. Why is invariance important here?

[kkngsm]

Thank you for your review!
The conclusion is that the impact of T can be reduced.

First, to correct you, according to The Rustonomicon fn() -> T is covariant.

Second, I quote from rust-lang/rust#46749 (comment)

PhantomData does 3 things:

• variance
• owns
• auto trait opt outs

The key here is "owns" and "auto trait opt outs".

An example of "auto trait opt outs", For example, with a code like this:

use std::{marker::PhantomData, rc::Rc, thread};
#[derive(Debug, Default)]
struct Wrapper<T>{
    _phantom: PhantomData<T> 
}
fn main() {
    let out = Wrapper::<Rc<usize>>::default();
    thread::spawn(move|| out);
}

Run in Rust Playground
This code can't be compiled, because Rc<usize> and Outer<Rc<usize>> are not impled Send.
PhantomData<T> inherits !Send.

How to fix this:

use std::{marker::PhantomData, rc::Rc, thread};

#[derive(Debug, Default)]
struct Wrapper<T>{
    _phantom: PhantomData<fn()->T>
}
fn main() {
    let out = Wrapper::<Rc<usize>>::default();
    thread::spawn(move|| out);
}

Run in Rust Playgound
This doesn't inherit !Send and can be compiled.
Even if T is !Send, the Wrapper is Send.

Also, regarding "owns", I am not familiar with it, but I heard that PhantomData<fn()->T> is better than PhantomData<T> due to DropChekcer (unless you are implementing an unsafe Drop).

[setzer22]

Nice touch! I like that the example app keeps the state now. One question though: Are we sure this won't panic when the type definitions change? I'm assuming the unwrap_or_else call would catch this.

[kkngsm]

If the field name is different, get_value returns None, but since it is stored in Ron format, the difference between usize and i32, etc., seems to be cast.

[setzer22]

If you want to include the change discussed in #57 about passing &mut UserState please go ahead 👍

[kkngsm]

I changed the UserState args to mutable and made NodeGraphExample::new() simpler using syntax sugar.

[setzer22]

Looks ready to merge! Thanks again for the PR 😄