Set main window min. size via Window object instead of DisplayServer to preserve it during window updates.

[bruvzg]

Fixes #70347

Note: when setting min/max window size from the _ready method, it should be done via Window object as well, e.g. get_tree().get_root().set_min_size(Vector2(1000, 1000)).

[YuriSizov]

I think a note should be added to the DisplayServer's method descriptions that calling them without changing the window itself does not work and may lead to bugs. And give that example on how to change the size of the main window too.

That said, while this seems like a correct fix, I still wonder if the current behavior is intended. Do we expect the DisplayServer to be the authority in that regard or do we not? If it is intended, then get_tree().get_root().set_min_size(Vector2(1000, 1000)) is also a significant downgrade in setting main window size compared to 3.x, very verbose, where it previously was just OS.min_window_size = Vector2(1000, 1000). (Same for other related methods/properties).

[Calinou]

Does this help with #62869?

[Riteo]

@YuriSizov

That said, while this seems like a correct fix, I still wonder if the current behavior is intended. Do we expect the DisplayServer to be the authority in that regard or do we not? If it is intended, then get_tree().get_root().set_min_size(Vector2(1000, 1000)) is also a significant downgrade in setting main window size compared to 3.x, very verbose, where it previously was just OS.min_window_size = Vector2(1000, 1000). (Same for other related methods/properties).

IMO the windowing API in general could have quite a few improvements (for example I'd prefer a viewport to be just... A viewport and not have embedded window logic as a specific mechanism mixed all in there along multiple classes or being extended into the Window class) and while I do definitely agree that this API is very unintuitive (even just trying to debug this issue without realizing that it was indeed what this PR was trying to fix was a very confusing task) I think that we're a bit too late for improving this part of the API and that breaking compatibility wouldn't be worth it right now.

That said, as of now the main window's default min/max size is broken and this PR fixes this issue without changing the actual API, using the already existing one "properly", so I'm definitely in favor of this change.

[YuriSizov]

@Riteo So, my question was two-fold.

First, I asked if the current behavior was intended. Because if not, then this needs to be fixed before we go in 4.0. It doesn't require changing any API, only fixing the logic in code, making sure windows respect the DisplayServer as an authority, instead of only informing it about their size and ignoring what it might have stored. But if this is how it's supposed to work, then we don't even need to do that.

However, and this is the second part, having to set the size of the window through the Window node should not be that convoluted for the main window. It's okay for subwindows, as you already have them as nodes in your scenes. But for the main window doing get_tree().get_root().set_min_size(Vector2(1000, 1000)) is way too unintuitive. And it's something that every Godot user will do in their project, because practically everyone has a settings menu which allows to configure the size of the game's window. For that purpose we need to have something straightforward and simple, similar to the old OS.min_window_size = Vector2(1000, 1000), but within the new API. However this is implemented, this also doesn't require any breaking changes.

---

All that said, we can merge this as is, it doesn't hurt. But it's also kind of a hack, and it would be nice to know beforehand if this is intended to be done this way or if the original code was supposed to work but it wasn't implemented proper. Probably only @reduz knows the answer to that.

Adding new API to simplify get_tree().get_root().set_min_size(Vector2(1000, 1000)) can be done with another PR.

[Riteo]

@YuriSizov

But if this is how it's supposed to work, then we don't even need to do that.

No idea, but considering that there is a Window-specific min_size property and it lately even got some updates it definitely feels like what it became. Anyways, as you noted, this is definitely something that only be explained in the grand scheme by @reduz.

However, and this is the second part, having to set the size of the window through the Window node should not be that convoluted for the main window.

Definitely agree.

However this is implemented, this also doesn't require any breaking changes.

That's great!

[YuriSizov]

I confirmed with @reduz that you are not supposed to use DisplayServer directly like we were (and like some users intuitively were).

[akien-mga]

Thanks!