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

Auto-compact editor log when errors are spawning (edited title) #160

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Johnny-the-happy-coder
Copy link

This patch blocks duplicated error messages to be propagated to error handlers.

The propagation to the OS default output is left untouched.

Rationale: The editor, and in-game handlers, have a limited capacity of storing error strings. An error in a physics routine (either caused by scripts or by a defective component of the engine) can be repeated at each physics loop, making the editor unresponsive, or even causing it to crash.

While the correct approach is that of fixing the code that is raising the error unconditionally, having that blocking the editor or causing it to crash is to be considered a critical bug in itself (akin to a non-caught exception).

OSs have their own rate limit strategies, hence it's less urgent to protect them from log spamming.

Example: IK skeletons propagate thousands errors per seconds when preparing the bone structure, if they are enabled before all the bones referenced by the IK skeleton groups are filled. While that is a bug in itself, the editor needs to be protected from becoming unresponsive or crashing due to an error condition incorrectly handled by others part of it.

@Adrian-Samoticha
Copy link

I don’t know how much work that would be UI-wise, but I think the best way to deal with that issue is the approach that VS Code’s Flutter extension uses (or maybe VS Code itself does that, I don’t really know): When the same output is printed multiple times, it just shows it once with a counter appended to it that displays how often that string has been printed by the application.

This way it’s still clear that the error is thrown once per tick, but the memory footprint and rendering overhead should be low.

@Johnny-the-happy-coder
Copy link
Author

Yeah, not a bad idea actually. Shouldn't take long, let me give it a try.

@Johnny-the-happy-coder
Copy link
Author

This is a compromise, but prevents the editor from becoming unresponsive. After a certain number of repeated messages, the editor log collapse method is invoked as if the user pressed the "collapse" button.

This happens only once, and if the user uncollapses the log, they are on their own; this won't help with excessive message generation on other channels, but at least it prevents the editor log to starve the main thread and the rest editor, and eventually cause a crash, when the user isn't aware of the error message being spawned.

@Adrian-Samoticha
Copy link

Yeah, I think that’s a good idea, actually.

@Johnny-the-happy-coder Johnny-the-happy-coder changed the title Limit propagation of error to handlers to 6 consecutive errors Auto-compact editor log when errors are spawning (edited title) Oct 8, 2024
@Spartan322
Copy link
Member

Spartan322 commented Oct 14, 2024

This should be sqaush and does this have a Godot PR and/or proposal? And do you prefer this not to be sent upstream to Godot if this isn't the same as another PR that's already there?

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.

5 participants