ZDoom

Rename the ZScript field to ShaderID to avoid conflicts and check TextureID for consistency.

[quote=JBenko post_id=1253201 time=1719933024 user_id=34530]What is the name of your discord server? I'd like to check out the Shader conversation.[/quote]
Late reply, this but likely they mean the [url=https://forum.zdoom.org/viewtopic.php?f=48&t=54921]official ZDoom discord.[/url] The conversation is probably long buried by now, however.

8-)

Cherno wrote:
> I posted this into the Shader channel on the Discord server for further
> discussion.
What is the name of your discord server? I'd like to check out the Shader conversation.

@Cherno: Hmm. That could work. I'm inclined to mirror TextureID as closely as possible though, since it's so similar: a number representing a kind of resource, which can by accessed by name.

"ShaderInstanceID" might be more appropriate if I understand the issue correctly.

@Cherno:
Thank you for posting this to Discord.

@phantombeta:
I'll look into serializing this and study TextureID to try and follow it more closely. I was on the fence between naming the Actor field "Shader" or "ShaderID". It seems like "ShaderID" is the way to go, especially with the increased parity with TextureID. Regarding using a setter & getter: I'd like to avoid this if possible, though I get your point about error-proofing. I'll check how TextureID handles it, as it should be pretty similar.

On the subject of following the style of TextureID: should I rename [b]TexMan.GetNamedShader(name)[/b] to [b]TexMan.CheckForShader(name)[/b] or maybe [b]TexMan.CheckForNamedShader(name)[/b]? Honestly, I never really understood why it was called [b]CheckForTexture()[/b] rather than [b]GetTextureID()[/b].

It's nice to see someone finally getting around to implementing something like this. :D

Here's some issues I see:
Currently there doesn't seem to be any code handling serialization and deserialization of the Shader variable, so it'll be undefined behaviour if the player loads a saved game in a new session, because the shaders won't necessarily have the same indices.

This should ideally be a dedicated type like TextureID instead of a bare int, otherwise the user could put in arbitrary values and trigger undefined behaviour. As is a user could also store the return value from GetNamedShader in a different int variable, thinking it's safe, and have problems later involving saved games.
As a bonus, if it were a dedicated type it could also potentially be expanded later to support uniform values more easily.

The new actor field should also be named something else in the ZScript side, as there's already a [c]Shader[/c] struct, and it'll break older mods if you add a field with the same name. (It might also be possible to version-lock the addition to a new ZScript version, but I'm not sure if that'd help here)

On the other hand, I personally think it might be a good idea to keep the variable itself hidden, and have the user set the shader using only functions instead, to make it less error-prone.

I posted this into the Shader channel on the Discord server for further discussion.

NamedHardwareShader highlight1
{
	Shader "shaders/hilight1.fp"
}

myActor.Shader = TexMan.GetNamedShader("highlight1"); // The actor will render with the "highlight" shader
myActor.Shader = -1; // The actor will render with whatever shaders are attached to its images in GLDEFS (or no shaders, if none are attached)
myActor.Shader = 0; // The actor will render with NO shaders, even if its images have shaders attached

I've implemented a new feature for GZDoom, but I'd like feedback on its interface before submitting a pull request. The feature is "Named Hardware Shaders". The basic idea is that, rather than assigning a shader [i]to an image[/i] at design time, you can, instead, setup a "named" shader, that can be assigned [i]to an actor[/i] at runtime, via ZScript. Being assigned to the actor, the shader will be used when rendering whatever sprite the actor currently displays.

The interface for this feature (explained below) is where I'd like feedback. I [i]think[/i] it's a pretty good interface, but I'm only one novice GZDoom user. Any comments or critiques are more than welcome. Also, if you like the feature as-is, posting to express this will (hopefully) help it be accepted into GZDoom more quickly.

Here's the interface:

1) In the GLDEFS file/lump, where you usually write "HardwareShader" definitions, you can write "NamedHardwareShader" definitions. The syntax is similar, except that there's no type, and the lumpname is replaced with a shader-name to be referenced in ZScript.

Example:
[code]
NamedHardwareShader highlight1
{
Shader "shaders/hilight1.fp"
}
[/code]

2) In ZScript you call `TexMan.GetNamedShader(name)` to get the numerical id of the named shader defined in GLDEFS, then you set the new actor field "Shader" to it. This makes the actor render with the shader. Two special Shader ids are -1 (to use the default shader of the rendered image) and 0 (to enforce NO shader). For example:

[code]
myActor.Shader = TexMan.GetNamedShader("highlight1"); // The actor will render with the "highlight" shader
myActor.Shader = -1; // The actor will render with whatever shaders are attached to its images in GLDEFS (or no shaders, if none are attached)
myActor.Shader = 0; // The actor will render with NO shaders, even if its images have shaders attached
[/code]

So, my implementation works fine for the basic examples I've tried it with. I am new to GZDoom source code, so I wouldn't be surprised if there's a gotcha or two the main dev's will point out. The speed parameter might need some work, for example. However, I think that it's a viable solution. If you are curious, [url=https://github.com/jon-heard/gzdoom/commit/13bb9b5d39b57a08315be43288e439b65ebd56cb]here's a link [/url] to the commit that adds this feature.

Thanks for any feedback you can provide!