Add `get_connection_count` function to `GraphEdit` #99564

JekSun97 · 2024-11-22T23:07:44Z

This is my second attempt at this PR, the first one had problems with Git.

Closes: godotengine/godot-proposals#10942

Adds a method to GraphEdit to get the number of connections to a given node and port get_connection_count(to_node,to_port).
With this we can easily limit the number of connections using the condition:

if get_connection_count(to_node,to_port)<1:
	connect_node(from_node,from_port,to_node,to_port)

Test project:
graphedit_test_func.zip

KoBeWi · 2024-11-23T00:17:21Z

You forgot to bind the method.

JekSun97 · 2024-11-23T01:59:26Z

You forgot to bind the method.

Thanks for the comment, I was a bit hasty, fixed

JekSun97 · 2024-11-27T10:52:57Z

I generated documentation with --doctool but the function disappeared from GraphEdit.xml, what could be the reason?

AThousandShips · 2024-11-27T10:54:34Z

Did you build the editor before running doctool?

JekSun97 · 2024-11-27T10:56:44Z

Did you build the editor before running doctool?

Yes, I compiled godot.windows.editor.x86_64.exe in the bin folder, and ran it as bin\godot.windows.editor.x86_64.exe --doctool

AThousandShips · 2024-11-27T10:57:42Z

Then did it vanish or did it get moved to the correct position?

AThousandShips · 2024-11-27T11:12:22Z

That's the version without binding the method so it won't show up, you need to rebuild with it bound

JekSun97 · 2024-11-27T11:30:35Z

Thanks @AThousandShips , I think the PR is ready now (If CI doesn't have problems again)

scene/gui/graph_edit.cpp

doc/classes/GraphEdit.xml

AThousandShips · 2024-11-27T13:30:02Z

You pressed the button to add the changes on GitHub, which creates commits on its own, you will need to squash the changes when you're done

AThousandShips · 2024-11-27T13:33:22Z

You need to pull the changes first, you can just make the changes locally and use git push --force otherwise

JekSun97 · 2024-11-27T15:24:39Z

Everything is finally ready! Thanks again for your help

AThousandShips · 2024-11-27T15:25:42Z

I'll let the GUI team review this but the code looks good

KoBeWi · 2024-11-28T20:23:38Z

doc/classes/GraphEdit.xml

+			<param index="0" name="from_node" type="StringName" />
+			<param index="1" name="from_port" type="int" />
+			<description>
+				Returns the number of connections from [param from_node] to [param from_port] port.


Suggested change

Returns the number of connections from [param from_node] to [param from_port] port.

Returns the number of connections from [param from_port] of [param from_node].

Since I've had a lot of trouble with commit suggestions in the past, to commit and not create a new commit, should I click Commit suggestion or add suggestion to batch ? 😅

Adding to batch will just add one commit for all suggestions. You have to manually copy the suggestion if you want to avoid GitHub's commits.

KoBeWi · 2024-11-28T20:26:07Z

scene/gui/graph_edit.cpp

+int GraphEdit::get_connection_count(const StringName &p_node, int p_port) {
+	int count = 0;
+	for (const Ref<Connection> &conn : connections) {
+		if ((conn->to_node == p_node || conn->from_node == p_node) && (conn->to_port == p_port || conn->from_port == p_port)) {


I'm not sure if this condition is correct. E.g. if to_node is p_node and from_port is p_port (in another node), it could count a connection to another port.

This allows us to check the right ports as well as the left ones, without this only the left or right port is counted

The test project did not reveal any problems, this allows us, for example, to test one node that is connected to itself, from left to right, which is the correct behavior. Since we should count any connections

This returns 2 connections to Port 0 in the middle node.

This is actually happening... I had an idea to create two functions, to check from_node from_port and to_node to_port, but it seems redundant, what else can I come up with to fix this, any tips?

You just have to rearrange your condition, so it always checks the correct port, i.e. use from_port only if connecting from this node and to_port if connecting to this node.

You just have to rearrange your condition, so it always checks the correct port, i.e. use from_port only if connecting from this node and to_port if connecting to this node.

The logic for checking the condition has been changed, now it works as expected

JekSun97 · 2024-12-02T11:17:03Z

I broke everything... Is it possible to create a new clean PR?

AThousandShips · 2024-12-02T11:18:58Z

I would suggest figuring out how to fix it instead, these kinds of mistakes will happen now and again and it's important to not lose the discussion and feedback in a PR by closing one instead of resolving those issues

In this case you should just revert it by using git reset --hard f6c96b and then force pushing after fixing the issues

AThousandShips · 2024-12-02T11:42:23Z

scene/gui/graph_edit.cpp

+int GraphEdit::get_connection_count(const StringName &p_node, int p_port) {
+	int count = 0;
+	for (const Ref<Connection> &conn : connections) {
+		if ((conn->from_node == p_node && conn->from_port == p_node) || (conn->to_node == p_node && conn->to_port == p_port)) {


Suggested change

if ((conn->from_node == p_node && conn->from_port == p_node) || (conn->to_node == p_node && conn->to_port == p_port)) {

if ((conn->from_node == p_node && conn->from_port == p_port) || (conn->to_node == p_node && conn->to_port == p_port)) {

JekSun97 · 2024-12-02T12:08:08Z

This PR was like a Git exam for me, but thanks to AThousandShips' tips, I got through it without opening a third PR, thanks!

All recommended edits have been applied, now this PR is DEFINITELY READY 🎉

AThousandShips · 2024-12-02T12:09:15Z

Once this has been approved please also remove the (Fixed incorrect condition order..) from the commit message, but that can wait until then

Will approve once someone from the GUI/GraphEdit team gives their approval

JekSun97 · 2024-12-02T12:19:45Z

Once this has been approved please also remove the (Fixed incorrect condition order..) from the commit message, but that can wait until then

Will approve once someone from the GUI/GraphEdit team gives their approval

Can I change this now?, before something weird happens to my Git :D

Should I use git commit --amend git push --force ?

AThousandShips · 2024-12-02T12:30:53Z

That would be the way yes!

JekSun97 · 2024-12-02T14:06:52Z

Once this has been approved please also remove the (Fixed incorrect condition order..) from the commit message

Ready

Repiteo · 2024-12-03T20:47:41Z

Thanks! Congratulations on your first merged contribution! 🎉

JekSun97 requested review from a team as code owners November 22, 2024 23:07

JekSun97 changed the title ~~Add get_connetion_count function to GraphEdit~~ Add get_connection_count function to GraphEdit Nov 22, 2024

This comment was marked as outdated.

Sign in to view

KoBeWi added enhancement topic:gui labels Nov 23, 2024

KoBeWi added this to the 4.x milestone Nov 23, 2024

JekSun97 force-pushed the get_connetion_count_method_graphedit branch from ed0ca8c to 8b3d8bc Compare November 23, 2024 01:56

This comment was marked as resolved.

Sign in to view

JekSun97 force-pushed the get_connetion_count_method_graphedit branch from 8b3d8bc to d21b421 Compare November 27, 2024 11:26

AThousandShips reviewed Nov 27, 2024

View reviewed changes

scene/gui/graph_edit.cpp Outdated Show resolved Hide resolved

doc/classes/GraphEdit.xml Outdated Show resolved Hide resolved

JekSun97 force-pushed the get_connetion_count_method_graphedit branch from d21b421 to 3dfad59 Compare November 27, 2024 11:33

This comment was marked as resolved.

Sign in to view

JekSun97 force-pushed the get_connetion_count_method_graphedit branch from 2b27176 to a809bab Compare November 27, 2024 13:49

KoBeWi reviewed Nov 28, 2024

View reviewed changes

This comment was marked as resolved.

Sign in to view

JekSun97 force-pushed the get_connetion_count_method_graphedit branch 2 times, most recently from d8e32df to 4ac3cd3 Compare December 2, 2024 11:02

JekSun97 marked this pull request as draft December 2, 2024 11:06

JekSun97 force-pushed the get_connetion_count_method_graphedit branch from f6c696b to 7d7ed0f Compare December 2, 2024 11:16

JekSun97 force-pushed the get_connetion_count_method_graphedit branch from 7d7ed0f to 7b0ceae Compare December 2, 2024 11:31

JekSun97 marked this pull request as ready for review December 2, 2024 11:33

JekSun97 force-pushed the get_connetion_count_method_graphedit branch from 7b0ceae to 17f0573 Compare December 2, 2024 11:39

AThousandShips reviewed Dec 2, 2024

View reviewed changes

JekSun97 force-pushed the get_connetion_count_method_graphedit branch from 17f0573 to 8865c0f Compare December 2, 2024 11:44

AThousandShips changed the title ~~Add get_connection_count function to GraphEdit~~ Add get_connection_count function to GraphEdit Dec 2, 2024

Add get_connetion_count function to GraphEdit

95ca0aa

JekSun97 force-pushed the get_connetion_count_method_graphedit branch from 8865c0f to 95ca0aa Compare December 2, 2024 13:15

KoBeWi approved these changes Dec 2, 2024

View reviewed changes

akien-mga modified the milestones: 4.x, 4.4 Dec 2, 2024

AThousandShips approved these changes Dec 3, 2024

View reviewed changes

Repiteo merged commit ec333a8 into godotengine:master Dec 3, 2024
20 checks passed

JekSun97 deleted the get_connetion_count_method_graphedit branch December 3, 2024 20:52

This comment was marked as outdated.

Sign in to view

This was referenced Dec 4, 2024

Add a method to get connections to a specific port in GraphEdit godotengine/godot-proposals#11287

Open

Add get_connection_list_from_port function to GraphEdit #100048

Closed

	Returns the number of connections from [param from_node] to [param from_port] port.
	Returns the number of connections from [param from_port] of [param from_node].

	if ((conn->from_node == p_node && conn->from_port == p_node) \|\| (conn->to_node == p_node && conn->to_port == p_port)) {
	if ((conn->from_node == p_node && conn->from_port == p_port) \|\| (conn->to_node == p_node && conn->to_port == p_port)) {

Add get_connection_count function to GraphEdit #99564

Add get_connection_count function to GraphEdit #99564

Conversation

JekSun97 commented Nov 22, 2024 • edited Loading

This comment was marked as outdated.

KoBeWi commented Nov 23, 2024

JekSun97 commented Nov 23, 2024

JekSun97 commented Nov 27, 2024

AThousandShips commented Nov 27, 2024

JekSun97 commented Nov 27, 2024 • edited Loading

AThousandShips commented Nov 27, 2024

This comment was marked as resolved.

AThousandShips commented Nov 27, 2024

JekSun97 commented Nov 27, 2024

This comment was marked as resolved.

AThousandShips commented Nov 27, 2024

This comment was marked as resolved.

AThousandShips commented Nov 27, 2024

JekSun97 commented Nov 27, 2024

AThousandShips commented Nov 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JekSun97 Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KoBeWi Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

JekSun97 commented Dec 2, 2024

AThousandShips commented Dec 2, 2024

Choose a reason for hiding this comment

JekSun97 commented Dec 2, 2024

AThousandShips commented Dec 2, 2024

JekSun97 commented Dec 2, 2024 • edited Loading

AThousandShips commented Dec 2, 2024

JekSun97 commented Dec 2, 2024

Repiteo commented Dec 3, 2024

This comment was marked as outdated.

Add `get_connection_count` function to `GraphEdit` #99564

Add `get_connection_count` function to `GraphEdit` #99564

JekSun97 commented Nov 22, 2024 •

edited

Loading

JekSun97 commented Nov 27, 2024 •

edited

Loading

JekSun97 Nov 28, 2024 •

edited

Loading

KoBeWi Nov 28, 2024 •

edited

Loading

JekSun97 commented Dec 2, 2024 •

edited

Loading