Zandronum Chat @ irc.zandronum.com
#zandronum
Get the latest version: 3.0
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0002258Zandronum[All Projects] Bugpublic2015-05-26 05:122018-09-30 21:37
Reporterunknownna 
Assigned ToEdward-san 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version2.0 
Target Version2.1Fixed in Version2.1 
Summary0002258: sv_forcegldefaults is broken
DescriptionThe flag seems to be broken and clients are able to change their gl_light_ambient value when the flag is enabled.
Steps To Reproduce1. zandronum -iwad doom2.wad -host +sv_forcegldefaults 1
2. zandronum -iwad doom2.wad -connect localhost +vid_renderer 1 +gl_light_ambient 255 +gl_distfog 2048 +gl_fogmode 2 +gl_lightmode 0
Additional InformationIt seems to be broken since 2.0.
Attached Files

- Relationships
related to 0001850acknowledgedDusk Software renderer users must be allowed to change r_visibility when sv_forcegldefaults is 0 
related to 0002312closedEdward-san sv_forcegldefaults 1 does not cover gl_distfog changes. 

-  Notes
User avatar (0012406)
Edward-san (developer)
2015-05-26 09:41
edited on: 2015-05-26 10:02

I believe I found a fix for this.

[edit]Does this happen only with gl_light_ambient?

[edit2] Fixed the gl_light_ambient issue with this changeset.

User avatar (0012407)
unknownna (updater)
2015-05-26 10:10
edited on: 2015-05-26 10:11

According to the wiki, it should also happen with gl_lightmode and gl_fogmode.

Quote from Wiki
If this is enabled, some default OpenGL rendering options are enforced, i.e. clients render as if gl_lightmode == 3, gl_light_ambient == 20.0, and gl_fogmode == 1. Users can not change the light/brightness (excluding Gamma) in OpenGL once this is enabled.


User avatar (0012408)
Edward-san (developer)
2015-05-26 11:33
edited on: 2015-05-26 12:03

Quote
According to the wiki, it should also happen with gl_lightmode and gl_fogmode.


So the bug appears also with these two? If yes, which wad or map shows the most evident effects of these settings?

[edit] nvm, managed to find out with normal doom2.wad. Fixed with these other changesets.

User avatar (0012409)
Edward-san (developer)
2015-05-26 12:21

If this will be merged to 3.0 repo, I'd like to change that code for improvements to code style, etc.
User avatar (0012416)
Torr Samaho (administrator)
2015-05-26 17:37

Quote from Edward-san
If this will be merged to 3.0 repo, I'd like to change that code for improvements to code style, etc.

Please elaborate. Does this mean you are unhappy with your own patch? If so, why don't you submit a patch that uses the code style you want?
User avatar (0012418)
Edward-san (developer)
2015-05-26 18:52
edited on: 2015-05-26 18:52

I was thinking about replacing any instance of gl_light_ambient and gl_fogmode with gl_GetLightAmbient and gl_GetFogMode, like it's done with gl_lightmode which has gl_GetLightMode, and remove the OVERRIDE_FOGMODE_IF_NECESSARY macro. Doing this would interfere with 3.0, wouldn't it?

User avatar (0012420)
Edward-san (developer)
2015-05-26 20:17

Here's the cleanup approach.
User avatar (0012429)
Torr Samaho (administrator)
2015-05-27 06:16

The whole purpose of the define was to minimize delta to GZDoom's code. Your approach has a much higher delta and is thus more difficult to merge with GZDoom code base upgrades. Instead, I would actually rather go into the other direction and introduce new defines for the light stuff or extend the current define.
User avatar (0012430)
Edward-san (developer)
2015-05-27 09:46
edited on: 2015-05-27 12:15

But this way, when we'll upgrade the code, with my patch it could be possible to track the move of these cvar instances. This bug happened because these cvars were moved around and the custom code was not updated properly to keep account of this.

[edit] Anyways, Here's the way you suggested.

User avatar (0012471)
Torr Samaho (administrator)
2015-05-31 12:04

Neat idea with the general define, but the fact that one has to copy and paste the default value of the CVAR to where the define is called is not so nice yet. Can you try to get the default value from the CVAR automatically?

Also, the history entry in zandronum-history.txt should not contain things like " After the code upgrade". First, the entries are meant for non-tech-savvy users and should just reflect what has changed since the last version. Second, after a large upgrade like in 2.0, probably half of the fix entries in the next minor version would have to have this prepended.
User avatar (0012472)
Dusk (developer)
2015-05-31 14:36
edited on: 2015-05-31 14:37

Quote from Torr Samaho

Can you try to get the default value from the CVAR automatically?


FBaseCVar has GetGenericRepDefault for this purpose.

User avatar (0012473)
Edward-san (developer)
2015-05-31 15:01

Quote from Dusk

Quote from Torr Samaho

Can you try to get the default value from the CVAR automatically?


FBaseCVar has GetGenericRepDefault for this purpose.


I still need to pass the cvar type to get the value:


cvar.GetGenericRepDefault().Int


if the cvar is not Int, how should we deal with it?
User avatar (0012484)
Edward-san (developer)
2015-05-31 21:45

Managed to work out with this.
User avatar (0012547)
Torr Samaho (administrator)
2015-06-06 15:15

I left a comment on bitbucket.
User avatar (0012579)
Edward-san (developer)
2015-06-07 15:13

Fixed in the same pull request yesterday. Just forgot to 'review' this ticket.
User avatar (0012642)
Edward-san (developer)
2015-06-10 21:14
edited on: 2015-06-10 21:14

Here's a build with the fix:https://www.dropbox.com/s/wnxdk66qp8p3e1e/zandronum-2.1-150606-2115-37376c5-windows.zip?dl=0 [^]

User avatar (0012647)
unknownna (updater)
2015-06-11 07:53

I'm still able to toggle the fog mode (gl_fogmode) in that build, and the light mode (gl_lightmode) setting doesn't seem to change if you connect with gl_lightmode set to 0.

Connect to the server like this:

zandronum -iwad doom2.wad -connect localhost +vid_renderer 1 +gl_light_ambient 255 +gl_distfog 2048 +gl_fogmode 2 +gl_lightmode 0


And speaking of which, I wonder if we should enforce gl_distfog to the default value when sv_forcegldefaults is set to 1.
User avatar (0012648)
Edward-san (developer)
2015-06-11 10:55

Quote from unknownna
I'm still able to toggle the fog mode (gl_fogmode) in that build


Do you mean that you can still see differences in the fog with sv_forcegldefaults is 1? Is it there a map which shows clearly what are you describing? I can't reproduce in doom2 map01, for example.
User avatar (0012650)
unknownna (updater)
2015-06-11 12:47

Yes. If gl_distfog is set to a high value, you'll see it very clearly in MAP01.
User avatar (0012651)
Edward-san (developer)
2015-06-11 14:50
edited on: 2015-06-11 15:36

I see it now. Strangely, every piece of code which uses gl_fogmode should've been already taken care, so I tested 1.3 and noticed that there was still a difference between gl_fogmode 1 and 2, although in my case there were artifacts for value 2. gl_fogmode 0 and 1 behave the same. Can you reproduce it?

User avatar (0012652)
unknownna (updater)
2015-06-11 19:16

Yeah, I'm able to reproduce the gl_fogmode issue in 1.3 and 1.0 as well. However, your fix still doesn't take care of the issue where the client connects with gl_lightmode set to 0.
User avatar (0012654)
Edward-san (developer)
2015-06-11 22:33
edited on: 2015-06-11 23:30

Fixed withhttps://bitbucket.org/crimsondusk/zandronum-sandbox-stable/branches/compare/crimsondusk/zandronum-sandbox-stable:9eda5bb5325a%0DTorr_Samaho/zandronum-stable:default [^] . I'll get a build with it asap.

User avatar (0012655)
WaTaKiD (updater)
2015-06-12 00:16

at Edward-san's request:https://www.dropbox.com/s/idl86hz3r3iho60/zandronum-2.1-150611-2305-9eda5bb-windows.zip?dl=0 [^]
User avatar (0012656)
unknownna (updater)
2015-06-12 01:38

Thanks for the build, WaTaKiD! While it seems to have taken care of the gl_lightmode issue, the gl_fogmode issue is still there, i.e., I'm still able to toggle between gl_fogmode 1 and 2.

And I still maintain that we have to enforce gl_distfog to its default value as well since you're able to effectively turn all fog off by setting gl_distfog to 0.
User avatar (0012657)
Edward-san (developer)
2015-06-12 09:34

Quote from unknownna
the gl_fogmode issue is still there


this could be discussed in the next dev meeting, because this was already present in 1.3 and, until Torr says otherwise, this seems intentional. If we need to improve this, I suggest to force gl_fogmode to 1 also in case gl_fogmode is 2, because that is not supported in very old gpus.

Quote from unknownna
And I still maintain that we have to enforce gl_distfog to its default value as well since you're able to effectively turn all fog off by setting gl_distfog to 0.

Interesting, this must be discussed, too.


Anyways, are there tickets or issues which are 2.0 regressions other than this?
User avatar (0012725)
cobalt (updater)
2015-06-14 19:58

Issue addressed by commit 4419bed7747c: - Fixed: sv_forcegldefaults 1 stopped working properly for the gl clients (addresses 2258).
Committed by Edoardo Prezioso [edward-san] on Friday 12 June 2015 01:05:18

Changes in files:

 docs/zandronum-history.txt | 3 ++-
 src/d_main.cpp | 24 ++++++++++++------------
 src/gl/data/gl_data.cpp | 12 ++++++++----
 src/gl/gl_functions.h | 12 ++++++++++--
 src/gl/models/gl_models.cpp | 6 +++++-
 src/gl/renderer/gl_lightdata.cpp | 19 ++++---------------
 src/gl/renderer/gl_renderstate.cpp | 6 ++++++
 src/gl/scene/gl_walls_draw.cpp | 2 +-
 src/gl/shaders/gl_shader.cpp | 8 ++++++++
 9 files changed, 56 insertions(+), 36 deletions(-)

User avatar (0012735)
Edward-san (developer)
2015-06-15 19:35

Adapted the code to the 3.0 codebase withhttps://bitbucket.org/crimsondusk/zandronum-sandbox/commits/0be3e866b1123015a19f9e631285447de728543b [^] .
User avatar (0012761)
unknownna (updater)
2015-06-20 13:29

Torr, what's your verdict on gl_fogmode?

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
2015-05-26 05:12 unknownna New Issue
2015-05-26 09:16 Edward-san Note Added: 0012405
2015-05-26 09:16 Edward-san Note Edited: 0012405 View Revisions
2015-05-26 09:17 Edward-san Note Deleted: 0012405
2015-05-26 09:41 Edward-san Note Added: 0012406
2015-05-26 09:41 Edward-san Assigned To => Edward-san
2015-05-26 09:41 Edward-san Status new => assigned
2015-05-26 09:48 Edward-san Note Edited: 0012406 View Revisions
2015-05-26 10:02 Edward-san Note Edited: 0012406 View Revisions
2015-05-26 10:02 Edward-san Status assigned => needs review
2015-05-26 10:02 Edward-san Target Version => 2.1
2015-05-26 10:10 unknownna Note Added: 0012407
2015-05-26 10:11 unknownna Note Edited: 0012407 View Revisions
2015-05-26 11:33 Edward-san Note Added: 0012408
2015-05-26 11:39 Edward-san Note Edited: 0012408 View Revisions
2015-05-26 11:40 Edward-san Note Edited: 0012408 View Revisions
2015-05-26 12:03 Edward-san Note Edited: 0012408 View Revisions
2015-05-26 12:21 Edward-san Note Added: 0012409
2015-05-26 17:37 Torr Samaho Note Added: 0012416
2015-05-26 17:44 Torr Samaho Status needs review => feedback
2015-05-26 18:52 Edward-san Note Added: 0012418
2015-05-26 18:52 Edward-san Note Edited: 0012418 View Revisions
2015-05-26 20:17 Edward-san Note Added: 0012420
2015-05-26 20:29 Edward-san Status feedback => needs review
2015-05-27 06:16 Torr Samaho Note Added: 0012429
2015-05-27 09:46 Edward-san Note Added: 0012430
2015-05-27 09:46 Edward-san Note Edited: 0012430 View Revisions
2015-05-27 12:15 Edward-san Note Edited: 0012430 View Revisions
2015-05-31 12:04 Torr Samaho Note Added: 0012471
2015-05-31 12:09 Torr Samaho Status needs review => feedback
2015-05-31 14:36 Dusk Note Added: 0012472
2015-05-31 14:37 Dusk Note Edited: 0012472 View Revisions
2015-05-31 15:01 Edward-san Note Added: 0012473
2015-05-31 21:45 Edward-san Note Added: 0012484
2015-05-31 21:45 Edward-san Status feedback => needs review
2015-06-06 15:15 Torr Samaho Note Added: 0012547
2015-06-06 15:15 Torr Samaho Status needs review => feedback
2015-06-07 15:13 Edward-san Note Added: 0012579
2015-06-07 15:13 Edward-san Status feedback => needs review
2015-06-10 21:14 Edward-san Note Added: 0012642
2015-06-10 21:14 Edward-san Note Edited: 0012642 View Revisions
2015-06-11 07:53 unknownna Note Added: 0012647
2015-06-11 10:55 Edward-san Note Added: 0012648
2015-06-11 12:47 unknownna Note Added: 0012650
2015-06-11 14:50 Edward-san Note Added: 0012651
2015-06-11 15:36 Edward-san Note Edited: 0012651 View Revisions
2015-06-11 19:16 unknownna Note Added: 0012652
2015-06-11 19:16 unknownna Steps to Reproduce Updated View Revisions
2015-06-11 22:33 Edward-san Note Added: 0012654
2015-06-11 23:30 Edward-san Note Edited: 0012654 View Revisions
2015-06-12 00:16 WaTaKiD Note Added: 0012655
2015-06-12 01:38 unknownna Note Added: 0012656
2015-06-12 09:34 Edward-san Note Added: 0012657
2015-06-12 10:57 Dusk Relationship added related to 0001850
2015-06-14 19:58 cobalt Status needs review => needs testing
2015-06-14 19:58 cobalt Steps to Reproduce Updated View Revisions
2015-06-14 19:58 cobalt Note Added: 0012725
2015-06-14 20:44 Edward-san Relationship added related to 0002312
2015-06-15 19:35 Edward-san Note Added: 0012735
2015-06-20 13:29 unknownna Note Added: 0012761
2015-06-24 22:45 Dusk Status needs testing => resolved
2015-06-24 22:45 Dusk Fixed in Version => 2.2
2015-06-24 22:45 Dusk Resolution open => fixed
2015-06-24 22:48 Dusk Fixed in Version 2.2 => 2.1
2018-09-30 21:37 Blzut3 Status resolved => closed






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2020 MantisBT Team
Powered by Mantis Bugtracker