Zandronum Chat on our Discord Server Get the latest version: 3.1
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0002633Zandronum[All Projects] Suggestionpublic2016-02-13 14:252018-09-30 21:40
ReporterDusk 
Assigned ToDusk 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version2.2Fixed in Version2.2 
Summary0002633: Remove sv_limitcommands from release builds
DescriptionI think sv_limitcommands needs the same treatment as sv_useticbuffer. People are disabling it in public servers (e.g. Best Ever) when they really shouldn't be doing that.
Attached Filesdiff file icon sv_limitcommands.diff [^] (2,608 bytes) 2016-02-13 15:59 [Show Content]
diff file icon sv_limitcommands-2.diff [^] (3,663 bytes) 2016-02-13 21:13 [Show Content]

- Relationships

-  Notes
User avatar (0014429)
Dusk (developer)
2016-02-13 15:59

For this to work, version.h needs to be included from sv_main.h. Is this a problem?
User avatar (0014430)
Torr Samaho (administrator)
2016-02-13 16:34

Did you introduce the define HAVE_SV_LIMITCOMMANDS so that people could compile a release version with sv_limitcommands (which would be incompatible with the normal release version)?

I'm not sure if it's worth making sv_main.h depend on version.h. This would mean pretty much everything depends on version.h. It's probably better to move the definition of sv_limitcommands to some other header.
User avatar (0014431)
Dusk (developer)
2016-02-13 19:24

I only had it so as not to copy-paste the BUILD_ID check around, but it could be removed just as well.

Now that I think of it, we could probably use a CVAR_ flag to make it constant and invisible in the console, such as CVAR_CONSTANT. This way we don't have to alter any headers or code at all, only the CVAR declaration. We could then also have CVAR_DEBUGONLY which evaluates to CVAR_CONSTANT in release builds and 0 otherwise.
User avatar (0014432)
Dusk (developer)
2016-02-13 21:13

New diff attached.
User avatar (0014438)
Torr Samaho (administrator)
2016-02-14 15:45

I don't think that CVAR_CONSTANT itself is a useful flag. Off the top of my head, I can't think of any example where you would want a CVAR that is always invisible and unchangeable.

CVARs that are only available in debug mode make a lot of sense though, so IMHO CVAR_DEBUGONLY would be a useful flag. In your implementation, it's no flag though, although it name implies that it is a flag.

In addition to adding CVAR_NOSET in release mode, how about also removing CVAR_ARCHIVE? Or does your current patch already make sure that the cvar is removed from the ini?

BTW: Is it safe to call EnableNoSet in the FBaseCVar constructor? Since this is a static function, doesn't it have a global effect?
User avatar (0014484)
Dusk (developer)
2016-02-23 22:10

I did not realize that m_DoNoSet was a static member. Aagh, what's with that prefix?

As for the cvar naming, I simply didn't realize that I could simply define CVAR_DEBUGONLY as a regular CVAR flag and test whether it should have any effect in FBaseCVar's constructor. I was too convinced that it had to be a macro.
User avatar (0014485)
Dusk (developer)
2016-02-23 22:24

Also realized that there isn't much reason to keep this private either.

'https://bitbucket.org/Torr_Samaho/zandronum-stable/pull-requests/28 [^]'
User avatar (0014515)
cobalt (updater)
2016-02-28 20:24

Issue addressed by commit 5fc7cd2b35ea: Restrict sv_limitcommands to debug builds, addresses 2633
Committed by Teemu Piippo [Dusk] on Wednesday 31 December 1969 23:59:57

Changes in files:

 docs/zandronum-history.txt | 2 +-
 src/c_cvars.cpp | 10 ++++++++++
 src/c_cvars.h | 3 +++
 src/sv_main.cpp | 8 ++------
 4 files changed, 16 insertions(+), 7 deletions(-)

User avatar (0014528)
Dusk (developer)
2016-03-01 22:05

I ran extensive tests of this before the patch was pulled. In a release-identifying build it was impossible to change sv_limitcommands or sv_useticbuffer from the console or from the ini.

Issue Community Support
This issue is already marked as resolved.
If you feel that is not the case, please reopen it and explain why.
Supporters: No one explicitly supports this issue yet.
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2016-02-13 14:25 Dusk New Issue
2016-02-13 14:33 Dusk Status new => needs review
2016-02-13 14:33 Dusk File Added: sv_limitcommands.diff
2016-02-13 15:58 Dusk File Deleted: sv_limitcommands.diff
2016-02-13 15:59 Dusk File Added: sv_limitcommands.diff
2016-02-13 15:59 Dusk Note Added: 0014429
2016-02-13 16:34 Torr Samaho Note Added: 0014430
2016-02-13 16:34 Torr Samaho Status needs review => feedback
2016-02-13 19:24 Dusk Note Added: 0014431
2016-02-13 19:24 Dusk Status feedback => new
2016-02-13 21:13 Dusk File Added: sv_limitcommands-2.diff
2016-02-13 21:13 Dusk Note Added: 0014432
2016-02-13 21:13 Dusk Assigned To => Dusk
2016-02-13 21:13 Dusk Status new => needs review
2016-02-14 15:45 Torr Samaho Note Added: 0014438
2016-02-14 15:45 Torr Samaho Status needs review => feedback
2016-02-23 22:10 Dusk Note Added: 0014484
2016-02-23 22:10 Dusk Status feedback => assigned
2016-02-23 22:24 Dusk Note Added: 0014485
2016-02-23 22:24 Dusk Status assigned => needs review
2016-02-23 22:24 Dusk View Status private => public
2016-02-28 20:24 cobalt Status needs review => needs testing
2016-02-28 20:24 cobalt Note Added: 0014515
2016-03-01 22:05 Dusk Note Added: 0014528
2016-03-01 22:05 Dusk Status needs testing => resolved
2016-03-01 22:05 Dusk Fixed in Version => 2.2
2016-03-01 22:05 Dusk Resolution open => fixed
2018-09-30 21:40 Blzut3 Status resolved => closed






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2024 MantisBT Team
Powered by Mantis Bugtracker