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

Add Min/Max Window Size Setting #31367

Merged

Conversation

codecustard
Copy link
Contributor

@codecustard codecustard commented Aug 14, 2019

Add project settings to easily set the minimum and maximum window size.

Imitates Test Width/Height Setting, meaning you need to set both the width and the height, otherwise it does nothing. Is that intended behavior?

Closes: #31303

@akien-mga
Copy link
Member

Imitates Test Width/Height Setting, meaning you need to set both the width and the height, otherwise it does nothing. Is that intended behavior?

I don't have a strong opinion for the existing Test Width/Height, but for the min/max window size they don't have to be coupled, you can set only a min or only a max depending on your game's needs.

@Calinou
Copy link
Member

Calinou commented Aug 14, 2019

Imitates Test Width/Height Setting, meaning you need to set both the width and the height, otherwise it does nothing. Is that intended behavior?

It would make sense if you could limit only the width or height, so I'd say it should work even if one of the dimensions is equal to 0. This is especially true if no error message is printed when invalid settings are supplied 🙂

@codecustard codecustard force-pushed the add_minmax_winsize_setting branch 2 times, most recently from 6ddbf57 to 91817e4 Compare August 15, 2019 02:30
main/main.cpp Outdated Show resolved Hide resolved
@akien-mga
Copy link
Member

It would make sense if you could limit only the width or height, so I'd say it should work even if one of the dimensions is equal to 0. This is especially true if no error message is printed when invalid settings are supplied.

Makes sense to change it indeed. I don't see a use case for not applying either test width or height if one is not supplied.

Add project settings to easily set the minimum and maximum window size.
@codecustard codecustard force-pushed the add_minmax_winsize_setting branch from 91817e4 to a7bf643 Compare August 15, 2019 19:19
@akien-mga akien-mga merged commit 822a41a into godotengine:master Aug 17, 2019
@akien-mga
Copy link
Member

Thanks!

It would make sense if you could limit only the width or height, so I'd say it should work even if one of the dimensions is equal to 0. This is especially true if no error message is printed when invalid settings are supplied.

Note that this isn't done in this PR, so it would have to be changed in a new PR.

@akien-mga
Copy link
Member

It crashes on X11 when you start a project:

[1] /lib64/libc.so.6(+0x3caf0) [0x7f5966679af0] (??:0)
[2] /lib64/libX11.so.6(XChangeProperty+0x20) [0x7f5966cbc500] (??:0)
[3] /lib64/libX11.so.6(XSetWMSizeHints+0x10f) [0x7f5966cd942f] (??:0)
[4] OS_X11::set_min_window_size(Vector2) (/home/akien/Projects/godot/godot.git/platform/x11/os_x11.cpp:1289)
[5] Main::setup(char const*, int, char**, bool) (/home/akien/Projects/godot/godot.git/main/main.cpp:942 (discriminator 3))
[6] /home/akien/Projects/godot/godot.git/bin/godot.x11.tools.64(main+0xcd) [0x13e8a3f] (/home/akien/Projects/godot/godot.git/platform/x11/godot_x11.cpp:49 (discriminator 2))
[7] /lib64/libc.so.6(__libc_start_main+0xeb) [0x7f5966663b0b] (??:0)
[8] /home/akien/Projects/godot/godot.git/bin/godot.x11.tools.64(_start+0x2a) [0x13e88ca] (/home/iurt/rpmbuild/BUILD/glibc-2.29/csu/../sysdeps/x86_64/start.S:122)
-- END OF BACKTRACE --

@akien-mga
Copy link
Member

After reverting this PR, calling OS.set_min_window_size() from script seems to work fine, so the problem seem to be that this PR calls it too early before OS is ready to take such VM-specific calls. It would likely have to be passed the the video mode like other window settings.

@akien-mga
Copy link
Member

Reverted by #31435 for now until a better approach can be implemented.

@bruvzg
Copy link
Member

bruvzg commented Aug 17, 2019

so the problem seem to be that this PR calls it too early before OS

It should be called in setup2, after the OS::get_singleton()->initialize call, which is responsible for creation of the main window.

@bruvzg
Copy link
Member

bruvzg commented Aug 17, 2019

It would make sense if you could limit only the width or height, so I'd say it should work even if one of the dimensions is equal to 0.

This should be quite easy to implement with just a few extra checks.

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.

Minimal and maximal window size in project settings
4 participants