Add the button pressed to some signals in Tree

[trollodel]

Supersedes #46588
Based on this comment

This PR replaces most rmb (some can't be changed easily due to dependencies with key events) signals with a mouse_button_index parameter.

Changes:

• button_pressed(TreeItem item, int column, int id) -> button_clicked(int mouse_button_index, TreeItem item, int column, int id)
• empty_rmb(Vector2 position) -> empty_clicked(Vector2 position, int mouse_button_index)
• empty_tree_rmb_selected(Vector2 position) -> void
• item_rmb_edited() -> custom_item_clicked(int mouse_button_index)
• item_rmb_selected(Vector2 position) -> item_mouse_selected(Vector2 position, int mouse_button_index)

Note: I prefer to wait a review on the signal changes before changing the affected editor code (and fix docs).
EDIT: The signals names were adapted as requested (to be consistent with #59720 changes), and the empty_tree_rmb_selected signal was removed, as discussed here.

[akien-mga]

As we discussed in a PR review meeting, the proposed changes here make sense. We just need to make sure that we harmonize the API to be similar between Tree and ItemList, the latter being changed in #59720 (so you're welcome to also participate in reviewing #59720 to make sure that the signal names we implement make sense for Tree too).

[KoBeWi]

button_mouse_clicked

We decided to remove the mouse part, so it should be button_clicked

item_edit_mouse_clicked

This name is awkward. Something like item_mouse_edited would be better.
Although that can be confused with item_edited, which is completely different signal. This one seems to be emitted on cells in custom mode. Maybe something like custom_item_clicked?

EDIT:
I noticed there is item_rmb_selected signal. Should be changed to item_mouse_selected or similar and pass mouse button.

[KoBeWi]

Suggested change

	// make sure in case of wrong reference after reconstructing whole TreeItems
	// Make sure in case of wrong reference after reconstructing whole TreeItems.

(also the sentence is a bit unclear. Make sure what?)

[trollodel]

This block was moved and that comment was already there. I can change it as you suggested if you want.

[KoBeWi]

From the old code it seems like both signals were for mouse clicks, p_lmb was just for the left click. It's ok to divide it like that, but the method doesn't properly get the mouse buttons (e.g. clicking a checkable item doesn't emit the mouse clicked signal, because it calls item_edited() without the last argument).

[trollodel]

The problem with item_edited is that it's not always called from a mouse event, but may come from a keyboard event or even an event that was fired outside Tree (for both, Tree::_text_editor_submit is an example). Maybe I can propagate the mouse button when it's known, but it won't be really consistent or predictable (because it depends on how the user use the Tree).

[KoBeWi]

No I mean e.g. here:

				if (force_edit_checkbox_only_on_checkbox) {
					if (x < cache.checked->get_width()) {
						p_item->set_checked(col, !c.checked);
						item_edited(col, p_item);
					}
				} else {
					p_item->set_checked(col, !c.checked);
					item_edited(col, p_item);
				}

This is done in response to mouse click, but you aren't passing the mouse button.

[trollodel]

I did a quick pass where item_edited is used and added the mouse button where possible.

[KoBeWi]

I checked the changes and they are an improvement, but tbh the Tree API is so bad that it would need much bigger rework. This could be good to merge as a start though.

[trollodel]

I noticed there is item_rmb_selected signal. Should be changed to item_mouse_selected or similar and pass mouse button.

Done, but now there is overlaps between item_mouse_selected and item_selected. I can't get rid of item_selected because it handles keyboard actions too.

[KoBeWi]

Sorry, but I was actually wrong about custom_item_clicked signal :/ It's emitted on any cell type that supports editing.
item_mouse_edited would be a better name.

[KoBeWi]

In any case, the changes look good now.

Tree actually needs much more work. We should look into all signals and methods and evaluate what should be changed/removed, as the class just tries to do too many things at once. I'm going to make a proposal about it.

[akien-mga]

Thanks!