Change UserState to outside of GraphEditorState

[kkngsm]

In some cases, you do not want to implement serde in UserState but want to pass it as an argument to bottom_ui().
However, currently, if the user implements serde in GraphEditorState, the user must also implement it in UserState.

Therefore, we suggest not keeping UserState in GraphEditorState.

Like this,

draw_graph_editor(ui, AllMyNodeTemplates, &self.user_state)

[kkngsm]

I am thinking of changing some the UserState arguments from &UserState to &mut UserState, is there any reason why it should be &UserState?

[setzer22]

Hi! You are right, there is no reason I can think of to have UserState live inside the graph state. Users could manage the lifetime of their state struct more flexibly if it was a separate struct.

I am thinking of changing some the UserState arguments from &UserState to &mut UserState, is there any reason why it should be &UserState?

When the user state struct was inside the graph, it wasn't possible to pass it as an exclusive reference to some methods. If that is possible now after your PR, it may help simplify some things so I wouldn't be against it 🤔

I still like the current design where people are encouraged to defer mutations to the user state until the end via the Response object. But I don't want to be overly opinionated on this.

[kkngsm]

Making user_state mutable in bottom_ui() makes the following line unnecessary, but requires an example of response usage.

egui_node_graph/egui_node_graph_example/src/app.rs

Lines 404 to 409 in 75308d0

	if let NodeResponse::User(user_event) = node_response {
	match user_event {
	MyResponse::SetActiveNode(node) => self.user_state.active_node = Some(node),
	MyResponse::ClearActiveNode => self.user_state.active_node = None,
	}
	}

I suggest adding a node such as AddScalarByUi (related #25).

[setzer22]

I like that the current example showcases the custom user response feature. Deferring side effects is useful, because it lets you decouple the UI code from the business logic. Showing that in the example helps users understand why there's a custom response type to begin with.

This would be more obvious if there was more complex logic attached to the "set active" action, but then that would complicate the example unnecessarily.

[kkngsm]

how about adding a SumScalar that can increase input param?

Responses will be as follows

match user_event {
    MyResponse::AddInputParam {
        node_id,
        name,
        typ,
        value,
        kind,
        shown_inline,
    } => {
        self.state.graph.add_input_param(
            node_id,
            name,
            typ,
            value,
            kind,
            shown_inline,
        );
    }
}

[setzer22]

Ok, I just realized I misunderstood your original comment. Sorry about that @kkngsm!

Yes, I'm all for an example like SumScalar using the response mechanism, and then having the simpler "Set active" button simply using mutation. Yours is a great example of why the response mechanism is necessary, because you can't modify the graph while you're drawing it, and some side-effects need to be deferred.