WohlSoft / LunaLua

LunaLua - LunaDLL with Lua, is a free extension mod for SMBX 1.3 game engine, core of the X2 project.
https://codehaus.moe/
GNU General Public License v3.0
33 stars 12 forks source link

Add Misc.toggleFullscreen() and Misc.runWhenUnfocused(bool) #41

Closed SolaceEllery closed 2 years ago

SolaceEllery commented 2 years ago

Up for showcasing today is a new pull request by me. Hopefully it's enough to meet some of SMBX2's standards.

It needs the following lines added to ffi_misc.lua.

Misc.setFullscreen(bool): Toggles the state of fullscreen without using maximize/window clicking. Set it to true to enable fullscreen, set it to false to disable fullscreen. Toggling it off while it's off or on while it's on won't do anything. Misc.runWhenUnfocused(bool): Set it to true if you want it to run when unfocused. Set it to false to set it to not run when unfocused. Misc.isFullscreen(): Returns true or false if it's in fullscreen mode.

Bluenaxela commented 2 years ago

Misc.runWhenUnfocused(bool): is probably mostly fine in concept, except that setting gMainWindowFocused = true; when calling with false causes messy behavior where if the window is unfocused, the user would have to manually focus then unfocus the window again to get it to pause properly. This is not good. When setting "runWhenUnfocused" to false, the game should pause if it's unfocused without any additional fiddling around. The best fix to this is changing the way gStartupSettings.runWhenUnfocused works, so it doesn't prevent setting gMainWindowFocused to false, but instead the places that currently check gMainWindowFocused should probably check gMainWindowFocused || gStartupSettings.runWhenUnfocused.

Misc.toggleFullscreen() seems seems like a questionable idea conceptually, particularly on it's own. Think about the use cases for a moment. Why would code want to toggle the fullscreen state, without knowing the current state? It's fair enough for a menu to have a "toggle fullscreen" option, but surely such a menu option should be implemented as some sort of toggle with a visual indication of what the current state is. That's just common sense user interface design. I can think of no good reason for code to want to toggle fullscreen without also being desirable to query fullscreen state, and if code is able to query that, it sure seems like it'd be more simple and versatile to have a Misc.setFullscreen(bool) instead.

In addition as a more minor thing, there is some inconsistent indentation.

Bluenaxela commented 2 years ago

The indentation changes made in acb8a7a45ba54d209f11b85547e95a06c4a5ef60 are not correct either. The standard setting for this project is 4 spaces per indentation level.

SolaceEllery commented 2 years ago

The indentation changes made in acb8a7a are not correct either. The standard setting for this project is 4 spaces per indentation level.

It looked fine in Notepad++… idk why it’s doing that

As for the other thing, Misc.setFullscreen(bool) sounds better than what I had originally. And I can remove the gMainWindowFocused part if it doesn’t do anything, since idk why I added that in the first place. I may have added it to keep it focused when setting it on/off…? Not sure

SolaceEllery commented 2 years ago

I just updated the pull. You'll need to update ffi_misc.lua for the latest changes.

SolaceEllery commented 2 years ago

Apparently it's not working. I'll test it further

MrDoubleA232 commented 2 years ago

Tiny thing, but it's odd for it to still be called a "toggle" when that's no longer what it does.

SolaceEllery commented 2 years ago

Tiny thing, but it's odd for it to still be called a "toggle" when that's no longer what it does.

I'm gonna actually rename it by the time I get everything settled and fixed

SolaceEllery commented 2 years ago

Got it fixed up and ready to go! You'll need to reupdate ffi_misc.lua

SolaceEllery commented 2 years ago

Added Misc.isFullscreen(). Sorry I've been commenting all over the place, just wanted to get this done. The thing to once again update ffi_misc.lua

SolaceEllery commented 2 years ago

I have a feeling this won't be accepted, so I'm gonna go ahead and close this. Thanks for helping me out on this though!