Make Display nullable in IWindow #6501
Conversation
|
Will fix tests soon... |
|
If you want to make it not crash, just ensure that the display is never set to null – in case there is no display, just lie to everyone that the last display is still current. Nullable display will complicated game code for no reason. |
|
I did what you suggested before trying this option, but it ended up with bunch of invalid display errors, so I decided to stop all operations regarding to Display until it becomes available again. |
|
|
||
| default: | ||
| windowLocation = Host.Window.CurrentDisplayBindable.Value.Bounds.Location; | ||
| windowLocation = Host.Window.CurrentDisplayBindable.Value?.Bounds.Location ?? Host.Window.Position; |
There was a problem hiding this comment.
This breaks my mind. Does this fallback really work?
There was a problem hiding this comment.
It should be fine as a fallback according to #6440. It seems to work fine on Linux, too.
| event Action<IEnumerable<Display>>? DisplaysChanged; | ||
|
|
||
| /// <summary> | ||
| /// Gets the <see cref="Display"/> that has been set as "primary" or "default" in the operating system. |
There was a problem hiding this comment.
Needs to explain how/when it can be null.
|
Along with peppy's comment, I'll revisit after I understand a bit more how SDL or X11/Wayland is dealing with display disconnection. Considering that this only happens on Linux (so far), this needs more consideration. |
|
Having no display seems to be an undefined behavior in SDL (obviously). I can probably submit some patches to SDL, but this isn't something to fix because... I think an user needs to fix their monitor first. We just need to handle it more gracefully than crashing out. Either Windows or my GPU driver probably doesn't report no display as I couldn't find code in SDL that keeps at least one display alive, so it doesn't happen on Windows but only happens on Linux. Anyway, it can happen with simple monitor disconnection/sleep and wakeup on Linux, so I think it's better to be as defensive as possible. |
As explained in the discussion, it may be the case that all monitors are turned off and the window is left alone without any displays. Current osu!framework implementation expects there to be at least one display, and such expectation makes the game crash when there is no display available.
To fix the issue, this PR makes Display nullable in IWindow, and adds some checks to make it less unsafe. However, there still needs to be one display on launch. This PR only lets Display nullable later on.
I understand that making Display nullable sounds not so good, but I think it's less surprising than letting the game crash on a sudden monitor disconnection. I made a contact to the issue reporter, and confirmed that this PR fixes the issue.
Feel free to reject this PR if it doesn't feel right for you.
It's hard to add the test case since it doesn't seem to happen on Windows, and it would require manual intervention (turning off the monitor)... I'd like to hear if there is any idea for this.