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.

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 Connection dock's popups always allowing disconnect #81750

Merged

Conversation

Mickeon
Copy link
Contributor

@Mickeon Mickeon commented Sep 16, 2023

Fixes the issue described in this comment as a regression from #81221.

A Connection Dock's TreeItem needs to be selected for the connection's PopupMenu, called slot_menu to update accordingly on the spot. However, this doesn't happen. It means that it is either possible to disconnect inherited signals, or the opposite, at seemingly completely random. This PR fixes that.

@dalexeev
Copy link
Member

This fix looks necessary for the TREE_ITEM_TYPE_SIGNAL case as well.

@dalexeev dalexeev requested a review from a team September 16, 2023 18:49
@Mickeon Mickeon force-pushed the fix-connection-dock-popup-for-inherited branch from e85b99c to ff99226 Compare September 16, 2023 18:56
@Mickeon Mickeon changed the title Fix Connection dock's slot popup always allowing edit and disconnect Fix Connection dock's popups always allowing disconnect Sep 16, 2023
@Mickeon
Copy link
Contributor Author

Mickeon commented Sep 16, 2023

That is correct. The issue was just less apparent because most item options worked regardless. I pushed a fix for the signal item type as well.

@dalexeev
Copy link
Member

Maybe do this above, for future proofing? And you can also add a comment.

	TreeItem *item = tree->get_item_at_position(mb_event->get_position());
	if (!item) {
		return;
	}
+	
+	if (item->is_selectable(0)) {
+		tree->set_selected(item);
+	}

@Mickeon Mickeon force-pushed the fix-connection-dock-popup-for-inherited branch 2 times, most recently from efbad5a to 281b0bb Compare September 16, 2023 19:11
@Mickeon
Copy link
Contributor Author

Mickeon commented Sep 16, 2023

Sure thing. I tested it again and it worked identically. A comment, too.

Copy link
Member

@dalexeev dalexeev left a comment

Choose a reason for hiding this comment

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

I haven't tested this, but it makes sense to me and I trust @Mickeon. Thank you!

editor/connections_dialog.cpp Outdated Show resolved Hide resolved
@Mickeon Mickeon force-pushed the fix-connection-dock-popup-for-inherited branch from 281b0bb to 4471e7f Compare September 16, 2023 19:46
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

tbh this code should be reworked to use item_mouse_selected instead of gui_input, but for now this solution works.

@dalexeev
Copy link
Member

tbh this code should be reworked to use item_mouse_selected instead of gui_input

This was changed in #81221 because class items are not selectable.

@akien-mga akien-mga merged commit e09ac40 into godotengine:master Sep 18, 2023
@akien-mga
Copy link
Member

Thanks!

@Mickeon Mickeon deleted the fix-connection-dock-popup-for-inherited branch December 30, 2023 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants