[libavg-devel] Dynamic Node Creation Enhancements

[Ulrich von Zadow]

Nick Hebner wrote:

>> 2. Store the offset of the member variable directly, like so:
>>
>>   NodeDefinition(...)
>>   .addArg("enablecrop", "true", (void*)&m_bEnableCrop-(void*)this)
>>
>> This isn't really nice either (though we could probably hide the void *
>> casting ugliness in a base class), and we need a this pointer in a
>> static function, so we'd probably need a temporary bogus object as well.
>> Still, maybe there are more people who will read and understand that
>> kind of code than there are people willing to understand template
>> trickery.
> 
> Hmm, I don't really like the idea of having to use a bogus object instance
> for some reason... We may be able to clean it up though as you say.

I don't think we can avoid the bogus object instance (and I don't like 
it either), but we can hide it in the base class.

> We would also have to store the type of the argument so that it can be set
> converted correctly.

True.

I think the interface could look like this:

   NodeDefinition::addArg(const string& sName, int* pMember,
		void* pThis, const int& Default);

We'd overload addArg() for string, int, double and bool. Default would 
be type-safe too.

 > Also, for strings, we might need multiple string types
> to handle std::string versus char* versus char[] (if there are even
> instances of this), which could get messy as well.

I don't think char* etc. are being used. If they are, this is a time to 
clean that up.

>> On the other hand, both methods would cause the member variable setting
>> in the node constructors to mostly evaporate down to a single line,
>> which, I think, would be incredibly cool. 
> 
> That would be pretty nice. On the other hand, it is also somewhat nice to
> see the variables be set explicitly. 

Well, there would still be all those addArg() calls a few lines further 
up that tell you what's going on.

 > Also, there are some instances where
> values obtained from the ArgList are not simply assigned to a variable, but
> instead used as an argument for another setter function. e.g. Words:
> "setAlignment(Args.getStringArg ("alignment"));". To get around this, we
> would have to add string parameters corresponding to these in addition to
> any other representation (in this case, the argument values are used to set
> Pango typed variables) and set the other variables after the strings have
> been set. Or, we could tell the ArgList not to set anything for certain args
> (pass -1 as the offset or something), and simply set them manually after the
> ArgList.setMembers(this) call. Both options - multiple representations of an
> argument and special casing some arguments - seem messy to me.

Yes, I saw that too, but: Why do you think setting them manually is messy?

> So in the end, I agree that this would be a pretty cool feature, but I am
> not sure that it can be implemented without sacrificing some code
> readability/design. Please prove me wrong if you see ways around these
> issues, however.

True, the actual implementation in NodeDefinition (or whereever it goes 
in the end) would contain some code that needs explanation. However, I 
think we can hide the messiness under the hood and provide a clean 
interface for all the node classes, reducing boilerplate code there.

(There is also the possibility of a largely generic Node::save() method 
that writes out an avg file :-)).

>>> The dynamic nature of the DTD is very
>> flexible,
>>> and has been designed with the idea of node plugins in mind, however, it
>> is
>>> now less visible then before. It would be nice to make the full DTD
>>> available to the designer (should we add a Player::getDTD() that returns
>> the
>>> current DTD string in python?).
>> I think the most important thing here is to have an executable that runs
>> as part of the make process and generates the dtd. (Granted, we don't
>> have that now either.)
> 
> The problem that I see with this is when we have node plugins that register
> with the player at runtime, the dtd will be different from the one generated
> at compile time. I think that we need to keep dtd generation a runtime
> process.

Ok, I see your point. Still, having a default dtd lying around would be 
great for things like xslt transforms. At runtime, we obviously need to 
use a different one.

>> Just something to keep in mind for the future: I'd like to keep the
>> amount of python-specific code in src/player/ to a minimum to allow for
>> people who want to use libavg without python.
> 
> Should we start wrapping python stuff with configuration #ifdefs?

In theory, there should be no dependency from src/player to python at 
all. The python-specific stuff should go to src/python...

Cheers,

   Uli

-- 

Ulrich von Zadow | +49-172-7872715
Jabber: cocacoder at jabber.berlin.ccc.de
Skype: uzadow