Implement toast notifications in the editor

[groud]

Base implementation for godotengine/godot-proposals#1489.

Old designs

 

Edit : New design:

For now, it only pops up the warning and error messages from the built-in error system (basically the ones we have in the output panel). I was not sure whether this was too much or not, but, in the editor, it makes a lot more noticeable some errors that could have been missed otherwise. I am in favor of keeping it, but well, it's to be discussed.

To be noted, this is also a simple way to trigger a message from code in the "/scene" folder. This is a quite useful feature that would be good to have anyway. Whatever we do, we should probably find a way to trigger a popup from the core or scene code anyway. (maybe a bool arg to ERR_FAIL_* ?).

I put it as draft so that we can discuss whether to add other things in the API, and because I need to implement a way to limit the amount of message on the screen.

Bugsquad edit: This closes godotengine/godot-proposals#1489.

[Calinou]

From an UI perspective, I'd increase the padding around the text and reduce the rounded corners (they look bad at higher radii when there's a highly visible border).

[AwayB]

I put it as draft so that we can discuss whether to add other things in the API, and because I need to implement a way to limit the amount of message on the screen.

I think it's a great idea.

Also from an UI perspective:

1. Having a small colored border on every message would help a lot. Red for errors, yellow for warnings, and if it becomes a function that users can call, let whatever colour users want to set. Understanding the message's nature before reading it is always good.
2. If you want to decide on a limited amount of messages, maybe make it 4 or 5 tops and have the earliest messages removed first.
3. If this is to be implemented, maybe have a checkbox in the options to turn it off, as well as timeout for the notifications and/or a custom number of max notifications on the screen.
4. I imagine at least the error messages will already be logged in the console or somewhere, but if they are not, these toasts should be logged in case they aren't read in time, and clicking on them should possibly open the log.

Just my 2 cents.

[Calinou]

As for the fade duration, I suggest a few changes to improve usability in future PRs:

• Increase the default toast duration to 5 seconds, then add an editor setting to increase it up to 30 seconds for accessibility purposes. Some people read slower than others, and 3 seconds is often not sufficient for longer messages. For comparison, Windows has a setting for notification duration in its Accessibility settings.
  • Other than that, I wouldn't bother adding too many editor settings. A setting to adjust the maximum number of toasts displayed at once doesn't seem very useful to me. Instead, adapt the maximum number of toasts dynamically depending on the editor window's height.
• Don't fade a toast if the mouse cursor is currently on it. If fading already started when the mouse enters the toast, reset the fade timer and set the opacity back to 1.0.
• Add a subtle "progress bar" behind toasts so that you know when they're about to fade. This can be a 3 pixel-thick translucent line at the top or the bottom of a toast.

[groud]

From an UI perspective, I'd increase the padding around the text and reduce the rounded corners (they look bad at higher radii when there's a highly visible border).

I don't think they look bad but ok, I'll reduce their size a bit (the radius is 5px here).
I still like them to be distinct from the overall editor interface, so I think rounded corners are a good fit IMO.

Having a small colored border on every message would help a lot. Red for errors, yellow for warnings, and if it becomes a function that users can call, let whatever colour users want to set. Understanding the message's nature before reading it is always good.

This is already implemented.

If you want to decide on a limited amount of messages, maybe make it 4 or 5 tops and have the earliest messages removed first.

Yes, that makes sense.

If this is to be implemented, maybe have a checkbox in the options to turn it off, as well as timeout for the notifications and/or a custom number of max notifications on the screen.

I don't think this is worth a settings, whatever the situation you are in, you don't want to see your screen covered in messages. Since warning/errors end up in the output panel too, there's no need to disable this limit.

Increase the default toast duration to 5 seconds, then add an editor setting to increase it up to 30 seconds for accessibility purposes. Some people read slower than others, and 3 seconds is often not sufficient for longer messages. For comparison, Windows has a setting for notification duration in its Accessibility settings.

I can go that route. Though I was wondering if I could simply modify the time according to the string length instead ?
I am not against a settings for that, but considering I have just added support for keeping the notification visible when hovering any of them, I am not sure it is worth. I'd rather see later on if there's any complain about it.

Don't fade a toast if the mouse cursor is currently on it. If fading already started when the mouse enters the toast, reset the fade timer and set the opacity back to 1.0.

As mentioned, I implemented the "stay visible on hovering" thing, but it's a little bit more complex to reset the state. So for now it will disappear anyway if it started to fade out. It's not a big problem though, as the fade out animation is quite quick.

As suggested by @fire on rocket chat, I can add a progress bar somewhere though, to notice the user of the timeout. It would be more obvious that the popup is about to disappear.

[EricEzaM]

Hey that's my proposal! 😄

Nice one, looks good! Implementation looks good to me. I think some options could be added to the notifications dialog for max messages, and disabling the notifications from popping up. This could allow them to be more accessible, rather than being hidden away in editor settings. Most likely for different PR though. In settings there would be something like default display time, default maximum shown, etc as discussed here already.

I'm not sure if this can be done, but having the "progress bar" be a thin top or bottom border that fades to the edge would be a good, subtle indicator. This could also match the warning/error color, and the side colored border could be removed. Basically what Calinou suggested. Might have to be manually drawn rather than a border.

Keen to have a play with this and add functionality if it gets merged! And of course add things that use it, as described in the proposal.

[groud]

Nice one, looks good! Implementation looks good to me. I think some options could be added to the notifications dialog for max messages, and disabling the notifications from popping up

You can already disable notifications by clicking the bell icon. It will still display the little red/yellow circle if messages popped up, but it wont show them. I

I don't think configuring the max message is interesting. I limited things it to 5 notifications, and since messages are merged into a single notification when the String is the same, almost all messages are quite sure to be displayed anyway. So I would not bother adding a settings for that.

I'm not sure if this can be done, but having the "progress bar" be a thin top or bottom border that fades to the edge would be a good, subtle indicator. This could also match the warning/error color, and the side colored border could be removed. Basically what Calinou suggested. Might have to be manually drawn rather than a border.

I can try something like that, I'll test that.

[groud]

I updated things. Here is how it looks now:

There's a subtle background progress bar, which gets reset when you hover notifications. Also, the number of similar notifications gets added at the end of the line.

[name-here]

Regardless of interestingness, different users may have different opinions on what the "right" number of messages to show is—especially if custom editor tools and stuff can send them, or for people with and unusually large/small display. I think an editor option would be warranted.

Also, not sure clicking the bell is the best button for toggling notifications. Personally, I would expect clicking it to bring me to a list of all the notifications that have been emitted—again, especially if these can come from custom scripts.

[groud]

Regardless of interestingness, different users may have different opinions on what the "right" number of messages to show is—especially if custom editor tools and stuff can send them, or for people with and unusually large/small display. I think an editor option would be warranted.

Sorry I still don't think this is necessary. As mentioned, most messages will end up merged together, and I don't think there's any situation where you would need more than 5 different messages displayed at the same time. Even if triggered from scripts. As we usually do with Godot, I'd rather see someone facing the problem first instead of speculating that there will be a situation like that later on. Too many settings are bloating the settings area, making it more difficult to find the relevant ones, so we have to be careful there.

That being said, I guess I could increase a little bit this number depending on the window's height, that could be useful on tall screens.

Also, not sure clicking the bell is the best button for toggling notifications. Personally, I would expect clicking it to bring me to a list of all the notifications that have been emitted—again, especially if these can come from custom scripts.

That's a good point. I am ok with this behavior but we need a quick way to disable the notifications too, and I'd rather not have two buttons next to each other. @EricEzaM 's solution would be ok, but the fact the icon is at the top would make it move up and down depending on the number of notifications. Also, that mean that the button would not be visible when notifications are hidden (though I guess we could make the main "show notifications" button re-activate the notifications then)

[groud]

So I've worked a few days on that, here is the last version I have:

Clicking the notification button now spawns again the last 5 temporary notifications, and there's now a button to the right of the list so that you can disable the notifications if you want. Clicking the main button enable them again.

[reduz]

I did not go in depth with reviewing this yet, but here is some feedback:

• The time animation before the fade makes me nervous because it makes me thing I have to act on this before its over somehow. I would just remove it as I think its unnecessary, only would leave it for processes that do some sort of progress on background.
• I am unconvinced that regular errors (ERR_FAIL) stuff should be shown here, in general for the most part they are designed to not be thrown due to things that happen in the editor. For editor related notifications I think its better to have a dedicated API, or have an API similar to it that does show them in the editor. Either way, it would be good to discern between errors the user can do anything about or some core unrelated error that may not be responsibility of the user.

[afk-mario]

I think instead of removing the time animation a button to dismiss the notification could be added.
One thing I like from a toast library is that the time animation stops while you are hovering the notifications indicating that you can hover it an read with calm the message.

React toast library with a lot of nice small micro interactions.
https://fkhadra.github.io/react-toastify/

[groud]

I think instead of removing the time animation a button to dismiss the notification could be added.

I think I'll most likely make the fade-in faster, it should be fine. The dismiss button is a good idea though, I can add it I believe.

One thing I like from a toast library is that the time animation stops while you are hovering the notifications indicating that you can hover it an read with calm the message.

This is implemented.

[groud]

I added a close button:

Peek.30-09-2021.12-24.mp4

[EricEzaM]

In your latest video, how come those two aren't collapsing into one? They are the same message, but it seems only the second instance merges duplicates?

[groud]

In your latest video, how come those two aren't collapsing into one? They are the same message, but it seems only the second instance merges duplicates?

The error messages are the same but they don't have the same tooltip (the tooltip has the code file and line where the error happened). Since the two errors are triggered in different places they are kept separated.

[akien-mga]

Thanks!

[follower]

TL; DR:

• Please consider removing the "timeout/progress indicator" as it creates an unnecessary sense of urgency.
• Please consider only using toasts for existing errors/warnings that are immediately useful and actionable, not all of them.

---

Feedback on duplicating errors/warnings as toasts

FWIW after experiencing this in recent builds, I'm wondering what is the intended benefit of toasts for people using Godot?

Godot is already quite "noisy"

Even ignoring the pre-alpha nature of Godot nightly builds I already tend to feel that Godot is already rather "spammy" with error/warning messages that seem both not particularly important & not something I can really do anything about--which means I get in the habit of ignoring the error/warnings tabs because of the noise--and thus missing actual helpful errors/warnings.

(And I'm someone who actually has some hope of deciphering the error/warning message, not someone making their first game in Godot who can't understand the message, let alone do anything about it.)

I almost immediately turned off toasts

So, even aside from the multi-threading/recursion issues encountered subsequently, it only took me a short while before I turned the toast feature off entirely.

Opportunities for refinement

In terms of specific feedback:

• I very much agree with Implement toast notifications in the editor #52940 (comment) in that the "timeout/progress indicator" creates an unnecessary sense of urgency and just generally contributes to the "busy-ness"/"noisy-ness" of the feature implementation.

• My impression is that part of the motivation of the feature is to ensure that people don't miss important/useful errors/warnings, which is admirable but do we know why are people are missing warnings/errors in the first place?

  If they're not useful/actionable (which has certainly been my experience in many cases) what benefit is there to repeating them as seemingly urgent toasts?

I would suggest not making existing warnings/errors into toasts automatically but only those which can be verified as actionable.

---

An example "non-actionable" warning/error message

In terms of a specific example even Godot 3.4 is still spamming this after I don't know how many years:

WARNING: XOpenIM failed
     at: initialize (platform/x11/os_x11.cpp:197)
WARNING: XCreateIC couldn't create xic
     at: initialize (platform/x11/os_x11.cpp:494)

And current Godot 4 has upgraded this to an error, for some unknown reason (despite it still using WARN_PRINT):

ERROR: XOpenIM failed
   at: DisplayServerX11 (platform/linuxbsd/display_server_x11.cpp:4313)
ERROR: XCreateIC couldn't create wd.xic
   at: _create_window (platform/linuxbsd/display_server_x11.cpp:4133)

And now also repeats the message every time a tooltip is drawn (due to tooltips being separate windows, I guess):

ERROR: XCreateIC couldn't create wd.xic
   at: _create_window (platform/linuxbsd/display_server_x11.cpp:4133)

From looking further into this particular case, I guess maybe it's useful to warn here:

godot/platform/linuxbsd/display_server_x11.cpp

Lines 4312 to 4314 in a000186

	if (xim == nullptr) {
	WARN_PRINT("XOpenIM failed");
	xim_style = 0L;

But given it seems that on any particular system if it fails once it's always going to fail, I don't think it's useful to report outside of --verbose.

And since we already know it's never going to succeed here, given xim is already null, this never seems like it'll be useful:

godot/platform/linuxbsd/display_server_x11.cpp

Lines 4115 to 4134 in a000186

	if (xim && xim_style) {
	// Block events polling while changing input focus
	// because it triggers some event polling internally.
	MutexLock mutex_lock(events_mutex);
	wd.xic = XCreateIC(xim, XNInputStyle, xim_style, XNClientWindow, wd.x11_window, XNFocusWindow, wd.x11_window, (char *)nullptr);
	if (XGetICValues(wd.xic, XNFilterEvents, &im_event_mask, nullptr) != nullptr) {
	WARN_PRINT("XGetICValues couldn't obtain XNFilterEvents value");
	XDestroyIC(wd.xic);
	wd.xic = nullptr;
	}
	if (wd.xic) {
	XUnsetICFocus(wd.xic);
	} else {
	WARN_PRINT("XCreateIC couldn't create wd.xic");
	}
	} else {
	wd.xic = nullptr;
	WARN_PRINT("XCreateIC couldn't create wd.xic");
	}

So, in this particular case, the message is the result of one or more "bugs" or inefficiencies which lead to a whole lot of noise, every time you test a scene or pop-up a tooltip--now amplified by the toasts.

But these "oh, wonder what that warning/error means, guess it's not important" occurrences are common enough that this certainly isn't the only case, just the one I've probably ignored the longest. :)

---

Thanks for taking the time to consider this feedback. :)

[groud]

Please consider removing the "timeout/progress indicator" as it creates an unnecessary sense of urgency.

Other people asked for it. We'll have more feedback about it once alpha is out, and we'll see.

Please consider only using toasts for existing errors/warnings that are immediately useful and actionable, not all of them.

This is on purpose. I assume you are using development builds, where all errors are pushed as popups to help developers acknowledge errors in their code. For release builds, those won't trigger at all by default, which will lead to a lot less messages popping up.