Skip to content

Conversation

slipher
Copy link
Member

@slipher slipher commented Aug 23, 2025

No description provided.

@VReaperV
Copy link
Contributor

Should this really be targeting master branch if a map relies on this bug?

@slipher
Copy link
Member Author

slipher commented Aug 31, 2025

Should this really be targeting master branch if a map relies on this bug?

I think it's fine. Assuming UnvanquishedAssets/map-thunder_src.dpkdir#7 is merged, users will get the updated engine and fixed map at the same time. Those using an engine from master will suffer a minor visual bug (all 5 rods always have lightning). Everyone already suffered this bug for a number of years while shader remap entities were broken.

Copy link
Member

@illwieckz illwieckz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

It looks like there is a missing word in commit message:

Also this fixes a bug that when the replacement shader was not found,
the 'defaulted' shader (one that was not found or failed parsing) for
rendering.

- Don't log if the shader has been registered with those flags before
- Log only once per each new flag combination
Make R_RemapShader handle shader registration flags correctly. For each
version of the shader, register the replacement shader with a matching
set of flags, instead of always using RSF_DEFAULT.

Also this fixes a bug that when the replacement shader was not found,
the 'defaulted' shader (one that was not found or failed parsing) was
used for rendering. New behavior is to not change the replacement state.
Note that Thunder has recently been relying on this bug - see
UnvanquishedAssets/map-thunder_src.dpkdir#7
@slipher slipher merged commit f812b98 into DaemonEngine:master Sep 25, 2025
8 of 9 checks passed
@slipher slipher deleted the remapshader branch September 25, 2025 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants