Anonymous | Login | Signup for a new account | 2024-04-19 23:14 UTC |
My View | View Issues | Change Log | Roadmap | Zandronum Issue Support Ranking | Rules | My Account |
View Issue Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||||
ID | Project | Category | View Status | Date Submitted | Last Update | ||||
0002633 | Zandronum | [All Projects] Suggestion | public | 2016-02-13 14:25 | 2018-09-30 21:40 | ||||
Reporter | Dusk | ||||||||
Assigned To | Dusk | ||||||||
Priority | normal | Severity | minor | Reproducibility | have not tried | ||||
Status | closed | Resolution | fixed | ||||||
Platform | OS | OS Version | |||||||
Product Version | |||||||||
Target Version | 2.2 | Fixed in Version | 2.2 | ||||||
Summary | 0002633: Remove sv_limitcommands from release builds | ||||||||
Description | I 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 Files | sv_limitcommands.diff [^] (2,608 bytes) 2016-02-13 15:59 [Show Content]
sv_limitcommands-2.diff [^] (3,663 bytes) 2016-02-13 21:13 [Show Content] | ||||||||
Notes | |
(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? |
(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. |
(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. |
(0014432) Dusk (developer) 2016-02-13 21:13 |
New diff attached. |
(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? |
(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. |
(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 [^]' |
(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:
|
(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. |
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 |
Copyright © 2000 - 2024 MantisBT Team |