bevy_transform_gizmo
bevy_transform_gizmo copied to clipboard
Avoid spamming errors in unexpected situations
Spamming errors greatly reduces the usability of a library, if every second, up to 500 errors are printed in the console, it is as good as if the library panicked.
With this PR, bevy_transform_gizmo
will only log stuff if it didn't already emit a similar message within the last second. (note that I downgraded the errors into warnings, since they fundamentally do not break stuff)
I also use system chaining for logging, because of separation of concerns.
Thanks for the PR.
Spamming errors greatly reduces the usability of a library, if every second, up to 500 errors are printed in the console, it is as good as if the library panicked.
Is it possible that the better solution, then, is to panic?
I specifically need to NOT panic. There is a few cases where I do things like remove the cast source, change the camera etc, which bevy_transform_gizmo
doesn't handle all that well. My use case is a fork of bevy_editor_pls
where the gizmo only shows up in editing mode. Panicking in unexpected situations would simply make such a use-case impossible.
My position on this is:
- Batching warnings can make debugging more difficult. It's not obvious or expected that errors should get swallowed up like this, nor is it commonly used elsewhere in the bevy ecosystem. I would prefer predictability over any convenience this adds, in part because:
- This should not be the happy path. If you are getting so many errors that this is becoming a problem, logging isn't the issue, the root cause needs to be solved. These errors are being reported because the plugin can't function as expected while in this state.
- If you know that you are going to be in these states, you should be disabling the plugin or removing the gizmo entity entirely. If this process is enough of a burden that you prefer not doing it, does that point to an issue with the public API of this plugin? Is there something else we need to make it easier to handle the use cases you mention?
Sorry for letting this rot. I intended to reply with specific examples.
Batching warnings can make debugging more difficult. It's not obvious or expected that errors should get swallowed up like this, nor is it commonly used elsewhere in the bevy ecosystem
There is a few counter-examples:
- The hierarchy error added in 0.9 does batch the warning message to avoid spamming it
- Same with the "UI node has parent without Style component" warning
- I wrote a plugin that does batching just this way https://lib.rs/crates/bevy_mod_sysfail (in fact it was inspired by this PR)
To be fair, two of those three were written by me.
you should be disabling the plugin
This might have been the thing I was missing! Well, I'll try migrating to use the disable method. I expect I won't need to silence messages with disabling. Thanks
If this process is enough of a burden that you prefer not doing it, does that point to an issue with the public API of this plugin? Is there something else we need to make it easier to handle the use cases you mention?
If you feel that the above is true, let's reopen and see if we can improve the docs or API around disabling the plugin.