Cleanups after changes in ItemList signals

[lufog]

• rmb_clicked replaced with empty_clicked
• _file_list_rmb_pressed renamed to _file_list_empty_clicked
• added additional arguments and check for the pressed button.
• _file_list_rmb_clicked renamed to _file_list_item_clicked
• removed extra argument from EditorFileDialog::_items_clear_selection (empty_clicked expects 2 arguments)

Continuation of the work done in #59720.

[akien-mga]

Are you sure that this code is not meant to be used? There was likely different logic based on whether a right click is done on an item (file or folder specific context menu) or empty space (actions that can be done in the current folder).

[lufog]

All context menus (rmb click on file/folder/empty space) in editor's FileSystem work correctly.

The method:

godot/editor/filesystem_dock.cpp

Lines 2668 to 2689 in e3371a5

	void FileSystemDock::_file_list_rmb_pressed(const Vector2 &p_pos) {
	// Right click on empty space for file list.
	if (searched_string.length() > 0) {
	return;
	}
	path = current_path->get_text();
	file_list_popup->clear();
	file_list_popup->reset_size();
	file_list_popup->add_icon_item(get_theme_icon(SNAME("Folder"), SNAME("EditorIcons")), TTR("New Folder..."), FILE_NEW_FOLDER);
	file_list_popup->add_icon_item(get_theme_icon(SNAME("PackedScene"), SNAME("EditorIcons")), TTR("New Scene..."), FILE_NEW_SCENE);
	file_list_popup->add_icon_item(get_theme_icon(SNAME("Script"), SNAME("EditorIcons")), TTR("New Script..."), FILE_NEW_SCRIPT);
	file_list_popup->add_icon_item(get_theme_icon(SNAME("Object"), SNAME("EditorIcons")), TTR("New Resource..."), FILE_NEW_RESOURCE);
	file_list_popup->add_icon_item(get_theme_icon(SNAME("TextFile"), SNAME("EditorIcons")), TTR("New TextFile..."), FILE_NEW_TEXTFILE);
	file_list_popup->add_separator();
	file_list_popup->add_icon_item(get_theme_icon(SNAME("Filesystem"), SNAME("EditorIcons")), TTR("Open in File Manager"), FILE_SHOW_IN_EXPLORER);
	file_list_popup->set_position(files->get_screen_position() + p_pos);
	file_list_popup->reset_size();
	file_list_popup->popup();
	}

has not been used anywhere except here (and since this signal is removed, this method cannot be called any way):

godot/editor/filesystem_dock.cpp

Line 3138 in e3371a5

files->connect("rmb_clicked", callable_mp(this, &FileSystemDock::_file_list_rmb_pressed));

As far as I understand, this is a reworked version of _file_list_rmb_pressed:

godot/editor/filesystem_dock.cpp

Lines 2640 to 2666 in e3371a5

	void FileSystemDock::_file_list_rmb_clicked(int p_item, const Vector2 &p_pos, MouseButton p_mouse_button_index) {
	if (p_mouse_button_index != MouseButton::RIGHT) {
	return;
	}
	// Right click is pressed in the file list.
	Vector<String> paths;
	for (int i = 0; i < files->get_item_count(); i++) {
	if (!files->is_selected(i)) {
	continue;
	}
	if (files->get_item_text(p_item) == "..") {
	files->deselect(i);
	continue;
	}
	paths.push_back(files->get_item_metadata(i));
	}
	// Popup.
	if (!paths.is_empty()) {
	file_list_popup->clear();
	_file_and_folders_fill_popup(file_list_popup, paths, searched_string.length() == 0);
	file_list_popup->set_position(files->get_screen_position() + p_pos);
	file_list_popup->reset_size();
	file_list_popup->popup();
	}
	}

used here:

godot/editor/filesystem_dock.cpp

Line 3135 in e3371a5

files->connect("item_clicked", callable_mp(this, &FileSystemDock::_file_list_rmb_clicked));

@Vitika9, can correct me here if I'm wrong.

[VitikaSoni]

This was not supposed to be removed. My PR included changing rmb_clicked(position) and nothing_selected to empty_clicked(position, mouse_button). Therefore, what you need to do is

• replace rmb_clicked with nothing_clicked
• change the name of _file_list_rmb_pressed to _file_list_empty_clicked and adding a MouseButton parameter named p_mouse_button_index to it
• add an early return to _file_list_empty_clicked if mouse button is not right.

[akien-mga]

Changes look correct to me. One last style nitpick (not from this PR but to improve the previous one) would be to also rename _file_list_rmb_clicked to _file_list_item_clicked to have it match its renamed signal too.

[lufog]

Yes, I thought about that too. Also, I would like to double-check everything again to make sure that I correctly understand how everything works and that I did not miss anything.

[lufog]

Initially, I misunderstood how the code works, because I experimented with normal FileSystem dock mode, but I needed to test it in the split mode.

By the way, I found an additional problem, but I think it can be fixed in another PR.

bug.mp4

[akien-mga]

By the way, I found an additional problem, but I think it can be fixed in another PR.

I can't seem to reproduce it in the current master branch (ccb583b), so it might actually be triggered by these fixes. But it's not a big deal indeed so it can be left for a follow-up PR.

Edit: I can't reproduce it either (on Linux X11) with a build from this PR, so it might not be related to this PR in the end.

[lufog]

I can reproduce it in the current master branch (ccb583b) and in v4.0-alpha7 on Windows 11. Perhaps the issue is platform dependent.

[akien-mga]

Thanks!

[KoBeWi]

#60921