Fix HELP / README routines to match Vanilla (with Heretic / Hexen Menu Fixes and Additions)

[andrikpowell]

So I've already integrated these fixes / optimisations in Nyan Doom, but I figured I should probably send them over to DSDA Doom.

The reason these two are together is that most of the changes have to do with the menus... and when I added Heretic / Hexen INFO screen (up to 3-4 screens as opposed to doom's 1-2 screens) support, I realised how wrong the doom HELP / README routines are in PrBoom and DSDA Doom... So this is me cleaning the doom routines up to match Vanilla.

It's worth noting in Nyan Doom, this code is much more complex, as I have an option to disable the dynamic HELP / README screens... In Nyan Doom, I've also extended the routine to 2 screens (Dynamic CREDIT -> Dynamic HELP) if a PWAD HELP/HELP1 lump is not present (with it reverting to the Vanilla number if a PWAD lump is found). I thought it'd be better for DSDA Doom if it actually matched the Vanilla routines.

For Raven, I also added support to use the raven menu (with the "Game Files" submenu), as well as Heretic shareware support.

Originally I had the doom and raven HELP / INFO screens merged, but I found that overcomplicated things quite a bit and elected to keep raven menu stuff separate in mn_menu instead, especially due to the RAW screen functionality.

HELP2 support was added for those using Ultimate Doom, but running in a complevel 0-3. (technically it's supposed to be 0-2, but there's really no easy way to detect compatibility when not in-game or demo-playback, since the complevel just resets to whatever the user's default is set to in the config when on the startup screen).

There are also some fixes and integrations regarding "Extended Help Screen" functionality.

[Pedro-Beirao]

We definitely want these changes. Thanks!

The code seems fine and inline with what was done for doom, but maybe I should leave it for the other guys to properly review.

And please remove your name from the comments you wrote.
The names you see on some comments are 20+ years old. No reason to do it now

[andrikpowell]

And please remove your name from the comments you wrote.
The names you see on some comments are 20+ years old. No reason to do it now

Fixed. I mostly just do that for my sanity of remembering what code I've added, but I'm fine amending it for DSDA.

So just to clarify about the Doom HELP / README routines, I've put in significant work and testing to figure out what Vanilla's and DSDA Doom's current routines were.

It's extremely confusing because whatever PrBoom+ had was also incorrect. But just to double check, I also tested various DOS Doom versions to make triple sure what the routines were supposed to be.

This post by JadingTsunami on the Extended Help thread was somewhat useful, but what was weird is that DSDA Doom's didn't match neither what PrBoom nor Vanilla had.

So this is the chart I put together to get the routines how they are in this pull request:
Doom HELP / READ ME Routines

(One minor thing to note is about Nyan Doom's routines is that it has the same forced background for the Dynamic HELP screen as the Dynamic CREDIT screen, so it matches better and is more readable (ofc this is a personal opinion)... so that's why they are put together in the routine in Nyan Doom)

[fabiangreffrath]

Thank you for working on this PR! However, this changes a bit too much to squash all commits into one and is a bit too fine-grained with its 12 commits on the other hand. Could you please try to separate the changes by topic and create e.g. 4 or 5 clean commits that introduce one change after the other?

[fabiangreffrath]

Also, please keep a consistent style with a space in if () checks between the if word and the opening parenthesis.

[andrikpowell]

However, this changes a bit too much to squash all commits into one and is a bit too fine-grained with its 12 commits on the other hand.

That's fair, originally it was just around 6 commits, but I found some stuff I missed / edited some stuff based on feedback, etc.

Could you please try to separate the changes by topic and create e.g. 4 or 5 clean commits that introduce one change after the other?

I have a couple questions about this.

1. This is my first pull request, and I'll admit I'm not super familiar with git functionality. Mostly the way I've approached code thus far is just pushing commits to a repo. As far as I'm aware, it's bad practice to remove, edit, or cherry-pick commits. Please correct me if I'm wrong about this, cuz I absolutely hate pushing a commit where I made a simple minor mistake, and then have to push a new "minor fix" commit to patch it. I'm asking this, because I'm unsure of how I'm supposed to edit the branch / pull request from this point.

2. My second question I would have would be what is it you're exactly looking for in a commit rework. This pull request is sorta tricky because a few commits have prerequisites needed to work.

Take for example, I could rework this pull request to include just these commits:

• Add W_PWADLumpNumExists and W_PWADLumpNameExists
• Fix ReadMe / Help to match Vanilla routines
• Add PWAD HELP2 support
• Show "Read Me!" in Doom II if Extended Help Screens are found
• Fix Raven INFO routines
• Fix Raven exclusive MainMenu
• Add Heretic Shareware support (heretic1.wad)

While I could also combine some of the commits that are more adjacent:

• Add W_PWADLumpNumExists and W_PWADLumpNameExists
• Fix ReadMe / Help to match Vanilla routines / Add PWAD HELP2 support / Extended Help "Read Me!" support in Doom II
• Fix Raven INFO routines and MainMenu
• Add Heretic Shareware support (heretic1.wad)

Hope I'm not being too annoying, I'm just really trying to clear this up for when I do pull requests in future, I can avoid things like this.

Also, please keep a consistent style with a space in if () checks between the if word and the opening parenthesis.

Sure I can do this, although I will say the DSDA Doom codebase is quite inconsistent in this regard.

[fabiangreffrath]

1. This is my first pull request, and I'll admit I'm not super familiar with git functionality. Mostly the way I've approached code thus far is just pushing commits to a repo. As far as I'm aware, it's bad practice to remove, edit, or cherry-pick commits. Please correct me if I'm wrong about this, cuz I absolutely hate pushing a commit where I made a simple minor mistake, and then have to push a new "minor fix" commit to patch it. I'm asking this, because I'm unsure of how I'm supposed to edit the branch / pull request from this point.

As long as this is still a pull request, everything happens in a branch in your respository. So, it's perfectly fine for you to do whatever you think is necessary and force-push the branch.

2. My second question I would have would be what is it you're exactly looking for in a commit rework. This pull request is sorta tricky because a few commits have prerequisites needed to work.

I prefer an "every logical step is a commit" approach. I'd try to avoid "fix white space in an unrelated function while I am here" types of commits and also "fix typo in my previous commit" type of commits.

In the examples you gave below, I'd thus prefer the first approach.

Hope I'm not being too annoying, I'm just really trying to clear this up for when I do pull requests in future, I can avoid things like this.

You're not annoying, thank you for working on this! I admit it's a bit difficult to match somebody else's style and everything I request from you here is strictly a matter of taste. Technically, your code change is already fine and I could just merge it as it is.

Sure I can do this, although I will say the DSDA Doom codebase is quite inconsistent in this regard.

That's right, but we should be consistent with newly added code at least.

[andrikpowell]

Ok, so I've just gone through and simplified my commits. Everything should be much more straight forward now :)

[fabiangreffrath]

Looks very good now, thank you!

[Pedro-Beirao]

Thanks to both!