Added panel to the editor to view and search UIDs - #1144
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughA new UID Viewer dock component is introduced to the editor, enabling users to enumerate and search resource UIDs alongside their file paths. The dock integrates into the editor UI with search/filter capabilities, context menu actions, and automatic refresh triggered by filesystem changes. Changes
Sequence DiagramsequenceDiagram
actor User
participant EditorNode
participant UIDViewerDock
participant EditorFileSystem
participant ResourceLoader
participant FileSystemDock
EditorNode->>UIDViewerDock: instantiate & add to bottom panel
UIDViewerDock->>EditorFileSystem: subscribe to filesystem_changed signal
EditorFileSystem-->>UIDViewerDock: filesystem_changed (initial or on change)
UIDViewerDock->>UIDViewerDock: _refresh_uid_list()
UIDViewerDock->>EditorFileSystem: traverse res:// directory
EditorFileSystem-->>UIDViewerDock: resource paths
UIDViewerDock->>ResourceLoader: query UID for each resource
ResourceLoader-->>UIDViewerDock: UID or null
UIDViewerDock->>UIDViewerDock: populate tree with UID + path items
User->>UIDViewerDock: type in search field
UIDViewerDock->>UIDViewerDock: _on_search_text_changed()
UIDViewerDock->>UIDViewerDock: _filter_tree_recursive() on root
UIDViewerDock->>UIDViewerDock: update item visibility based on search
User->>UIDViewerDock: click tree item
UIDViewerDock->>UIDViewerDock: _on_item_activated()
UIDViewerDock->>FileSystemDock: navigate to resource file
User->>UIDViewerDock: right-click tree item
UIDViewerDock->>UIDViewerDock: _on_tree_rmb_selected()
UIDViewerDock->>UIDViewerDock: show context_menu
User->>UIDViewerDock: select Copy UID/Copy Path
UIDViewerDock->>UIDViewerDock: _on_context_menu_id_pressed()
UIDViewerDock->>User: copy to clipboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
editor/docks/uid_viewer_dock.h (1)
33-40: Consider removing redundant header guards.Both
#pragma once(line 33) and traditional header guards (lines 39-40) are present. While harmless, modern C++ codebases typically use one approach. Since#pragma onceis already present and widely supported, the traditional guards can be removed.Additionally, the author comment (lines 35-37) is inconsistent with the rest of the codebase style, which doesn't include such comments in other files.
🔎 Proposed cleanup
#pragma once -// -// Created by Andrew Martin on 1/2/26. -// - -#ifndef UID_VIEWER_DOCK_H -#define UID_VIEWER_DOCK_H - #include "scene/gui/box_container.h"And remove the closing
#endifat the end:public: UIDViewerDock(); }; - -#endif // UID_VIEWER_DOCK_Hplatform/android/java/nativeSrcsConfigs/CMakeLists.txt (1)
17-19: Redundant explicit file listing (likely harmless).The GLOB_RECURSE patterns on lines 14-15 already recursively collect all source and header files from
${GODOT_ROOT_DIR}, which includeseditor/docks/uid_viewer_dock.{h,cpp}. Explicitly adding them again (lines 18-19) is redundant.However, since this is a non-functional CMake file for Android Studio editor support (as noted in line 1), the explicit listing might improve IDE indexing. Consider removing the explicit entries unless they're needed for IDE features.
🔎 Optional cleanup
-add_executable(${PROJECT_NAME} ${SOURCES} ${HEADERS} - ../../../../editor/docks/uid_viewer_dock.h - ../../../../editor/docks/uid_viewer_dock.cpp) +add_executable(${PROJECT_NAME} ${SOURCES} ${HEADERS})editor/editor_node.cpp (2)
82-82: Align UID viewer dock include path with other dock includesThe include works as-is, but it’s the only dock header included via
"docks/...while all the others in this file use"editor/docks/...". For consistency with the rest ofeditor_node.cpp, consider switching to:#include "editor/docks/uid_viewer_dock.h"
8453-8458: UID Viewer bottom panel integration looks good; consider small cleanupsThe bottom panel registration is correct and follows existing patterns (e.g. Output panel). Two minor polish points:
UIDViewerDockalready callsset_name("UID Viewer")in its constructor, souid_viewer_dock->set_name("UID Viewer");here is redundant and could be dropped to avoid future drift if the internal name changes.- For consistency with nearby code, you might want a slightly clearer comment and spacing, e.g.
// UID Vieweror// UID Viewer bottom panel.Functionally this is fine to ship as-is.
editor/docks/uid_viewer_dock.cpp (3)
105-110: Misleading variable name:queueis used as a stack.The variable operates in LIFO order (pop from end), making it a stack, not a queue. Consider renaming to
stackfor clarity.🔎 Suggested rename
- Vector<String> queue; - queue.push_back("res://"); + Vector<String> stack; + stack.push_back("res://"); - while (!queue.is_empty()) { - String dir_path = queue[queue.size() - 1]; - queue.resize(queue.size() - 1); + while (!stack.is_empty()) { + String dir_path = stack[stack.size() - 1]; + stack.resize(stack.size() - 1); ... - queue.push_back(dir_path + subdir->get_name() + "/"); + stack.push_back(dir_path + subdir->get_name() + "/");
156-159: Consider the asynchronous nature ofscan().
EditorFileSystem::scan()is asynchronous, so the immediate_refresh_uid_list()call reads the current (potentially stale) filesystem state. The list will auto-update when the scan completes via thefilesystem_changedsignal connection.This works but may cause a brief UI flicker (list refreshes twice). You could either:
- Accept current behavior (functional, minor UX quirk), or
- Only call
scan()and let the signal handler refresh the list once.🔎 Alternative: let signal handler refresh
void UIDViewerDock::_on_refresh_pressed() { EditorFileSystem::get_singleton()->scan(); - _refresh_uid_list(); }
196-211: Consider using named constants for menu IDs.Magic numbers
0and1are fragile if menu items are reordered. An enum improves maintainability:🔎 Suggested refactor
In the header or at file scope:
enum ContextMenuID { CONTEXT_COPY_UID = 0, CONTEXT_COPY_PATH = 1, };Then in the constructor:
- context_menu->add_item("Copy UID"); - context_menu->add_item("Copy Path"); + context_menu->add_item("Copy UID", CONTEXT_COPY_UID); + context_menu->add_item("Copy Path", CONTEXT_COPY_PATH);And in the handler:
- if (id == 0) { // Copy UID + if (id == CONTEXT_COPY_UID) { text_to_copy = last_selected_item->get_text(0); - } else if (id == 1) { // Copy Path + } else if (id == CONTEXT_COPY_PATH) { text_to_copy = last_selected_item->get_text(1); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
editor/docks/uid_viewer_dock.cppeditor/docks/uid_viewer_dock.heditor/editor_node.cppplatform/android/java/nativeSrcsConfigs/CMakeLists.txt
🧰 Additional context used
🧬 Code graph analysis (2)
editor/docks/uid_viewer_dock.h (1)
editor/docks/uid_viewer_dock.cpp (19)
UIDViewerDock(46-94)_refresh_uid_list(96-137)_refresh_uid_list(96-96)_on_search_text_changed(139-154)_on_search_text_changed(139-139)_on_refresh_pressed(156-159)_on_refresh_pressed(156-156)_on_item_activated(161-174)_on_item_activated(161-161)_on_context_menu_id_pressed(196-211)_on_context_menu_id_pressed(196-196)_gui_input(176-194)_gui_input(176-176)_on_tree_rmb_selected(257-272)_on_tree_rmb_selected(257-257)_show_all_items(213-228)_show_all_items(213-213)_filter_tree_recursive(230-255)_filter_tree_recursive(230-230)
editor/editor_node.cpp (2)
editor/docks/uid_viewer_dock.cpp (1)
UIDViewerDock(46-94)editor/docks/uid_viewer_dock.h (1)
UIDViewerDock(48-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: 🤖 Android / Template arm64 (target=template_release, arch=arm64)
- GitHub Check: 🐧 Linux / Editor w/ Mono (target=editor)
- GitHub Check: 🐧 Linux / Minimal template (target=template_release, everything disabled)
- GitHub Check: 🏁 Windows / Template w/ GCC (target=template_release, use_mingw=yes)
- GitHub Check: 🐧 Linux / Template w/ Mono, debug (target=template_debug)
- GitHub Check: 🐧 Linux / Template w/ Mono, release (target=template_release)
- GitHub Check: 🐧 Linux / Editor with ThreadSanitizer (target=editor, dev_build=yes, use_tsan=yes, use_llvm=yes, linker=lld)
- GitHub Check: 🏁 Windows / Template (target=template_release)
- GitHub Check: 🌐 Web / Template w/ threads (target=template_release, threads=yes)
- GitHub Check: 🤖 Android / Template arm32 (target=template_release, arch=arm32)
- GitHub Check: 🌐 Web / Template w/o threads (target=template_release, threads=no)
- GitHub Check: 🍎 macOS / Template (target=template_release)
- GitHub Check: 🏁 Windows / Editor (target=editor)
- GitHub Check: 🍏 iOS / Template (target=template_release)
- GitHub Check: 🍎 macOS / Editor (target=editor)
🔇 Additional comments (4)
editor/docks/uid_viewer_dock.cpp (4)
46-94: LGTM! Well-structured UI initialization.The constructor properly sets up the dock with search, refresh button, tree view, and context menu. Signal connections are appropriate for the intended functionality.
161-174: LGTM!Proper null checks and appropriate use of
FileSystemDockAPI to navigate and select the file.
213-255: LGTM!The recursive filter and show-all implementations correctly handle tree visibility. Good defensive null checks and proper case-insensitive search logic.
257-272: LGTM!This is the correct handler for tree RMB selection. The position coordinates from
item_mouse_selectedare properly relative to the tree.
|
@tindrew can you please run 'pre-commit run -a' and then re submit? Looks like it failed the formatting check. |
| String path_text = item->get_text(1).to_lower(); | ||
|
|
||
| bool matches = uid_text.contains(search_lower) || path_text.contains(search_lower); | ||
| bool visible = matches || has_visible_child; |
There was a problem hiding this comment.
I would rename visible here to result or something.
This is hiding a base member causing the compiler to complain.
Arctis-Fireblight
left a comment
Arctis-Fireblight
left a comment
There was a problem hiding this comment.
Looks good and works great!
Approved!
Added panel to the editor to view and search UIDs
3 participants
UID Viewer Panel
The UID Viewer Panel allows the user to see all the UIDs and associated paths to the file in their project.
The user can filter results by typing in the search box, and double clicking on any UID or path in the panel will target that file in the project dock.
The user can also right click on any uid and can copy the uid or the path.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.