-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
[GraphEdit] Make connections a property #97449
Conversation
a34bfda
to
9e2003f
Compare
9e2003f
to
a6e7298
Compare
You can get rid of |
Editing connections in the inspector is quite buggy. I think the property should either be hidden or readonly (with a note in the docs). |
fe9aeab
to
b4ea09f
Compare
47b3c06
to
92ce651
Compare
Fixed the editing in the inspector via implementing a new connection "property": |
92ce651
to
46f3b38
Compare
Needs rebase. |
46f3b38
to
e5be03a
Compare
Rebased. |
Thanks! |
-const List<Ref<GraphEdit::Connection>> &GraphEdit::get_connection_list() const {
+const Vector<Ref<GraphEdit::Connection>> &GraphEdit::get_connections() const { This is not renamed in the scripting API, but my module uses it, so it kind of breaks compatibility, in addition to making names different between extensions and core (which we try to keep matching with PRs such as #98132). |
@Zylann I guess we can rename it in the scripting API as GraphEdit is still experimental. What do you think? |
If it's ok to break compat, then that would make sense. |
Fixes #97015
After investigating the issue above, I found no obvious reasons why connections shouldn't be a property.
Main changes:
Additional refactoring/optimization:
Vector
instead ofList
internally for connections (this also removes unreadable list element access, e.g. E->get()->...)Ref<Connection>
consistent (c
->conn
)Note: In order to avoid breaking compatibility, I kept the name
get_connection_list
(just for scripting), but since this name is kind of weird and GraphEdit is still experimental we could change it for consistency.