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

Add "Show In FileSystem" to the Scene hierarchy right-click #84882

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

Invertex
Copy link
Contributor

Currently, right-clicking either an open Scene tab or Resource in the Inspector shows an option for "Show in FileSystem", which will highlight the source asset in the FileSystem dock. This is missing from the right-click context in the Scene hierarchy though.

As projects grow, it becomes very useful to be able to focus the source of an instance in your scene to manage other assets related to it in that location or do modifications to source assets.

This commit adds that option for anything that has a source path in the Scene hierarchy, allowing easy navigation to the source of an instance.

I've also created/implemented an icon for this option and implemented it in the "Show In FileSystem" of other context menus that had it.

img

@KoBeWi
Copy link
Member

KoBeWi commented Nov 14, 2023

I think the new option should be near other scene options, maybe above Open in Editor:
image

editor/scene_tree_dock.cpp Outdated Show resolved Hide resolved
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Overall looks fine and includes some bonus fixes for navigate_to_path().

EDIT:
Also remember to squash your commits once you are finished.

@Invertex
Copy link
Contributor Author

Invertex commented Nov 14, 2023

I think the new option should be near other scene options, maybe above Open in Editor: image

Perhaps in the grouping with Open Documentation (but in first slot)? Since it's an action that takes you away from the Scene tab?
img

@Invertex Invertex force-pushed the select-instance-asset branch from 2028f57 to 83b080b Compare November 14, 2023 16:34
@Invertex
Copy link
Contributor Author

EDIT: Also remember to squash your commits once you are finished.

In this situation I'm not sure how to handle that safely, since a merge commit from one of the suggested fixes here is blocking the squash. Not sure on the process in that situation and don't want to mistakenly break the PR. Thanks

@AThousandShips
Copy link
Member

AThousandShips commented Nov 14, 2023

You need you push with git push --force, see here

You should always use git push --force instead of making me the commits

If you can't do the squashing I can do it for you, but it's required

@Invertex Invertex force-pushed the select-instance-asset branch 2 times, most recently from 725ea71 to 98951eb Compare November 14, 2023 20:29
@Invertex
Copy link
Contributor Author

I believe I've done it correctly, I hope!

@YuriSizov YuriSizov modified the milestones: 4.x, 4.3 Dec 13, 2023
@YuriSizov YuriSizov changed the title Adds "Show In FileSystem" to the Scene hierarchy right-click Add "Show In FileSystem" to the Scene hierarchy right-click Dec 13, 2023
-Implemented shared function for focusing FileSystem tab and highlighting the node path.
-Created right-click option that shows up in the Scene-Hierarchy on Nodes that have a file-system source path.
-Created custom icon for this right-click option
-Implemented the shared function and icon for other places that already had this features (Open Node tab, Inspector Resource)

Co-authored-by: MewPurPur <mew.pur.pur@gmail.com>
@Invertex Invertex force-pushed the select-instance-asset branch from 98951eb to 5978768 Compare December 13, 2023 15:43
@YuriSizov YuriSizov merged commit 8f33b4e into godotengine:master Dec 14, 2023
15 checks passed
@YuriSizov
Copy link
Contributor

Thanks, and congrats on your first merged Godot contribution!

@@ -165,7 +165,7 @@ void EditorSceneTabs::_update_context_menu() {

if (tab_id >= 0) {
scene_tabs_context_menu->add_separator();
scene_tabs_context_menu->add_item(TTR("Show in FileSystem"), EditorNode::FILE_SHOW_IN_FILESYSTEM);
scene_tabs_context_menu->add_icon_item(get_editor_theme_icon(SNAME("ShowInFileSystem")), TTR("Show in FileSystem"), EditorNode::FILE_SHOW_IN_FILESYSTEM);
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't have been changed. No other option has icon in this menu.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured the option should remain consistent across menus, and a discussion should be had about having options in that menu have icons too for consistency. But I should have brought that up.

Is there something I should do on my part to revert this part? Not sure on the process after a PR is already accepted.

Copy link
Member

Choose a reason for hiding this comment

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

You can open a new PR that reverts this particular change or someone else can do it.

For adding more icons to that menu you should open a proposal.

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.

5 participants