-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
EditorResourcePicker: Replace options to load file with button for QuickOpenDialog #97860
base: master
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
I added a proposal that answers some of your feedback, I hope. |
I love the addition of the icon, but will mention that I occasionally use the slow-load option, when I can't remember exactly what a file is called, but do remember where it is in my directory tree. I would be fine seeing the option removed though -- dragging from FileSystem is just as good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally, it works as expected.
My only concern is that the short viewable area for the path will become even shorter with the addition of a new button:
I don't know a good solution to this. I'd say the saved click is worth losing some space here, since it's often needed, especially when setting up materials or audio samples.
editor/editor_resource_picker.cpp
Outdated
const Vector<String> &base_types_string = base_type.split(","); | ||
|
||
Vector<StringName> base_types; | ||
for (const String &type : base_types_string) { | ||
base_types.push_back(type); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const Vector<String> &base_types_string = base_type.split(","); | |
Vector<StringName> base_types; | |
for (const String &type : base_types_string) { | |
base_types.push_back(type); | |
Vector<StringName> base_types; | |
for (String type : base_type.split(",")) { | |
base_types.push_back(type); |
Would be cleaner IMO
(my bad clicked wrong should have been a simple comment) |
…ickOpenDialog - Remove option to load files from drop down - Make _update_menu_items solely responsible for adding the right separators. Previously the overridable set_create_options would need to do this, but it could not know whether more properties were going to be added.
With the new quick open dialog, I think it is helpful to make it more accessible quickly.
2024-10-06.08-52-13.mp4
Additionally, I had to change some code to handle the separators. Now all separators are added in one method: _update_menu_items. Previously the overridable set_create_options would need to do this to ensure proper separation (this wasn't documented), but it could not know whether more properties were going to be added.
Closes godotengine/godot-proposals#10910