ShaderData: SUPER::GetDDescription() always returns false

fwilleke80

Hi,

I just noticed, rather by concidence, that in one of my shaders, the following code always returns false:

Bool MyShader::GetDDescription(GeListNode* node, Description* description, DESCFLAGS_DESC& flags)
{
    return SUPER::GetDDescription(node, description, flags);
}

Interestingly, everything works as expected. Returning false doesn't seem to throw Cinema off.

So, what could be the reason for that? What causes NodeData::GetDDescription() to return false?

And yes, this post is a bit short on information. If you need any additional info, please tell me and I'll provide you with it.

Cheers,
Frank

ferdinand

Hey @fwilleke80,

thank you for reaching out to us. Well, that depends. First, I assume that you actually defined INSTANCEOF(MyShaderData, ShaderData) in your MyShader for the SUPER attribute to actually compile/work. Secondly, that call should call GetDDescription() of ShaderData and not NodeData, unless you did declare your MyShader to be an instance of NodeData in the macro (which IMHO should not compile in the first place). Lastly, to why this actually happens. I would have to try myself (once you confirmed that the other cases all do not apply), but there are basically three options:

(1) Your qualifier 'always' in the statement 'the following code always returns false' is imprecise. It is quite normal that scene elements do not have always access to their description, which is why one should implement GetDDescription always like this (see SDK for examples):

Bool MyNode::GetDDescription(GeListNode* node, Description* description, DESCFLAGS_DESC& flags)
{
  if (!description->LoadDescription(node->GetType()))
    return false;
  ...

Later on in the life cycle of the node, one will be then able to load that description. Or in other words, it is quite normal that some GetDDescription calls do fail. Can you confirm that this does indeed always happen in your case?

(2) ShaderData has itself no base description. I.e., there is nothing to load and modify. Seems unlikely to me since there is Xbase which should kick in when an implementing entity somehow manages to screw up its description.

(3) There is a bug in ShaderData

Cheers,
Ferdinand

edit: Okay, I had a super-brief look, and from what I can see, ShaderData does indeed not seem to implement GetDDescription in any form. Which would then mean that your calls do always end up here:

~~So, calling the base implementation of a shader for GetDDescription seems kind of pointless.~~ (no, bad Ferdinand, bad, very bad )

fwilleke80

Hi Ferdinand,

@ferdinand said in ShaderData: SUPER::GetDDescription() always returns false:

edit: Okay, I had a super-brief look, and from what I can see, ShaderData does indeed not seem to implement GetDDescription in any form. Which would then mean that your calls do always end up here

Ah, okay, so it's actually normal for the SUPER call to return false! I just never noticed.

Telling from the documentation, which says about the NodeData::GetDDescription() return value:

true if successful, otherwise false. It is recommended to include a call to the parent function as last return.

... I assumed that false would indicate an error. Apparently, it doesn't. So I'll just leave it like this

Thanks!

Cheers,
Frank

ferdinand

Hey @fwilleke80,

you are right, it does indicate an error, hence my example from above. When I searched in our internal code base for similar code, I found a lot of

return NodeData::GetDDescription(node, description, flags);

calls. While there are certainly node types which customize their description handling, this seems to be more a precaution for the case when we decide that we want to implement something in NodeData::GetDDescription in the future. Or something has been there in the past and we optimized it away. In any case, one should not take my comment about calling the base implementation being pointless too seriously. It is best to leave the call in, although it currently has no practical effect and probably will never have.

Cheers,
Ferdinand