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

Allow showing messages from threads in Editor Log #77080

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented May 15, 2023

Previously if EditorLog received a message from another thread it would get silently ignored. So I changed it to instead use call_deferred(), which AFAIK is safe to use this way.

This PR only affects tool scripts and editor errors, runtime thread errors still don't show (although maybe a similar approach could be used to at least report something). Example code that previously would fail silently:

@tool
extends Node

var thread

func _ready() -> void:
	thread = Thread.new()
	thread.start(test)

func test():
	call("error")

@akien-mga
Copy link
Member

Makes sense, though we should make sure we won't trigger situations with infinite feedback loops such as the ones EditorToaster can trigger: #76820.

Would it make sense to defer all messages, even on the main thread?

@KoBeWi
Copy link
Member Author

KoBeWi commented May 15, 2023

Makes sense, though we should make sure we won't trigger situations with infinite feedback loops such as the ones EditorToaster can trigger

I think infinite loop is not possible. Also this PR simply makes some warnings/errors that only appeared in the external console to appear in the editor log. I don't recall anything spammy enough to overload the log.

Would it make sense to defer all messages, even on the main thread?

I just modified an existing code that had a special case for other threads; making everything deferred would work too.

@akien-mga akien-mga merged commit 965db42 into godotengine:master May 16, 2023
13 checks passed
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the spam_error_log_with_multiple_threads_for_better_spam_efficiency branch May 16, 2023 10:15
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.

None yet

2 participants