Nelarius / imnodes

A small, dependency-free node editor for dear imgui
MIT License
2.01k stars 244 forks source link

issues with example/save_load.cpp? #52

Closed erwincoumans closed 3 years ago

erwincoumans commented 4 years ago

Thanks for sharing this nice project!

1) object_pool_update clears the in_use flag to false for all nodes. At exit, none of the node positions are saved to save_load.ini, since the SaveEditorStateToIniString checks for in_use. Commenting out this check makes it work. Why was that check there?

2) I had to move the node creation (released key A) right after BeginNodeEditor and before the rendering of all nodes, otherwise some assert triggered in EndNodeEditor. This happens when you start with an empty editor (no nodes, no ini file) and press 'a'. The assert was this one:

void draw_list_activate_node_background(const int node_idx)
{
    const int submission_idx =
        g.node_idx_to_submission_idx.GetInt(static_cast<ImGuiID>(node_idx), -1);
    assert(submission_idx != -1);
Nelarius commented 4 years ago

Hi @erwincoumans ! Sorry it took a while to respond to the issue.

  1. That was a bug that was introduced when merging the focus-stack changes in to master. I just pushed a fix to master which effectively retains the state of the in_use array after the call to EndNodeEditor.

Commenting out this check makes it work. Why was that check there?

in_use is used to keep track of nodes that were actually rendered during a frame, since the user can at any point just not call BeginNode for a node. The check in SaveEditorStateToIniString was there to avoid writing out state for nodes which aren't currently being used. But now that I'm writing this, I'm wondering how useful that check is since it only really "helps" in the situation where there is a huge amount of unused nodes in the backing array...

  1. Yeah, this is actually a breaking change that was introduced by the focus-stack change. Is handling node creation right after BeginNodeEditor an issue for you? I think I can create a workaround if that is the case. As a side note, the examples were also updated to reflect this change: https://github.com/Nelarius/imnodes/commit/dd7b728792f761c8c7479363b551c3e5cf7e363b
Nelarius commented 3 years ago

Updated draw_list_activate_node_background with a comment to indicate why the assert was triggered.

    // There is a discrepancy in the submitted node count and the rendered node count! Did you call
    // one of the following functions
    // * EditorContextMoveToNode
    // * SetNodeScreenSpacePos
    // * SetNodeGridSpacePos
    // * SetNodeDraggable
    // after the BeginNode/EndNode function calls?

If handling node creation right after BeginNodeEditor is an issue for you, feel free to reopen the issue!