Don't expose underscored signals#112770
Conversation
|
Even if we can work around it, this is also nice for user code, I suppose. I'd let this PR sit for a bit for potential consequences. |
Mickeon
left a comment
Mickeon
left a comment
There was a problem hiding this comment.
Either way, I do approve of it myself and you know it. We've been talking about this ad nauseam
aaronfranke
left a comment
aaronfranke
left a comment
There was a problem hiding this comment.
I think this is a good idea. Signals starting with an underscore are private/protected, like other things starting with an underscore, which also don't appear in the documentation and also don't show up in editor autocomplete.
|
|
||
| if (signal_list.size()) { | ||
| for (const MethodInfo &mi : signal_list) { | ||
| if (mi.name.is_empty() || mi.name[0] == '_') { |
There was a problem hiding this comment.
It dawned on me, how could a signal name possibly be empty?
There was a problem hiding this comment.
No idea, I copied it from methods
godot/editor/doc/doc_tools.cpp
Lines 624 to 625 in 68410ac
I think it's just safeguard, to avoid out of bounds crash on
name[0].
There was a problem hiding this comment.
Rrright, I guess it's faster than begins_with('_') (which probably doesn't even exist).
|
Thanks! |
|
I'm testing Godot 4.6 and both signals and methods starting with an underscore appear in autocompletion. Am I missing something? |
|
Looks like a bug. The signal is not added to the list by any method changed by this PR. Seems to affect only script signals. Open an issue. |
|
I personally don't think we can just hide those completly. For C++ there is an asymmetry: the classes are used in GDScript but they are never written in GDScript. This means there is no situation were you would want to access something private. For script members that's different. Not only the user code is written with autocompletion. Also the original code which works with private members is written with it. Hiding them without some sort of access resolution would make working with such classes easier, but it would make it annoying to develop classes with private members. And well, access resolution isn't exactly straight forward, if we want to do that, we should have consensus on behaviour first. Related: |
|
Right, this is irrelevant for scripts. It's the same for methods and properties. |
6 participants
Inspired by https://github.com/godotengine/godot/actions/runs/19370312655/job/55424339576?pr=102963
This PR makes signals starting with underscore no longer appear in the documentation (including script documentation), nor autocompletion, making them consistent with methods. Interestingly, we don't have any such signal in core, but I know a few cases that were worked around with
add_user_signal().