Pause menu

Open-source port of PoP that runs natively on Windows, Linux, etc.
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: Settings window

Post by Norbert »

Falcury wrote: January 22nd, 2018, 2:39 pmBit much for now, but something to think about for later: maybe display a list of mods, and allow switching to another levelset on the fly.
Related: I think maybe instead of manually setting the mod in SDLPoP.ini, the program could check how many directories are in mods/, and if there is one or more, then during startup display a scrollable list with selectable entries of all mod directory names, plus an entry to pick the original game.

But probably better to complete current works in progress - and comment out/remove what's far from ready - to work towards a new release.
As David recently wrote,
David wrote: January 27th, 2018, 4:24 pmI guess it's time to make a new release...
A week from now it'll be a year since the last release.
David wrote: January 27th, 2018, 5:06 pm
Falcury wrote: January 22nd, 2018, 2:39 pm Settings are now saved to a file SDLPoP.cfg. Basically, your changed settings will be remembered from that file,
I see it's a binary file.
I haven't looked into this (it's 0.54am), but based solely on David's remark I'll chime in that I'd personally advise against saving settings in binary files. If it's too difficult to read and write SDLPoP.ini, then maybe the format of SDLPoP.ini should be changed?
Falcury
Calif
Calif
Posts: 565
Joined: June 25th, 2009, 10:01 pm

Re: Settings window

Post by Falcury »

Did not get around to posting about my progress earlier...

I fixed the remaining issues, as far as I can tell at least.
I disabled the CHEATS menu.
The menu is no longer redrawn if there is no mouse or keyboard/controller input, which fixes the constantly high CPU load.

Additions:
* Added an option for disabling all customizations at once.
* Added confirmation dialog for quitting the game using the pause menu.
* Made navigating the menu using a controller faster (can hold down a direction for repeated movement).
* Restoring all settings to the default state now works (under the 'GENERAL' menu). Added a confirmation dialog box, to prevent accidental use.
* Integrated the fuzzy pixels feature as a setting that can be changed in-game.
* Turning the music off now works.
* Controller buttons 'start' and 'back' now trigger the menu.
* Clicking the screen now brings up the menu.

There are no longer any serious to do items on my list.
The only open issue is: how to prevent mod settings and user settings from interfering with each other, in the case of using a custom levelset.
Norbert wrote: January 29th, 2018, 1:54 am I haven't looked into this (it's 0.54am), but based solely on David's remark I'll chime in that I'd personally advise against saving settings in binary files. If it's too difficult to read and write SDLPoP.ini, then maybe the format of SDLPoP.ini should be changed?
An immutable SDLPoP.ini file also has its advantages. If you accidentally mess up some of the settings in the menu (in particular, the customization settings could be 'fragile'), then you still have your original SDLPoP.ini as a fallback. (The binary file can always be safely deleted)
Also, SDLPoP.ini is under version control, which makes it very annoying if it changes all the time.
That said, I understand the hesitation.
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: Pause menu

Post by David »

(I moved the posts about Falcury's pause menu to this new topic, from here.)
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: Pause menu

Post by Norbert »

Falcury wrote: February 4th, 2018, 12:16 amThere are no longer any serious to do items on my list.
Will the pause menu be part of v1.18?
Falcury
Calif
Calif
Posts: 565
Joined: June 25th, 2009, 10:01 pm

Re: Pause menu

Post by Falcury »

Progress:
* Squashed the commits on GitHub into a single commit on a new branch.
* The binary config file SDLPoP.cfg is now also ignored if the CRC-32 of the prince executable changes (ensuring that we don't ever need to worry about the danger of the config file being exchanged between different versions of SDLPoP).
* Menu can now also be navigated using the left analog stick on a controller (instead of only the D-pad).
* Changed Ctrl+M keystroke for opening the menu to Backspace (because that is already usable for closing the menu, I thought this made sense?)
* Suppressed repeated keystrokes for Escape/Backspace when entering or exiting the menu (which would cause rapid pausing and unpausing of the game)
* Updated ChangeLog.txt
* Updated Readme.txt with some very minimal mention of the menu (the keybindings, and how to enable/disable bug fixes). Maybe more explanation is needed, I don't know.
* Refactor: added a macro NAMES_LIST() for declaring and initializing name_list_type structs.
* Other minor stuff (some cleanup and refactoring).

I'll post a test build so people can try it out.

Question:
Should the menu be enabled or disabled by default?
If it's enabled by default, people will be able to discover/stumble upon it by pressing escape. That may encourage people to try out the various possibilities that SDLPoP has to offer, even those people who wouldn't normally touch the configuration file (e.g., because they just want to play the game, or because they are playing on a console platform).

I expect there will still be bugs or unexpected behavior with mods, and replays to a lesser extent. I haven't done enough testing to know for sure.
In particular, it's probably necessary to make sure that mod-specific customizations will not interfere with settings changed by the user, and vice versa.
Right now, if you load a mod that has custom options in a mod.ini file, I think the menu system would (blindly) follow whatever was configured in mod.ini. Then, if you change any setting and close the menu again, SDLPoP.cfg would get saved with those modified settings applied. Then, if you switch to another mod that doesn't override any settings using mod.ini, those settings from the other mod will still linger.

(Hey, come to think of it, actually that is not a problem as long as you cannot switch between mods using the menu. Right now, you can only switch to another mod by editing SDLPoP.ini, which would reset all in-game settings anyway.)
(Follow-up thought: wait, actually, you can switch to another mod without editing SDLPoP.ini, by passing the mod as a command-line parameter. So it would be possible for the problem described above to occur.)

Maybe settings should have a variable attached to them that tracks whether they are managed/owned by the mod, or by the global configuration. And then that variable would determine whether the setting gets saved in SDLPoP.cfg, or not.
Need to think about it some more.

I'd say the menu is now ready as a 'beta' feature, but maybe not ready for merging into the main branch, mainly because of the issues described above.
My own thoughts on the possibility of including it in v1.18:
* Maybe David will want to reject the menu proposal, or maybe he will require large revisions before he will accept it. If so, it should of course not be included.
* If David wants to accept the menu proposal (or with minor revisions), but also wants to release quickly, then it's maybe best to defer the menu until the next version.
* If David wants to try to include the menu in v1.18, and if a delay of the release is acceptable, then I could try my best to quickly address the mod.ini problem described above, as well as any other issues that may still be discovered. Depending on how that works out, David could then decide whether to merge or not before releasing the new version.

EDIT:
Attached test build.
Attachments
SDLPoP_menu2.zip
(4.71 MiB) Downloaded 119 times
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: Pause menu

Post by Norbert »

Falcury wrote: February 10th, 2018, 1:40 amShould the menu be enabled or disabled by default?
I'd personally vote in favor of enabling it by default. It doesn't impact/modify gameplay, and it makes the experience more user-friendly. Plus, as you wrote, it encourages players to explore options. The start-up message could perhaps be modified to mention the menu? (I see, for example, that the quicksave/load and replay keys are now already mentioned in menu entry tooltips.) In the future, the menu might get scrollable HELP and/or KEYS and/or CREDITS and/or CHANGELOG and/or CHEATS and/or who-knows-what-else entries. ;) I think maybe it makes sense to mention in the Readme.txt the menu as one of your contributions, given the amount of work you put into it and how it improved the package.

As for including or not including it in 1.18. It's up you David and you, of course, but maybe give yourself time to tinker with it until you're more confident about the cfg/ini situation, as that seems to be the only thing that impacts features that were already available (and change, instead of are build upon)? I don't think it's a problem to release 1.19 a couple of weeks, or even a week or even a day after 1.18.

Then again, one could say that being able to put out new releases whenever is an argument to include the menu in 1.18 as-is and work out any remaining quirks later, especially from the RERO perspective. Also, things can be improved ad infinitum, so 'feature creep' could excessively delay new releases. For instance, I was thinking you could make ON/OFF entries also togglable with Enter. But that's polishing that's not necessary for a new release.

[Edit: I see that in the MODS sub-menu even the level specific options are customizable. Nice. I'm not sure whether the quicksave time penalty should be on by default...]
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: Pause menu

Post by David »

I merged #153 into master, then I tried to merge #154, but there were some ugly merge conflicts.
So I undid the merge with `git pull`, `git reset --hard <previous revision>` and `git push --force`.
Is it okay If I merge only #154? Since it "Includes #153".

(Why does #153 still say "merged commit"?)
Falcury
Calif
Calif
Posts: 565
Joined: June 25th, 2009, 10:01 pm

Re: Pause menu

Post by Falcury »

David wrote: February 11th, 2018, 4:54 pm I merged #153 into master, then I tried to merge #154, but there were some ugly merge conflicts.
So I undid the merge with `git pull`, `git reset --hard <previous revision>` and `git push --force`.
Is it okay If I merge only #154? Since it "Includes #153".

(Why does #153 still say "merged commit"?)
Hey, that's strange, I did not expect that to cause any problems. Sorry about the inconvenience. Yeah, that should be fine!
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: Pause menu

Post by David »

Falcury wrote: February 11th, 2018, 5:09 pm Hey, that's strange, I did not expect that to cause any problems. Sorry about the inconvenience. Yeah, that should be fine!
Right, I merged #154 and fixed the project files: https://github.com/NagyD/SDLPoP/commits/master
David wrote: February 11th, 2018, 4:54 pm (Why does #153 still say "merged commit"?)
I guess GitHub didn't notice that I undid #153 with git reset.
Falcury
Calif
Calif
Posts: 565
Joined: June 25th, 2009, 10:01 pm

Re: Pause menu

Post by Falcury »

David wrote: February 11th, 2018, 5:29 pm Right, I merged #154 and fixed the project files: https://github.com/NagyD/SDLPoP/commits/master
Thanks! :)
By the way, do you usually update the Dev-C++ project files by hand, or does Dev-C++ automatically do that when the new file is added?
David
The Prince of Persia
The Prince of Persia
Posts: 2846
Joined: December 11th, 2008, 9:48 pm
Location: Hungary

Re: Pause menu

Post by David »

Falcury wrote: February 11th, 2018, 5:43 pm By the way, do you usually update the Dev-C++ project files by hand, or does Dev-C++ automatically do that when the new file is added?
I have to add the new file by hand, but within Dev-C++. (Project -> Add To Project...)

I'd note here that in Dev-C++, "Save All" won't save the project file.
It seems that the only way to save the project file is to quit, and then answer yes to the "Save changes to SDLPoP?" question.
This has bugged me for quite a while.
Well, actually, there is "Save Project As...", but then I have to select the project file that I just opened. How silly.

At least this is how it works in the current version (5.11).
If I recall correctly, the older version (4.9.9.2) did save the project file when I pressed "Save All".
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: Pause menu

Post by Norbert »

David wrote: February 11th, 2018, 6:04 pmI'd note here that in Dev-C++, "Save All" won't save the project file.
It seems that the only way to save the project file is to quit, and then answer yes to the "Save changes to SDLPoP?" question.
This has bugged me for quite a while.
Maybe file a bug report about it.
https://sourceforge.net/p/orwelldevcpp/tickets/new/
User avatar
Norbert
The Prince of Persia
The Prince of Persia
Posts: 5743
Joined: April 9th, 2009, 10:58 pm

Re: Pause menu

Post by Norbert »

In ChangeLog.txt it says:

Code: Select all

DONE: Added in-game menu (press Escape or Backspace) with ability to change settings while in-game.
Based on what I see on other lines in this file, I'm guessing "DONE" might need to be "ADDED" here.
[Edit: Hm, actually, the use of ADDED appears to be inconsistent. Especially when also looking at other releases. Also, personally, I always keep added entries, changed/fixed entries, and removed entries grouped together. And I usually start with what has been added, then changed/fixed, then removed.]
YURA
The Prince of Persia
The Prince of Persia
Posts: 1425
Joined: February 9th, 2017, 11:12 pm

Re: Pause menu

Post by YURA »

Writes that on the computer there is no file:
VCRUNTIME140D.dll
What is it?

Or it is possible to learn about all functions (keys) of the program SDLPoP - 1.18 ?
Falcury
Calif
Calif
Posts: 565
Joined: June 25th, 2009, 10:01 pm

Re: Pause menu

Post by Falcury »

YURA wrote: February 14th, 2018, 3:41 pm Writes that on the computer there is no file:
VCRUNTIME140D.dll
What is it?

Or it is possible to learn about all functions (keys) of the program SDLPoP - 1.18 ?
Thank you for reporting the problem.
Try replacing prince.exe with the (hopefully fixed) executable in the attachment. Let me know if it works.

I guess the problem was that the build script passed the "/MD" option (or "/MDd"), telling the MSVC compiler to dynamically link that DLL file which contains parts of the C runtime library. See:
https://msdn.microsoft.com/en-us/library/abx4dbyh.aspx

However, that DLL is clearly not present on everyone's system. Maybe the build.bat script should be changed to statically link the CRT, using the "/MT" option. (And fix "release" and "debug" modes being messed up, which I guess is the reason why the missing DLL was VCRUNTIME140D.dll and not VCRUNTIME140.dll)
Attachments
prince.exe_fixed_CRT_dependency.zip
(186.66 KiB) Downloaded 136 times
Post Reply