Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve SceneTreeEditor usability #85386

Merged

Conversation

RedMser
Copy link
Contributor

@RedMser RedMser commented Nov 26, 2023

Closes godotengine/godot-proposals#5480

A mix of usability improvements and bug fixes for the SceneTreeEditor and SceneTreeDialog. Here's an overview of the changes:

  • Mark contextually relevant node using alternate text color
  • Set/Clear initial selection as appropriate
  • Scroll to selected node (or if none is selected, scroll to marked node)

An example of how the dialog now starts out when assigning an @export var x: Node variable:

image

@Mickeon
Copy link
Contributor

Mickeon commented Mar 30, 2024

What's the "marked" Node in this context?

@RedMser
Copy link
Contributor Author

RedMser commented Mar 30, 2024

What's the "marked" Node in this context?

All nodes that use the theme's accent color as their text color. It's not the same as the selected node.

For example, if you have a script on node X that contains @export var foo: Node, then you try assigning a node to foo, the marked node in the dialog is X.

Similar goes for e.g. adding a new track in AnimationPlayer. The marked node there is the animation root.

editor/plugins/cpu_particles_3d_editor_plugin.cpp Outdated Show resolved Hide resolved
editor/gui/scene_tree_editor.cpp Outdated Show resolved Hide resolved
editor/editor_properties.cpp Outdated Show resolved Hide resolved
@RedMser RedMser force-pushed the scene-tree-editor-improvements branch from 8206493 to c39c125 Compare March 30, 2024 19:21
@RedMser RedMser force-pushed the scene-tree-editor-improvements branch from c39c125 to 865bf59 Compare March 31, 2024 11:42
- Mark contextually relevant node
- Remember/Clear selection as appropriate
- Scroll to marked/selected node
@RedMser RedMser force-pushed the scene-tree-editor-improvements branch from 865bf59 to 9cd3020 Compare March 31, 2024 19:35
@akien-mga akien-mga modified the milestones: 4.x, 4.3 Apr 4, 2024
@akien-mga akien-mga merged commit e302fb2 into godotengine:master Apr 4, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

fire added a commit to V-Sekai/godot that referenced this pull request Apr 5, 2024
…editor-improvements"

This reverts commit e302fb2, reversing
changes made to c196d12.

if (item) {
// Must wait until tree is properly sized before scrolling.
callable_mp(tree, &Tree::scroll_to_item).call_deferred(item, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR seems to be causing crashes. Probably deferred call should use ObjectID instead of raw pointer, otherwise the tree item might be removed before callback execution and pointer will become invalid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you show me other code locations that do something like this? Also if there's a reliable way to reproduce the crash would help.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #90235.

Copy link
Member

@bruvzg bruvzg Apr 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you show me other code locations that do something like this?

Not sure if there's exactly same situations, buy you can do something like:

Add a new callback:

void Tree::_scroll_to_item_id(ObjectID p_item_id, bool p_center_on_item) {
	TreeItem *item = Object::cast_to<TreeItem>(ObjectDB::get_instance(p_item_id));
	if (item) {
		scroll_to_item(item, p_center_on_item);
	}
}

And call it like this

if (item) {
	ObjectID item_id = item->get_instance_id();
	callable_mp(tree, &Tree::_scroll_to_item_id).call_deferred(item_id, true);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SceneTreeDialog: better navigation for large scene trees
6 participants