[SOLVED][2.1]Require to add custom parameters to custom pass

[al2950]

FYI I will be creating a pull request for the following, but need a direction that will suite the majority.

I have a number of custom passes in my engine, and its really useful to be able to supply custom parameters to those passes via the compositor scripts. I currently do it by using the 'identifier' property and a set bit mask, but that is, well, rubbish!

So I propose one of the following, please let me know what you think;
- Store the AbstractNode used to parse the custom pass inside Ogre::CompositorPassDef, which can be parsed further in Ogre::CompositorPassProvider
- For any value that is not recognized when initially parsing, store it in a map<string, string>. EG

Code: Select all

...
pass custom MyPass
{
     customProperty 1 0 0 1
}

Would be stored as map["customProperty"] = "1 0 0 1"

Any thoughts, any better ideas!?

[dark_sylinc]

I believe a much better idea is that the script parser calls the Pass provider for parsing additional elements.

[al2950]

I believe a much better idea is that the script parser calls the Pass provider for parsing additional elements.

Ha, just wrote a reply trying to explain why that was not an option due to the implementation, and thought I should just check and guess what... I got was wrong! (For some reason I had made the assumption that addPass was called after the AbstractNode used to parse was destroyed)

Anyway yes, your solution is they way forward and what I started at. Quick question though what would you, and others prefer,
- An extra function in Ogre::CompositorPassProvider (eg CompositorPassDef* parseCustomDef(CompositorPassDef* customPass, AbstractNodePtr node)
OR
- pass AbstractNodePtr into Ogre::CompositorPassProvider::addPass via Ogre::CompositorTargetDef::addPass.

I much prefer the second option, but it is an API breaking solution...??

[dark_sylinc]

First option.

Not even regular passes send the node on creation (i.e. you would have to modify CompositorTargetDef::addPass!)
Also not everyone would want to use the functionality, thus they can overload it if they want. Bonus: Doesn't break the API to existing users.

[al2950]

Pull request done, but not sure on implementation!

Thought about having a function like this instead

Code: Select all

virtual ScriptTranslator* getCustomPassTranslator(IDString customId);

This may fit nicer, but current method allows users to use ScriptTranslators if they want or some other custom code.

[al2950]

Sneaky bump, but I am a bit stalled while waiting for this PR to be merged or rejected..

[al2950]

@dark_sylinc
Forgot to say thanks for merging, I know you are busy!

For everyone else this is now merged.