Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 'no specular' support #254

Closed
wants to merge 1 commit into from
Closed

Fix 'no specular' support #254

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 5, 2019

Light entities can have the param 'nospecular' set, which skips the specular lighting.
However this never actually worked in Doom 3.
This fix addresses that.

Using IceColdDuke's PBR interaction shader (which exposes the problem clearly), this is without the fix: (nospecular is set on the laser, yet they are still generated, hence the strange lighting around the laser dot)
https://imgur.com/lIst0NA

With the fix applied (specular lights are properly skipped)
https://imgur.com/qO0m34V

Light entities can have the param 'nospecular' set, which skips the specular lighting.
However this never actually worked in Doom 3.
This fix addresses that.

Using IceColdDuke's PBR interaction shader (which exposes the problem clearly), this is without the fix:
https://imgur.com/lIst0NA

With the fix applied:
https://imgur.com/qO0m34V
@DanielGibson
Copy link
Member

Somehow all the tabs used for indentation have been replaced with spaces, which makes it hard to tell what has actually changed in that function (vs what is just indented differently). It also makes the code look inconsistent in the editor and git blame will also be less useful for the affected code..

Can you try keeping the original indentation style?

@DanielGibson
Copy link
Member

Also, doesn't this change the look of the original game?

@ghost
Copy link
Author

ghost commented Oct 8, 2019

Ahh, the tabs > spacing thing might be related to my text editor.

Afaik it doesn't affect the base game at all. If they actually used nospecular anywhere, they were bound to notice it didn't work, right?

@DanielGibson
Copy link
Member

DanielGibson commented Oct 8, 2019

There's 30 maps that contain "nospecular" "1", 1043 occurrences.
I checked and it's indeed set at light entities.
All those lights will look different if this behavior is fixed..

@ghost
Copy link
Author

ghost commented Oct 8, 2019

Then dismiss the patch.

One could argue that the patch makes Doom3 look like the developers intended, though?

@DanielGibson
Copy link
Member

Maybe there's a way to make sure it's only applied for new maps (where we can assume they have been tested with the fix applied and thus look as intended)?
Like a "usenospecular" "1"flag in the worldspawn?
Do you think that'd be useful for mappers/modders?

@ghost
Copy link
Author

ghost commented Oct 9, 2019

That could be a good solution, yes

@ghost
Copy link
Author

ghost commented Oct 9, 2019

A better approach might be to control it via a cvar?

@DanielGibson
Copy link
Member

Why do you think that would be better?
I thought it would be best to let mappers specify that so the user doesn't have to change a cvar between playing old and new maps

@Arl90
Copy link

Arl90 commented Jan 16, 2020

The idea of a worldpsawn flag is great. I would also add a cvar (disabled by default) to enable the correct behavior of this feature in the entire game.

@ghost ghost closed this by deleting the head repository Apr 10, 2023
DanielGibson added a commit to DanielGibson/dhewm3 that referenced this pull request Jan 19, 2025
based on dhewm#254

The "nospecular" parm will only be used if either
r_supportNoSpecular is set to 1
or r_supportNoSpecular is set to -1 (the default) and the maps spawnargs
contain "allow_nospecular" "1"
@DanielGibson
Copy link
Member

@Arl90 @EoceneMiacid Is anyone still interested in this?

If so, see #643
Please test that and provide a testmap, else I won't merge it!

DanielGibson added a commit to DanielGibson/dhewm3 that referenced this pull request Jan 19, 2025
based on dhewm#254

The "nospecular" parm will only be used if either
r_supportNoSpecular is set to 1
or r_supportNoSpecular is set to -1 (the default) and the maps spawnargs
contain "allow_nospecular" "1"

This probably doesn't work with (time)demos yet, because I think when
they're being played I can't access the worldspawn entity
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants