Settings window

Open-source port of PoP that runs natively on Windows, Linux, etc.

Moderator: English Moderator Team

User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 3139
Joined: April 9th, 2009, 10:58 pm
Contact:

Settings window

Post by Norbert » July 1st, 2017, 7:20 pm

I've added code for a Settings window to this fork:
https://github.com/EndeavourAccuracy/SDLPoP

It's a work-in-progress and still partially non-functional. Basically, you mouse click the main window and a Settings window shows up. Changes in the "live" column are meant to update the game on-the-fly (in real-time), and changes in the "INI" column are meant to be saved with the "Save" button. I'm hoping Falcury will help me out with some things, I'll mention below. For some other things David's knowledge of the code base and SDL may be helpful.

Some issues that maybe David could look into:
- For some reason Alt+F4 (or clicking the close cross) no longer works.
- Clicking the main window again should (and does) hide the Settings window, but making the Settings window visible again - for some reason - requires two clicks. I don't understand why.

Falcury, where it says "TODO: Save all settings" could you make it save all the INI settings to sPathFile. (sPathFile is already either SDLPoP.ini or mod.ini.) Of course there's no hurry, we're all busy with our jobs and such. I'm asking you because you've worked with/created the 'INI system' and are familiar with related variable names.
[Edit: I mean literally all INI settings, not just what is currently available on the Settings window.]

Also, some variables, such as "enable_controller_rumble", should be split in two. One variable should be used to store the value that was read from the INI file, and one variable should copy that value to be used by the actual game. This will allow the Settings window to both change on-the-fly and keep track of INI settings. For several variables this is already happening, e.g. "start_minutes_left" (INI) and "rem_min" (live).

Keeping all the above in mind, the options currently available on the Settings window already provide some functionality to make real-time changes. It's just that for the game pad rumble, changing the INI setting impacts the live settings and vice versa, minutes left is only visible when pressing Space in-game, and hit points left is limited by the max HP and the game only updates the indicators after taking damage.

I think I've done a proper job making the code flexible enough to easily add additional options to modify. Plus the Settings window already has 7 tabs, that, overall, should provide a fair amount of space for these options.
Attachments
settings.png
Settings window
Last edited by Norbert on July 1st, 2017, 7:46 pm, edited 1 time in total.

User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 3139
Joined: April 9th, 2009, 10:58 pm
Contact:

Re: Settings window

Post by Norbert » July 1st, 2017, 7:31 pm

I'm actually already having fun with this and I think this whole thing could be both useful and entertaining.
When the game starts, wait for the intro to reach the demo fight sequence.
Then open the settings and you can keep the "live" hit points left on 4 to make sure the prince always wins. :)
I'm totally looking forward to being able to turn on and off David's new lighting effects whenever I please.
I'll make sure entering numbers will also be possible by just typing them, not just by mouse clicking arrows.

David
The Prince of Persia
The Prince of Persia
Posts: 1493
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: Settings window

Post by David » July 8th, 2017, 10:35 am

Norbert wrote: I've added code for a Settings window to this fork:
https://github.com/EndeavourAccuracy/SDLPoP
The settings-window commits are on master, maybe you should move them to a new branch?
(I can help you if you don't know how to do that.)

Norbert wrote: - For some reason Alt+F4 (or clicking the close cross) no longer works.
The SDL docs say:
http://wiki.libsdl.org/SDL_EventType#SDL_QUIT wrote: An SDL_QUIT event is generated when the user clicks on the close button of the last existing window. This happens in addition to the SDL_WINDOWEVENT/SDL_WINDOWEVENT_CLOSE event, so the application can check whichever is appropriate, or both, or neither.
There are two windows now, so neither is the last, so there is no SQL_QUIT sent.
You should handle the SDL_WINDOWEVENT with the SDL_WINDOWEVENT_CLOSE subtype instead.

Something like this:

Code: Select all

					case SDL_WINDOWEVENT_CLOSE:
						if (event.window.windowID == SDL_GetWindowID(window_)) { // main window
							quit(0);
						}
						if (event.window.windowID == SDL_GetWindowID(window2_)) { // settings window
							SDL_HideWindow(window2_);
						}
						break;
Maybe windowID should be checked first (in an outermost "if"), and the settings window could have its own separate event handler function.
Currently I can control the prince even it the settings window is active. :)
But not all events have a windowID...

Btw, window_ and window2_ could use a better name...

Norbert wrote: - Clicking the main window again should (and does) hide the Settings window, but making the Settings window visible again - for some reason - requires two clicks. I don't understand why.
I get something different:
When the settings window is focused, the first click on the main window just focuses the main window.
(It might cover the settings window, but it's not closed.)
The second click hides the settings window, and the third click reopens it.

Are you sure this isn't what happens for you?

Norbert wrote: and changes in the "INI" column are meant to be saved with the "Save" button.
What are we going to do with comments?
Maybe just hardcode them, like DOSBox does? (In DOSBox you can save a the config file with config -writeconf <filename>.)

And what about the commented-out options in the [CustomGameplay] section?
Comment out each option if and only if it has the default value?
Or just never comment out any option?


As for your ShowText() and related functions, err...
Since the font is the same, I think you could just use show_text_with_color().
It writes to current_target_surface, so you need to save, change, and restore it, though.

... Except that the settings window has no associated Surface, only a Renderer. :?
I guess I should rewrite my code at some time to use Textures and Renderers instead of Surfaces?...

User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 3139
Joined: April 9th, 2009, 10:58 pm
Contact:

Re: Settings window

Post by Norbert » July 8th, 2017, 12:11 pm

> Something like this: [...]

Doesn't seem to work here.

> Are you sure this isn't what happens for you?

Yes, what you describe does not match what I get.

> Maybe just hardcode them, [...]

Sounds good.

> Or just never comment out any option?

Sounds like a good solution.

> I guess I should rewrite my code at some time to use Textures and Renderers instead of Surfaces?...

I think so.
Quoting from the Migration Guide: "SDL2 still has SDL_Surface, but what you want, if possible, is the new SDL_Texture."

Falcury
Wise Scribe
Wise Scribe
Posts: 326
Joined: June 25th, 2009, 10:01 pm

Re: Settings window

Post by Falcury » July 8th, 2017, 12:32 pm

This is quite inspiring! :)
I have to think about this a bit more...
Norbert wrote:Falcury, where it says "TODO: Save all settings" could you make it save all the INI settings to sPathFile. (sPathFile is already either SDLPoP.ini or mod.ini.)
David wrote:What are we going to do with comments?
Maybe just hardcode them, like DOSBox does? (In DOSBox you can save a the config file with config -writeconf <filename>.)

And what about the commented-out options in the [CustomGameplay] section?
Comment out each option if and only if it has the default value?
Or just never comment out any option?
Hm, I think we could try to just let the raw text be preserved... The INI file including all the comments would be initially read into memory as a kind of 'template'. Then, for saving the changed configuration, the option values themselves would be inserted to replace the original option values (or, if one some option is missing, appended at the end of the corresponding section).
Norbert wrote:Also, some variables, such as "enable_controller_rumble", should be split in two. One variable should be used to store the value that was read from the INI file, and one variable should copy that value to be used by the actual game. This will allow the Settings window to both change on-the-fly and keep track of INI settings. For several variables this is already happening, e.g. "start_minutes_left" (INI) and "rem_min" (live).
So then maybe we should again create some sort of data structure that keeps together all of the settings (like the old options_type struct that was removed some time ago)?
David wrote:Btw, window_ and window2_ could use a better name...
Maybe main_window and settings_window?
And we don't really need the trailing underscores, I suppose? They probably signify that it's a global variable, but most global variables in the codebase don't have them.

David
The Prince of Persia
The Prince of Persia
Posts: 1493
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: Settings window

Post by David » July 9th, 2017, 11:33 am

Falcury wrote:
David wrote:Btw, window_ and window2_ could use a better name...
Maybe main_window and settings_window?
Something like that.
Falcury wrote: And we don't really need the trailing underscores, I suppose? They probably signify that it's a global variable, but most global variables in the codebase don't have them.
That one has an interesting tale...
In the original (DOS) prince.exe, onscreen_surface is not a pointer, but a statically allocated global struct.
But SDL always allocates surfaces dynamically, and they must be accessed through pointers.
So, to preserve what the disassembly did, I wrote something like this:

Code: Select all

extern SDL_Surface* onscreen_surface_;
#define onscreen_surface (*onscreen_surface_)
...
onscreen_surface.field = something;
current_target_surface = &onscreen_surface;
That was quite pointless, and I removed this silliness before the first release.
However, the name remained.

As you said, we don't need the trailing underscores.
I guess it's time to delete the far/near attributes as well, together with the __pascal calling convention.

User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 3139
Joined: April 9th, 2009, 10:58 pm
Contact:

Re: Settings window

Post by Norbert » August 18th, 2017, 10:50 pm

Norbert wrote:I'm totally looking forward to being able to turn on and off David's new lighting effects whenever I please.
Took me a while, but I made that commit.

Maybe the number of torches in the room could impact the darkness of the overlay.
Would make it feel like you're really there, if it's very dark without torches...

Falcury
Wise Scribe
Wise Scribe
Posts: 326
Joined: June 25th, 2009, 10:01 pm

Re: Settings window

Post by Falcury » September 14th, 2017, 11:59 pm

I'm experimenting with adding a menu bar to the SDLPoP main window, taking inspiration from oitofelix's recent additions to MININIM, as well as Norbert's work on the settings window.

So far so good:
window menu.png
Unfortunately there is no easy way to do it in SDL. At the very least, we need separate native implementations for Windows, X11, etc...
SDL normally hides the native window handle and callback. This makes it hard (on Windows) to intercept the WM_COMMAND messages (these are sent each time a menu item is clicked, but apparently SDL does not send these messages through as events; see for instance here). The way I'm getting it to work now is by creating a native window first using CreateWindow(), and then giving that window handle to SDL using SDL_CreateWindowFrom()

User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 3139
Joined: April 9th, 2009, 10:58 pm
Contact:

Re: Settings window

Post by Norbert » September 15th, 2017, 7:51 pm

Experiments can be useful/educational/fun/etc. For anything beyond experimentation, I'd vote against separate native implementations. If SDL alone, such as with the window that I suggested and implemented, is insufficient, if more complex widgets such as menus are truly necessary, it might be time to consider adding a framework or toolkit in the mix that's cross-platform. If you had to rewrite my networking code using Winsock, then I made a mistake and should have used SDL_net or something similar. My 2¢.

salvadorc17
Calif
Calif
Posts: 521
Joined: August 27th, 2011, 2:04 am

Re: Settings window

Post by salvadorc17 » September 15th, 2017, 10:16 pm

Falcury wrote:I'm experimenting with adding a menu bar to the SDLPoP main window, taking inspiration from oitofelix's recent additions to MININIM, as well as Norbert's work on the settings window.
Windows api with sld is not best option, but could be used...

Falcury
Wise Scribe
Wise Scribe
Posts: 326
Joined: June 25th, 2009, 10:01 pm

Re: Settings window

Post by Falcury » September 15th, 2017, 11:42 pm

Hm, maybe GTK?
The Windows API basically provides menu bars for free, though. So on Windows, it wouldn't be necessary to use middleware. I appreciate the concern about native implementations, but... the differences could quite likely be abstracted away. I would expect there to be minimal code duplication in the end. (Yeah, I actually enjoy coding that kind of stuff. ;) )

I expect a menu bar system by itself will not be sufficient for things like editing numbers, like you could do with an actual settings window.
Similarly, I imagine that a settings window is more logical to use for very long lists of options (e.g. choosing which bugfixes to enable/disable, etc.)
And I agree that sticking to SDL for a settings window looks like a good strategy. (It's good that you have experience with GUIs, Norbert... I'm noticing how hard it is...)

A menu system would I think be more convenient for quick toggles, file loading, a reminder of which hotkeys to use, that sort of thing.

I've attached a test build (Windows only). Not all options are working, but it should be enough to try out for testing purposes.
The menu bar auto-hides in fullscreen mode. If you move the mouse to the top of the screen it should become visible again.
Attachments
SDLPoP menubar test.zip
(4.67 MiB) Downloaded 3 times

User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 3139
Joined: April 9th, 2009, 10:58 pm
Contact:

Re: Settings window

Post by Norbert » September 15th, 2017, 11:59 pm

Falcury wrote:I've attached a test build (Windows only).
The better you make this Windows-only thing, the more you are going in the wrong direction.

Falcury
Wise Scribe
Wise Scribe
Posts: 326
Joined: June 25th, 2009, 10:01 pm

Re: Settings window

Post by Falcury » September 16th, 2017, 9:18 am

Norbert wrote:The better you make this Windows-only thing, the more you are going in the wrong direction.
If you're concerned that I might neglect the other platforms, I assure you that I wouldn't even have started on this if I didn't see it as a step towards a good cross-platform implementation with minimal OS abstraction. If it turns out that I can't achieve that, then I'm abandoning the effort. (Maybe I wasn't clear on that...)

Otherwise, I am not really sure what you are saying.

salvadorc17
Calif
Calif
Posts: 521
Joined: August 27th, 2011, 2:04 am

Re: Settings window

Post by salvadorc17 » September 16th, 2017, 10:18 pm

Falcury wrote:
Norbert wrote:The better you make this Windows-only thing, the more you are going in the wrong direction.
If you're concerned that I might neglect the other platforms, I assure you that I wouldn't even have started on this if I didn't see it as a step towards a good cross-platform implementation with minimal OS abstraction. If it turns out that I can't achieve that, then I'm abandoning the effort. (Maybe I wasn't clear on that...)

Otherwise, I am not really sure what you are saying.
Is not that bad, windows api can still be used on others with Wine configurations..

User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 3139
Joined: April 9th, 2009, 10:58 pm
Contact:

Re: Settings window

Post by Norbert » September 16th, 2017, 11:57 pm

Falcury wrote:If you're concerned that I might neglect the other platforms, [...]
The entire approach is fundamentally flawed, because it attacks the platform-agnostic nature of the program. Let me ask you this: do you have the skills and time to not only implement native menus for all operating systems supported by SDL - that's Linux, Windows, OS X, iOS, Android, FreeBSD, Haiku - but can the project also rely on you to keep updating this code until the end of time to keep these native menus functional on all these OSes and whatever else will arrive on the scene, or do you think it might be wise to stick with SDL (and, if truly necessary, a single cross-platform framework or toolkit)? Even if you set up all the development environments required to test these native menus everywhere, do you expect other developers to do the same thing if they want to build on and improve your work? If you can get it to work everywhere except on iOS, will you tell those people 'tough luck'? If you look closely, you may even see the embrace, extend, and extinguish parallels. And if your OS doesn't get the fanciest features, just use Wine. Right, Salvador? ;)

Post Reply