Bugfixing - Node Tree Design Rant

Argh! Stupid Node Trees!

I've just been doing some work on a bugfix related to animating material nodes. As it turns out, I've seriously misunderstood how the node trees are stored, leading to some "interesting" consequences...


The "Wrong" Way
When I originally set up the animation system, I assumed that all "node trees" (i.e. ID-blocks containing node networks) were stored in a single collection of node-trees in the main database (main->nodetree).

Furthermore, I assumed that Materials, Textures, and Compositing nodes were simply accomplished by having the relevant Material/Texture/Scene have an ID-link to the relevant node-tree ID-block in the main collection. This would have meant that you could in theory have linked the same node setup to multiple users without having to create a group out of them first, along with simplifying the design in many other ways.

Perhaps it's just the Compositor
A few months later, I was surprised to hear a bugreport that the compositing nodes could not be animated. Investigating into this, I found that lo and behold, they weren't getting touched because nothing was aware of their existence. They just didn't live in the main collection, but rather, were attached to the scene datablock as a nameless/headless ID-block.

Although this irked me a bit at the time, I figured that since the compositor had other oddities already, perhaps this was just another one of those...

Actually, the whole world is upside down
A few weeks ago, I heard for the first time that the material and texture nodes also couldn't be animated (at least without going through the datablocks editor :)). What!?

In the ensuring discussion, I finally heard that actually, the "main->nodetree" collection was a strictly speaking only a place where the "Group" node definitions lived. Furthermore, as a result of this, material and texture nodes were also like the compositing nodes... And apparently, all this was to allow backwards compatibility.

What this all means
The consequences aren't really that severe: it just means that I'll have to add some more code in a few places... nothing harmful, though a bit less elegant than we would've had...

Having said that, I'm not convinced yet that it's such a good idea to have ID-blocks as local-only/"headless" data attached to other chunks of data, or how this would have allowed for backwards compatibility when the method proposed wouldn't have. It certainly means that some of the original benefits of having this data as ID-blocks is lost, not to mention lost opportunities for potentially simpler setups. Also, more "special exception" code needs to be added in many places to make sure these datablocks are dealt with properly.

It would've been nice if the original way I mentioned was the real way that this worked.