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

Prevent connect scroll_to_paragraph multiple times to class_desc #92454

Merged
merged 1 commit into from
May 29, 2024

Conversation

jsjtxietian
Copy link
Contributor

Fixes #92405

@YeldhamDev YeldhamDev added this to the 4.3 milestone May 28, 2024
@@ -2340,7 +2340,11 @@ void EditorHelp::_help_callback(const String &p_topic) {

if (class_desc->is_ready()) {
// call_deferred() is not enough.
if (class_desc->is_connected(SceneStringName(draw), callable_mp(class_desc, &RichTextLabel::scroll_to_paragraph))) {
class_desc->disconnect(SceneStringName(draw), callable_mp(class_desc, &RichTextLabel::scroll_to_paragraph));
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we really need to disconnect and reconnect again? Or maybe we can just do
if !connected -> connect

Copy link
Member

Choose a reason for hiding this comment

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

Yes, checking !is_connected is how it's done usually.

Copy link
Contributor Author

@jsjtxietian jsjtxietian May 29, 2024

Choose a reason for hiding this comment

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

If the line is not changed during multiple connect, then yes. I will take a look more closely.

Update: It controls which line to scroll to, IMHO it should favor the latter one, so disconnect and connect again with new line seems more reasonable.

Copy link
Member

Choose a reason for hiding this comment

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

I missed the argument binding, indeed. Makes sense, then.

editor/editor_help.cpp Outdated Show resolved Hide resolved
@akien-mga akien-mga merged commit 4d255a1 into godotengine:master May 29, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@jsjtxietian jsjtxietian deleted the multi-connect branch May 29, 2024 09:26
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.

Calling goto_help on same object multiple times throws error
4 participants