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.

Sign up for GitHub

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

Fix "View Owners" dialog not acknowledging that some resources aren't scenes #68697

Merged
merged 1 commit into from
Jun 6, 2023
Merged

Fix "View Owners" dialog not acknowledging that some resources aren't scenes #68697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions editor/dependency_editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,23 @@ void DependencyEditorOwners::_list_rmb_clicked(int p_item, const Vector2 &p_pos,
file_options->clear();
file_options->reset_size();
if (p_item >= 0) {
if (owners->get_selected_items().size() == 1) {
file_options->add_icon_item(get_theme_icon(SNAME("Load"), SNAME("EditorIcons")), TTR("Open Scene"), FILE_OPEN);
PackedInt32Array selected_items = owners->get_selected_items();
bool only_scenes_selected = true;

for (int i = 0; i < selected_items.size(); i++) {
int item_idx = selected_items[i];
if (ResourceLoader::get_resource_type(owners->get_item_text(item_idx)) != "PackedScene") {
only_scenes_selected = false;
break;
}
}

if (only_scenes_selected) {
file_options->add_icon_item(get_theme_icon(SNAME("Load"), SNAME("EditorIcons")), TTRN("Open Scene", "Open Scenes", selected_items.size()), FILE_OPEN);
} else if (selected_items.size() == 1) {
file_options->add_icon_item(get_theme_icon(SNAME("Load"), SNAME("EditorIcons")), TTR("Open"), FILE_OPEN);
Comment on lines +300 to +303
Copy link
Contributor

Choose a reason for hiding this comment

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

Both the old and the new code is rather misleading, as both options are identical. Only the text is different. I think we could simplify this. And we could also rework the awkward return below in the process with an early return.

Copy link
Contributor Author

@MewPurPur MewPurPur Jun 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure I follow... You can open multiple scenes at once, unlike other resources, so they are separated.

Copy link
Contributor

@YuriSizov YuriSizov Jun 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Both these options trigger FILE_OPEN with no additional information passed from this bit, only the text on the option is different (even the icon is the same).

Copy link
Contributor

@YuriSizov YuriSizov Jun 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Well, in fact, this is quite a lot of checking just to display a slightly different text. Perhaps we should just call the option "Open Selected"? What's the significance of writing "Open Scenes"?

Edit: I guess we still want to know if only scenes are selected, otherwise the action should be simply disabled. Eh, let's leave it as is.

} else {
file_options->add_icon_item(get_theme_icon(SNAME("Load"), SNAME("EditorIcons")), TTR("Open Scenes"), FILE_OPEN);
return;
}
}

Expand All @@ -303,9 +316,19 @@ void DependencyEditorOwners::_select_file(int p_idx) {

if (ResourceLoader::get_resource_type(fpath) == "PackedScene") {
EditorNode::get_singleton()->open_request(fpath);
hide();
emit_signal(SNAME("confirmed"));
} else {
EditorNode::get_singleton()->load_resource(fpath);
}
hide();
emit_signal(SNAME("confirmed"));
}

void DependencyEditorOwners::_empty_clicked(const Vector2 &p_pos, MouseButton p_mouse_button_index) {
if (p_mouse_button_index != MouseButton::LEFT) {
return;
}

owners->deselect_all();
}

void DependencyEditorOwners::_file_option(int p_option) {
Expand Down Expand Up @@ -372,6 +395,7 @@ DependencyEditorOwners::DependencyEditorOwners() {
owners->set_select_mode(ItemList::SELECT_MULTI);
owners->connect("item_clicked", callable_mp(this, &DependencyEditorOwners::_list_rmb_clicked));
owners->connect("item_activated", callable_mp(this, &DependencyEditorOwners::_select_file));
owners->connect("empty_clicked", callable_mp(this, &DependencyEditorOwners::_empty_clicked));
owners->set_allow_rmb_select(true);
add_child(owners);
}
Expand Down
1 change: 1 addition & 0 deletions editor/dependency_editor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class DependencyEditorOwners : public AcceptDialog {
static void _bind_methods();
void _list_rmb_clicked(int p_item, const Vector2 &p_pos, MouseButton p_mouse_button_index);
void _select_file(int p_idx);
void _empty_clicked(const Vector2 &p_pos, MouseButton p_mouse_button_index);
void _file_option(int p_option);

private:
Expand Down