Unexpected CheckForTexture behavior w/ certain sky textures

[JPL]

Doom and Doom 2's skies use the same texture names, but not the same patch names. I resolve this conflict in the IWAD that WadSmoosh generates by renaming the Doom 2 sky textures to RSKY1/2/3 (the same name as the patches they use), and handling custom PWAD skies with some ZScript that detects whether the sky is custom (details below), and setting it on WorldLoaded. As of the 1.2 release last month, this code has correctly handled everything I've thrown at it.

I noticed today that maps 21-30 of Requiem (1997) display a missing texture for the sky using the WadSmoosh IWAD. This issue seems subtle and unrelated to the previous compat challenges I was fixing. I pared down the relevant content and zscript to a minimal example case, ZIP is attached.

To reproduce the issue I'm seeing using only the ZIP (ie without needing to touch WadSmoosh), run this:

Code: Select all • Expand view

gzdoom -iwad doom2.wad -file skytest.pk3 req2.wad -warp 21

This will load you into vanilla Doom 2 MAP21, with a clear view of the sky overhead, but the sky is the white-and-blue missing texture.

Now run this, using another example WAD (based on Alien Vendetta) with a very similiar setup, but works as expected:

Code: Select all • Expand view

gzdoom -iwad doom2.wad -file skytest.pk3 av2.wad -warp 21

This time MAP21 has the correct, custom sky.

Strangely, loading up map15 (which uses SKY2) provides the correct result with both example wads.

The core of WadSmoosh's Doom 2 custom sky detection is this:

Code: Select all • Expand view

TextureID skyA = TexMan.CheckForTexture("SKY3", TexMan.Type_Any, TexMan.Type_Any);
TextureID skyB = TexMan.CheckForTexture("SKY3", TexMan.Type_Override, TexMan.Type_Any);
if ( skyA == skyB )
    return; // do nothing
level.ChangeSky(skyA, skyA);


Basically it checks what it gets for two different namespaces, and if they're different, SKY# must be getting overridden, so use that instead.

For all other WADs, including the av2.wad example provided, the skyA TextureID comes back as a valid (non-zero) value. For the req2.wad example, the skyA TextureID is 0, and I can't figure out why. This is what has me wondering if it's a bug with GZDoom. Maybe req2.wad's similar SKY3 patch names (RSKY31, RSKY32, RSKY33) aren't getting handled correctly deep in some asset loading code? Maybe I'm misusing CheckForTexture? I'm not super familiar with the nuances of texture namespaces. Any ideas?

[Gez]

The core of WadSmoosh's Doom 2 custom sky detection is this:

Code: Select all • Expand view

TextureID skyA = TexMan.CheckForTexture("SKY3", TexMan.Type_Any, TexMan.Type_Any);
TextureID skyB = TexMan.CheckForTexture("SKY3", TexMan.Type_Override, TexMan.Type_Any);
if ( skyA == skyB )
    return; // do nothing
level.ChangeSky(skyA, skyA);


AFAIK, Type_Override is for TX textures, not walltextures as defined by the TEXTURE1/2 lumps. I think what would work more reliably would be to check in which archive the texture is defined -- if it's not the IWAD, it's a replacement. I'm not sure it's possible to do that check, however.

Another possibility would be to just not have any SKY1/2/3 in the WadSmoosh IWAD. Also rename the sky textures for Doom 1. Could be USKY for Ultimate Doom skies, RSKY for Doom II skies, TSKY and PSKY for Final Doom. And when you find a SKY1 texture exists anyway? It's guaranteed to be a replacement. So in your level preprocessor, if SKY1 doesn't exist, you replace it with the appropriate default sky (USKY1 for a Ultimate Doom level, RSKY1 for a Doom II level, etc.) but if it does exist, then you just don't do a texture replacement.

[Graf Zahl]

That check totally depends on undefined behavior and texture type priorization. If you need to be absolutely certain Gez's suggestion would be the way to go, i.e. not having SKY1-4 defined by default. Of course that can also cause problems with custom MAPINFOs referencing the original skies. The texture management simply wasn't made for such a use case.

[JPL]

AFAIK, Type_Override is for TX textures, not walltextures as defined by the TEXTURE1/2 lumps. I think what would work more reliably would be to check in which archive the texture is defined -- if it's not the IWAD, it's a replacement. I'm not sure it's possible to do that check, however.

This was my initial thought years ago, I asked about it and was told it was a bad way to do things.

Your suggestion about leaving SKY1/2/3 undefined in the IWAD sounds like it could work, I can investigate that. I do remember trying out all kinds of solutions ~3 years ago, and what I have now seemed to have the best compatibility, but maybe there's a better way. Requiem maps 21-30 are the first ones I've seen break with the current setup though.

[Graf Zahl]

Requiem SKY3 is special. It is texture 0, which most checking commands treat as invalid. The sky renderer makes a special exception for that.

[JPL]

Requiem SKY3 is special. It is texture 0, which most checking commands treat as invalid. The sky renderer makes a special exception for that.

Oh! This is good to know actually, if it's that much of a special case I can probably justifiably add a special case to the existing check.

[JPL]

Hmm, okay so I can detect when the "skyA" TextureID variable = 0, but Level.ChangeSky(0, 0) always sets it to the checkerboard texture in such a case. If the engine does something special in these cases, it seems like Level.ChangeSky should also take that into account so it can do the correct thing instead of what it does now.

[Graf Zahl]

That's because the texture you are looking for is not 0, but since it is marked "texture 0" it returns 0. You have to add the 'ReturnFirst' flag to the CheckForTexture call.

[JPL]

That's because the texture you are looking for is not 0, but since it is marked "texture 0" it returns 0. You have to add the 'ReturnFirst' flag to the CheckForTexture call.

This is what I was looking for, using that flag gave me the check that I wanted, and it works as desired now. Feel free to close this.