sv_overridegetcvar

[Rachael]

ZDoom: https://github.com/rheit/zdoom/pull/906
GZDoom: https://github.com/coelckers/gzdoom/pull/122

Synopsis:

Short version:
This forces GetCVar(vid_renderer) to return 1.

Long version:
This enables a server option to force GetCVar to "lie" about what values it actually has stored by returning defaults, instead. In the past, some mods have abused vid_renderer checks to kill or annoy the player if the mod is not playing the renderer they want, essentially trying to force the user to run in a certain renderer. This was designed so that any CVar can be protected, but for now only vid_renderer is. Any CVar that gets the flag CVAR_OVERRIDEGET will always return its default value, no matter its actual setting, if sv_overridegetcvar is set to true.

Note the GZDoom version has two commits. The second commit is intentional - it is to remove the dummy CVar line from an actual ZDoom codebase merge should this get accepted.

[Major Cooke]

Does this include compatibility for Ghoul's Forest 2 which actually kills if its 1?

Also I don't know if there are other mods aside that one which kill based on using vid_renderer 1 or 0. Does anyone else know of any?

In the event, I think a -1 or 2 option should be added to force the renderer to lie as 0 instead.

[Ed the Bat]

Also I don't know if there are other mods aside that one which kill based on using vid_renderer 1 or 0. Does anyone else know of any?

The City of the Damned II: Apocalypse would kill the player if vid_renderer was 0. However, when I spear-headed the project to update it for a re-release in the past year and some, I made damn sure to remove that feature. The differences between renderers in that map are purely cosmetic, and it should be the player's choice.

[Major Cooke]

And Ghoul's Forest 2 would kill if vid_renderer was 1.

It should be able to lie about both. I view this as half-complete.

[Rachael]

It is half-complete. But the other half is not going to be as easy to implement as this was, mainly because now there are going to be different use-cases that have to be excepted and built upon. And I don't like setting arbitrary number hacks, I'd rather have an overridecvars.txt in the main zdoom.pk3 file which reads a maps MD5 sum as well as a list of what to pass to the engine for GetCVar. So far Ghoul's Forest 2 is the only one that I know of that expects vid_renderer to be 0.

That may not even be necessary though - Ed the Bat - do you have any plans to do touch ups on GF2? If so - I believe this can be ignored entirely. Then this can be taken as-is, complete.

[Rachael]

It turns out I may have to expand on this even more. QZDoom's vid_renderer default is 0...

[Ed the Bat]

That may not even be necessary though - Ed the Bat - do you have any plans to do touch ups on GF2? If so - I believe this can be ignored entirely. Then this can be taken as-is, complete.

I could do that, sure. I don't have the endorsement of CutmanMike on it, and there wasn't much else I had in mind to fix in it (it doesn't use as many crappy coding practices as GF3 did, which I have done a revision for), but I'd be happy to fix this issue in the mod itself, especially if it will simplify things engine-side.

[Rachael]

We'll see. With QZDoom throwing everything for a loop, I want to wait and see what Graf says, first. Fixing this may actually be a project, at this point, now.

I could either just accept it, and set QZDoom's renderer default to 1, or override the check from within QZDoom itself, or implement the overridecvars.txt file originally proposed. (Not sure how to start on that project)

As it is, I accidentally implemented a way for a mod to detect whether it is running QZDoom or not. That's not good...

[Graf Zahl]

Overall I am all in favor of this, but with abusable CVARs it always goes both ways, so the end user should be able to set the override value. Otherwise it's inevitable to cause a completely different kind of problem. I also do not think that this should be defined in the source but in the user config.

[Rachael]

What about override_vid_renderer? Any time GetCVar is called it checks for an override_* of any matching CVar, and returns that, instead? And for obvious reasons - GetCVar will not be able to query override_?

[Graf Zahl]

My idea would be a section in the INI and/or a control lump where the user can enter default values for any CVAR they want to protect. This shouldn't just be limited to the stuff you and I may come up - I have seen abuse of too many seemingly benign CVARs that anything should be protectable. I have seen mods that started to play foul, even with something seemingly harmless as 'screenblocks', trying to force a specific setting the player may not have liked. I had to hack a few mods to get rid of this stupid stuff over the years.

Ideally I just would write a small text file with the desired values, and copy it into the WAD/PK3, never to be bothered again by such things.

[Nightfall]

This looks like the beginnings of another mess not much unlike the pain forcing nonsense. We force some ACS function to behave a little differently with a new feature and next we'll see modders want something in return for this. Also I don't think we should pollute the already proliferated CVAR namespace with entries that are completely unnecessary most of the time. Furthermore the proposed solution requires the user to activate this override CVAR and - worse yet - know of it beforehand. Not too great for usability IMO as it will limit this ability only for power users and whoever else would get to know of the solution by them. It'll be buried deep into the bowels of the wiki.

Instead, I would suggest to add this to compatibility.txt. A list of wads that we know to be problematic in this regard, and specify what CVar overrides are needed for these wads to work correctly. That way it's out of the user's hands and works out of the box.

For example:

Code: Select all

12347ASDA8124798AGASDA // dumb map that absolutely wants vid_renderer to be 1 for no good reason
{
    getcvaroverride "vid_renderer" 1
}

[arookas]

Code: Select all

12347ASDA8124798AGASDA // dumb map that absolutely wants vid_renderer to be 1 for no good reason
{
    getcvaroverride "vid_renderer" 1
}

This would only be for old maps not being updated anymore? Because, then developers would have to keep up-to-date with the mod(s) in question in case they force more cvar values, adding them to the compats. If a mod does something you do not like (such as force a renderer, screenblocks, autorun, etc.), maybe simply not play said mod?

Anything in the engine can be abused (player speed, freezing player input, hudmessage + sethudsize, autosave, etc.). Look at terrywads. How do we combat those? By not playing them. For instance:

Code: Select all

script "dont do this" ENTER {
  do {
    if (GetPlayerInput(-1, INPUT_BUTTONS) & BT_JUMP) {
      PrintBold(s:"you can't jump in this mod");
      Thing_Destroy(0, 0, 0);
    }
    Delay(1);
  } while (true);
}

...doesn't even use CVARs. Let's censor GetPlayerInput now.

Adding engine-side counters to these kinds of silly mods (GF2 included) seems like an endless game of catch-up that doesn't seem worth it. It also seems outside the bounds of what compatibility.txt was originally meant to be used for, i.e. objective forcing of compat options to let older mods run properly rather than subjective "hiding" of cvars because people can find CVAR-forcing annoying.

Since this is going to inevitably be added in some way or another, I'll at least vote for this to be purely a user option, just like [wiki=CVARs:Automap#am_showalllines]this is[/wiki].

[Major Cooke]

This looks like the beginnings of another mess not much unlike the pain forcing nonsense.

At least it works...

[Graf Zahl]

Instead, I would suggest to add this to compatibility.txt. A list of wads that we know to be problematic in this regard, and specify what CVar overrides are needed for these wads to work correctly. That way it's out of the user's hands and works out of the box.

For example:

Code: Select all

12347ASDA8124798AGASDA // dumb map that absolutely wants vid_renderer to be 1 for no good reason
{
    getcvaroverride "vid_renderer" 1
}

Good idea, but it won't help a bit with gameplay mods playing stupid.