Page MenuHome

Add c-variable binding to ID Properties (necessary for new plugin interface)
Closed, ArchivedPublicPATCH

Description

Hi Joeedh,

this patch adds c-variable binding support to ID properties.

Made the data-val accessing interface somewhat more clean on the way,
but take a look yourself!

Must confess, that I have no blend file that takes advantage of properties, so please give it a test run and tell me what find. (Bad crashes, world breaks down and so on :)

Greetings,
Peter

P.S.: There was one line in source/blender/python/api2_2x/IDProp.c:

*(float*)&self->prop->data.val

that looked suspicious. Ints are assigned with prop->data.val, but floats are assigned to self->prop? If it is always the same variable, just ignore it. I made it consistent with ints!

Event Timeline

Angemeldet: JA
user_id=4246

(in case you are interested, the current state of the plugin-system can be found here:

http://peter.schlaile.de/blender/sequencer/plugin-system-20070422.tgz
)

Logged In: YES
user_id=3002

I'm not sure binding C variables to id properties is a good
idea. The idea of ID properties is to provide a simple,
flexible data structure to store data in, not to provide a
wrapper to other data.

Could you explain you're reasoning in more detail? I could
always be wrong of course :)

Joe

Angemeldet: JA
user_id=4246

Hi Joe,

after a long discussion with pippin, I found, that for the the new plugin system, parameters should be stored within some kind of property-system.

Reasons for that are various and discussed here (http://wiki.blender.org/index.php/Talk:Requests/Plugin-System).

Using properties for plugin parameters has the drawback from a c-programmers view, that you always have to use get_property / set_property functions to access variables. Even worse, the wrapper for the old plugin architecture would have to always copy data around, to make them work. If someone decides to combine this with IPO-binding, I'm not sure right now, how this can be made to work consistently.

To make it short: the best thing, I came up with, was a method, to bind c-variables to properties.

Now considering _any_ property system, I think it is a really nice feature, that the user can have control over the way of data storage. In fact, the needed abstraction does not cost you a thing, but makes even the code more clean, if you look at my patch.

If you definitely don't like it, I will have to invent just another property system within blender (or extend bProperties), but I don't think, that's a good idea either.

To summarize:
- variable binding saves you from copying data around, if you want to keep the property variable consistent with a c-variable
- it makes life a lot easier for plugin authors
- type checking is reduced to only one place, the moment of binding the variable. This is maybe the most important point, since users of the property system have to agree on the data type used!
- I hope to have finally only one property system in blender, that works very well for all purposes. (I still don't really get, why there should be a difference in semantic between ID properties and bProperties...)
- I don't really see a drawback in this addition. I never thought of changing the notion from "arbitrary storage" to "wrapper". I thought more of: add more control to storage.

If this still sounds silly to you, just tell me.

Greetings,
Peter

Logged In: YES
user_id=3002

To be honest, I'm not sure. It sounds like a good idea, I'm
just afraid that it'll violate the idea of properties in
general.

Maybe you could bring the idea up at tomorrow's meeting? I'm
not sure I'll be there, but you could say something like,
joeedh is okay with it if no one (specifically the more
experienced devs) has any problems with it.

Joe

Angemeldet: JA
user_id=4246

Hi Joe,

had a long discussion with Ton on IRC today. Patch is okay for him. I also send a mail to the list. Maybe just wait some additional days, that people can think about it a little bit.

Greetings,
Peter

Joe, any decision on this?

regards,
Martin

I'd totally forgotton about this.
Man it's almost been a year.

Anyway, patch was approved but never committed;
I'll have to take another look at it. If anyone
else wants to commit it they can.

This looks like it could be useful for a future animation system basis. Assigning to self to get it ready for committing.

Sounds more like todo issue now?

I guess we could close this now, since RNA probably allows similar capabilities now?

Joshua Leung (aligorith) changed the task status from Unknown Status to Unknown Status.Aug 18 2010, 10:42 AM