godot-orchestrator icon indicating copy to clipboard operation
godot-orchestrator copied to clipboard

Node sort order is not persisted across restarts.

Open Naros opened this issue 1 year ago • 1 comments

Describe the bug

An orchestration stores all graph nodes in a hash map keyed by the node's numeric ID. The default implementation sorts the elements by the key's order, so nodes with lesser numeric IDs are stored earlier. This approach is excellent for serializing the node data to the underlying file system. It guarantees that the serialization order is preserved across saves and that newly added nodes are always done in an "append" fashion.

However, as a user interacts with nodes in an orchestration, these nodes trigger "raise" events, which reorder the order of the graph node child elements in the editor's scene tree, forcing nodes you've selected more recently to be drawn over nodes that have not be raised or selected more recently.

So, imagine a scene with two simple nodes:

image

In this use case, the Ready node has an ID of 0, and the Set Visible node has an ID of 1. We can also see that the Ready event is drawn in the foreground over the Set Visible node because the user has recently selected it and changed the render order of the graph's nodes. When this orchestration is saved, nodes will be saved as 0 and then 1. During the load, they'll again be read and added to the graph in the order of 0 and then 1. This means when the graph is reopened, the nodes will look like this:

image

Expected behavior

In an ideal world, the render order of nodes should also be retained across restarts, just like a node's size and position.

Actual behavior

The render order is not preserved.

How to Reproduce?

No response

Godot full version

4.3.stable / 4.2.2.stable

Orchestrator version

2.1.stable / 2.0.2.stable

Additional information

No response

Naros avatar Aug 16 '24 01:08 Naros

There are effectively two ways this could be solved:

  • Create an attribute in OScriptNode populated with the render order when apply_changes() is called. Only open graphs would have their render orders updated. Any graph that is not opened will be left unchanged. Then, when the graph is loaded, all nodes will be sorted in render order before being added to the GraphEdit widget.
  • Use an editor-specific cache file in .godot to track the node render order for orchestrations by name. The editor already does this for various things, such as tracking favorites, recently used items, layouts, etc.

The latter is certainly the best choice for version control purposes, and it reduces the change noise seen in diffs.

However, there is an argument that the latter may not be ideal in every use case. Consider a new orchestration where nodes are added and layered in a specific way, after which the file is committed to version control. When a colleague opens that orchestration after checkout for the first time, any layering you did is lost. The only guarantee would be that comment nodes are rendered first so that regular nodes and knots are rendered atop them. But if two normal nodes or comments overlap, then the render order depends on their IDs, as described above.

The main question is whether users will have many use cases of this sort of "overlap." I think that this would likely be a case that wouldn't happen often in normal practice, so perhaps the editor cache paired with comments rendered before normal nodes/knots is likely mostly satisfactory.

Naros avatar Aug 16 '24 02:08 Naros