[Added] More commands to safe list

Moderator: GZDoom Developers

More commands to safe list

Postby Major Cooke » Wed Feb 14, 2018 9:49 pm

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.
User avatar
Major Cooke
QZDoom Maintenance Team
 
Joined: 28 Jan 2007

Re: More commands to safe list

Postby _mental_ » Thu Feb 15, 2018 2:16 am

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.
_mental_
 
 
 
Joined: 07 Aug 2011

Re: More commands to safe list

Postby Rachael » Thu Feb 15, 2018 2:21 am

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)
User avatar
Rachael
Admin
 
Joined: 13 Jan 2004
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle
Operating System: Debian-like Linux (Debian, Ubuntu, Mint, etc) 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: More commands to safe list

Postby drfrag » Thu Feb 15, 2018 5:51 am

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?
User avatar
drfrag
Os voy a romper a pedazos!
Vintage GZDoom Developer
 
Joined: 23 Apr 2004
Location: Spain
Discord: drfrag#3555
Github ID: drfrag666

Re: More commands to safe list

Postby Major Cooke » Thu Feb 15, 2018 5:56 am

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


Code: Select allExpand view
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.
User avatar
Major Cooke
QZDoom Maintenance Team
 
Joined: 28 Jan 2007

Re: More commands to safe list

Postby _mental_ » Thu Feb 15, 2018 5:59 am

It's still bad. What if player had different settings?
_mental_
 
 
 
Joined: 07 Aug 2011

Re: More commands to safe list

Postby drfrag » Thu Feb 15, 2018 6:13 am

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).
_mental_ wrote:It's still bad. What if player had different settings?

I was about to say the same thing.
User avatar
drfrag
Os voy a romper a pedazos!
Vintage GZDoom Developer
 
Joined: 23 Apr 2004
Location: Spain
Discord: drfrag#3555
Github ID: drfrag666

Re: More commands to safe list

Postby _mental_ » Thu Feb 15, 2018 6:53 am

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.
_mental_
 
 
 
Joined: 07 Aug 2011

Re: More commands to safe list

Postby Rachael » Thu Feb 15, 2018 7:01 am

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.
User avatar
Rachael
Admin
 
Joined: 13 Jan 2004
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle
Operating System: Debian-like Linux (Debian, Ubuntu, Mint, etc) 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: More commands to safe list

Postby Major Cooke » Thu Feb 15, 2018 7:19 am

drfrag wrote: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.
User avatar
Major Cooke
QZDoom Maintenance Team
 
Joined: 28 Jan 2007

Re: More commands to safe list

Postby drfrag » Thu Feb 15, 2018 7:47 am

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

Code: Select allExpand view
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.
User avatar
drfrag
Os voy a romper a pedazos!
Vintage GZDoom Developer
 
Joined: 23 Apr 2004
Location: Spain
Discord: drfrag#3555
Github ID: drfrag666

Re: More commands to safe list

Postby Graf Zahl » Thu Feb 15, 2018 11:00 am

CVARs represent the user's configuration. Messing around with them is at the very least considered rude behavior.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: More commands to safe list

Postby Rachael » Thu Feb 15, 2018 11:06 am

Graf Zahl wrote: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. >_<
User avatar
Rachael
Admin
 
Joined: 13 Jan 2004
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle
Operating System: Debian-like Linux (Debian, Ubuntu, Mint, etc) 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: More commands to safe list

Postby Xaser » Thu Feb 15, 2018 11:12 am

Rachael wrote: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.
User avatar
Xaser
anarchivist
 
 
 
Joined: 20 Jul 2003

Re: More commands to safe list

Postby Major Cooke » Thu Feb 15, 2018 12:11 pm

Same here. Sounds good to me too.
User avatar
Major Cooke
QZDoom Maintenance Team
 
Joined: 28 Jan 2007

Next

Return to Closed Feature Suggestions

Who is online

Users browsing this forum: No registered users and 0 guests