Change UserState to outside of GraphEditorState about egui_node_graph HOT 6 OPEN

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.

from egui_node_graph.

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.

from egui_node_graph.

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.

from egui_node_graph.

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

from egui_node_graph.

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

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

from egui_node_graph.

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,
        );
    }
}

from egui_node_graph.