setzer22 / egui_node_graph

Build your node graph applications in Rust, using egui
MIT License
715 stars 134 forks source link

Delete the active node in egui_node_graph_example, App will crash #22

Closed kkngsm closed 2 years ago

kkngsm commented 2 years ago

This is a great library 😍. Thank you!

I will report it because egui_node_graph_example crashed.

Operation until the crash

  1. Create any Node.
  2. Set Active the Node.
  3. Delete the Node
  4. Crashed!!
    thread 'main' panicked at 'NodeId index error for NodeId(1v1). Has the value been deleted?', /egui_node_graph/egui_node_graph/src/index_impls.rs:33:1
    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Reason for the bug

This is because MyResponse::SetActiveNode(node) holds the NodeId even if the node is deleted, and uses that NodeId to access the deleted Node in the evaluate_node function.

How to fix

https://github.com/setzer22/egui_node_graph/blob/8408503866201ee5db4af72ab19494278942afbd/egui_node_graph_example/src/app.rs#L378-L390

if let Some(node) = self.state.user_state.active_node {
+    if self.state.graph.nodes.contains_key(node){
        let text = match evaluate_node(&self.state.graph, node, &mut HashMap::new()) {
            Ok(value) => format!("The result is: {:?}", value),
            Err(err) => format!("Execution error: {}", err),
        };
        ctx.debug_painter().text(
            egui::pos2(10.0, 10.0),
            egui::Align2::LEFT_TOP,
            text,
            egui::TextStyle::Button,
            egui::Color32::WHITE,
        );
+    }
}
kkngsm commented 2 years ago

Sorry. It was self.state.user_state.active_node, not MyResponse::SetActiveNode(node), that was holding NodeId.

And, I should reset self.state.user_state.active_node,

To be exact

if let Some(node) = self.state.user_state.active_node {
+    if self.state.graph.nodes.contains_key(node){
        let text = match evaluate_node(&self.state.graph, node, &mut HashMap::new()) {
            Ok(value) => format!("The result is: {:?}", value),
            Err(err) => format!("Execution error: {}", err),
        };
        ctx.debug_painter().text(
            egui::pos2(10.0, 10.0),
            egui::Align2::LEFT_TOP,
            text,
            egui::TextStyle::Button,
            egui::Color32::WHITE,
        );
+    }else{
+         self.state.user_state.active_node = None;
+    }
}