More commands to safe list

[Major Cooke]

fov, m_pitch, m_yaw, autoaim, and crosshairgrow have been integral to the 'suit zoom' effect of AEons of Death for a long time and now they are no longer accessible. AEoD has an alias for those in particular.

[_mental_]

The initial implementation is not OK. All these CVARs are saved to user’s configuration file.
What will happen if player will exit the game when zoom is active? Player will end up with altered settings.

In general, it's no longer allowed to set not-mod CVARs from KEYCONF aliases.
Alternatively, we can remove this restriction and protect important variables instead.
But this require reviewing of all CVARs and explicitly mark some of them protected.

[Rachael]

I agree with you _mental_ but unfortunately this is a mod-breaking change. The need for FOV binds predates the introduction of A_ZoomFactor into DECORATE, and therefore is used a lot in older mods.

It might be possible to implement a mechanism whereby if a CVAR is changed in an unsafe context (i.e. KEYCONF or ''bind') its value will have to be marked as "dirty" when saving to the user's config so as not to overwrite the user's actual preference.

Another way to handle it might be to give a CVAR an 'effective' value and a 'user set' value and the user set value can only be set directly from the console, itself, or from the menu using the menu's in-built control widgets, or, obviously, when the CVAR is initially read from the ini during initialization. The 'user set' is what gets saved, if a CVAR is marked as 'archive'.

Either way, this of course should be restricted to certain CVARs since we certainly don't want even a mod to access every CVAR. (So the safe/unsafe context would still be needed for them, too)

[drfrag]

Unless i'm missing something now there's no safe list but an unsafe list.
I've tried the old version of the mod from 03/14/16 and what a surprise only one month after the last ZDoom official release it won't run and requires an SVN build, don't you think that's absurd?
Moreover the old mod (6.06.02) crashes in GZDoom 3.2.5 and in the last SVN with an "unsafe state call" DECORATE error message so it won't even run, that's really mod breaking. I don't know when that code was introduced but it's not in ZDoom, i couldn't test the mod but it should run without allowing modification of the user CVARS (no zoom then?). So will this issue really break many mods?

[Major Cooke]

What will happen if player will exit the game when zoom is active? Player will end up with altered settings.

Code: Select all

alias Zoom "fov 45; m_pitch .45;m_yaw .45; set autoaim 0; crosshairgrow 10; echo Zoom in; rebind Zoom_out;"
alias Zoom_out "fov 90; m_pitch 1;m_yaw 1; set autoaim 0; crosshairgrow 1; echo Zoom out; rebind Zoom;"

It's pretty simple enough to fix though, they just press it again twice and presto, it's fixed again.

Also, drfrag, use the latest version. That one works on the latest versions of gzdoom.

[_mental_]

It's still bad. What if player had different settings?

[drfrag]

I've also tried the old version with GZDoom 2.4 and i get the same error so i guess it's a zscript thing.
With ZDoom32 runs but i get no zoom with any of the weapons (i haven't ported the security fixes yet however and there's a lot of pending stuff to do first).

It's still bad. What if player had different settings?

I was about to say the same thing.

[_mental_]

It can be done better by saving affected variables to backup CVARs and restore them after zooming out.
This won't fix 'zoomed in exit' issue but at least will keep user's preferences unaltered.

However the best thing to do is to extend engine with ability to make such zoom feature via scripting without messing with CVARs.
As the last resort (in case of severe compatibility issues) I would add command line switch that disables unsafe context checks.

[Rachael]

Did you miss this?

Another way to handle it might be to give a CVAR an 'effective' value and a 'user set' value and the user set value can only be set directly from the console, itself, or from the menu using the menu's in-built control widgets, or, obviously, when the CVAR is initially read from the ini during initialization. The 'user set' is what gets saved, if a CVAR is marked as 'archive'.

The CVAR field can be extended for this - since it already stores the 'default' value for every CVAR, it can also store a backup copy of the user's actual preferences.

[Major Cooke]

I've also tried the old version with GZDoom 2.4 and i get the same error so i guess it's a zscript thing.

Okay, but did you try to use 6.66 Alpha 4? Yes, the older versions are broken because a line in the duke nukem code didn't have a complete A_Jump statement.

[drfrag]

Doesn't look like that kind of error, i get all these:

Code: Select all

Script error, "AEoDDat.pk3:decorate/monm&m.txt" line 2369:
Unsafe state call in state PhoenixCountdownTick.0 which accesses user variables, reached by PhoenixCountdownTick.Use.

Script error, "AEoDDat.pk3:decorate/monm&m.txt" line 2246:
Unsafe state call in state PhoenixAlliancePrep.2 which accesses user variables, reached by PhoenixAlliancePrep.Use.

Script error, "AEoDDat.pk3:decorate/monportal2.txt" line 403:
Unsafe state call in state AngleMinus1.0 which accesses user variables, reached by AngleMinus1.Pickup.

Script error, "AEoDDat.pk3:decorate/monportal2.txt" line 392:
Unsafe state call in state AnglePlus1.0 which accesses user variables, reached by AnglePlus1.Pickup.

Script error, "AEoDDat.pk3:decorate/monportal2.txt" line 381:
Unsafe state call in state VelZMinus1.0 which accesses user variables, reached by VelZMinus1.Pickup.

Script error, "AEoDDat.pk3:decorate/monportal2.txt" line 370:
Unsafe state call in state VelZPlus1.0 which accesses user variables, reached by VelZPlus1.Pickup.

Script error, "AEoDDat.pk3:decorate/directorl4d.txt" line 51:
Unsafe state call in state SmallCDUp.0 which accesses user variables, reached by SmallCDUp.Use.

Script error, "AEoDDat.pk3:decorate/directorl4d.txt" line 52:
Unsafe state call in state SmallCDUp.1 which accesses user variables, reached by SmallCDUp.Use.

Script error, "AEoDDat.pk3:decorate/directorl4d.txt" line 37:
Unsafe state call in state BuildupToken.0 which accesses user variables, reached by BuildupToken.Pickup.

Also is allowing modification of CVARS that dangerous? I'm thinking about reverting all CVAR protection.

[Graf Zahl]

CVARs represent the user's configuration. Messing around with them is at the very least considered rude behavior.

[Rachael]

CVARs represent the user's configuration. Messing around with them is at the very least considered rude behavior.

And here we are left with an unpleasant situation.

We can block it completely, as _mental_ suggested, and break the mods that depended on the pre-A_ZoomFactor method of changing FOV, or introduce a work-around to allow said mods to work but apply some restrictions on it that get reset the moment a user leaves the game.

I don't personally have a dog in this fight, I don't mind if it's blocked off completely. But I do remember what you said about how important it was for GZDoom to work with backwards compatibility. This is one of those things, unfortunately. >_<

[Xaser]

Another way to handle it might be to give a CVAR an 'effective' value and a 'user set' value and the user set value can only be set directly from the console, itself, or from the menu using the menu's in-built control widgets, or, obviously, when the CVAR is initially read from the ini during initialization. The 'user set' is what gets saved, if a CVAR is marked as 'archive'.

I'm quite fond of this approach. Best of both worlds.

[Major Cooke]

Same here. Sounds good to me too.