[Bf-committers] Auto-registration in Python

[Martin Poirier]

NOTE: TL/DR at bottom

--- On Thu, 10/28/10, Nathan Vegdahl <cessen at cessen.com> wrote:

> > It should be even more trivial
> with automatic registration.
> 
> And yet in reality...

Yet in really it's exactly as I was saying. Those problems concern properties, which aren't taken care off by the autoregistration system (yet), which really sucks.

I agree.

Part of the problem is that IDPropertyGroup derived classes, as a RNAType, is autoregistered while property definitions aren't, that's why property definition needs to be done in the Register function (especially in cases where they reference subclasses of IDPropertyGroup, which I would guess is more than a few times). [Well, that and otherwise addons wouldn't be good citizen and you'd end up with properties from all installed stuff, which is a huge NO).

This isn't even a problem with autoregistration, the same issue would crop up with manual registration. You just can't define properties in the main namespace of a module if they reference IDPropertyGroup classes that will be registered later (whether automatically or manually in the Register function, as before). Also, the addons system won't work correctly in those cases, as mentioned before.

One major problem is that there is (as far as I'm aware), no real written guidelines as to how addons should be coded in that regard.

(In the future, as a solution, adding a delayed call system for properties would be the way to solve it, and that should pretty much be doable in the same way autoregistration currently works and pretty much remove the need for Register and Unregister functions, although we could still keep them around if people want to initialize custom resources).

> Here's the thing: I'm speaking as someone who was not
> involved in
> designing the system.  For someone who doesn't have
> knowledge of the
> internals of how auto-registration works, it becomes a
> magical system
> with semi-unpredictable and unclear rules.  Like, what
> gets
> registered?  What doesn't?  How do I control that
> so that I can
> structure my code how I want to?  When do things get
> registered?  Etc.

I agree, there's a serious lack of documentation.

The rules are rather simple though:

Anything that derives from an RNAType is automatically registered (and will work correctly with the addon system). Don't derive from an RNAType if your class isn't a fully formed RNAType (with all the defined methods and whatnots that this imply).

Any properties that you want to add need to be added in a Register function.

The ordering doesn't really matter. RNATypes will always be registered (in the order they are defined) before the Register function is called, so if you add properties to a IDPropertyGroup in your Register function, it will always work the first time.

Where it fucks up is when turning stuff on and off in the addons system and registering a module (or a package) multiple time in the same blender session. That's a problem with the properties system, NOT with autoregistration.

> And to make things concrete: it is not obvious, for
> example, how I
> should structure my code so that I don't get weird errors
> like this
> upon enabling an addon a second time: http://www.pasteall.org/16500

Enabling an addon a second time, if it defines custom properties, will rarely if not never work properly (at this time). It barely has anything to do with autoregistration (though, of course, I understand your frustration. I had to go through the same thing with netrender, so you can't in good faith say that I don't see where you're coming from).

> (Hint: after randomly trying things with no particular logic

That's something I don't understand. Instead (or while) trying stuff at random, did you send an error report to anyone indicating that there might be something wrong there or at the very least, that the correct way to do things should be documented correctly and in an accessible location?

[/me gets on a soap box]

(I definitely don't want to single you out in this, so take from what follows only what applies)

The API is still in stabilizing phase, if something (anything) doesn't seem right now is THE BEST time to talk about it, not three years from now.

Assuming that we know of the issue is the best way to have it falls through the cracks.

It's safer to have to repeat to people that we are aware of an issue then to have it go unnoticed, however self-evident is might seem to you.

[/me gets down from the soap box]

> I discovered that if I move my ID property class definitions
> into the
> register function, some--not all--of these errors go
> away.  But why
> that is?  No idea.  It's not intuitive at all,
> and therefore a
> crap-shoot to solve.  And it's also patently silly
> that I have to move
> those class definitions there at all.)

You shouldn't have to move the IDPropertyGroup classes in the Register function. I didn't have to do it for netrender and reloading the modules works fine (minus the properties issue).

> It removes a _trivial_ amount of boiler plate code. 
> One
> register/unregister call per class is not a lot of code.

It's not a matter of whether there's little or a lot of boiler plate code. By definition, boiler plate code is something that we should strive to remove. This isn't just a matter of good practices, it minimizes occurrences of errors in the real world.

> > as well as the possibility of easier debugging in
> those cases where
> > registration fails and gives a easy way internally to
> figure out what
> > is defined where.
> 
> It is still quite easy to find classes with registration
> problems,
> because they are referenced by name in the explicit
> registration
> calls.  If one of my panel classes fails to register,
> it is not going
> to be at all difficult to track down.
> 
> Unless I'm misunderstanding what you mean?

I'm talking about debugging RNA registration code, on the C side, something not many people have to do but it helps a lot that it's easier (at least the last time I had to debug in there).

> > it ensures that every script is a "good citizen" and
> correctly
> > registers and unregisters RNA types when the system
> needs
> > and not when bad code mistakingly does it
> 
> Sure, I can see garbage collection parallels, for
> example.  And I
> agree that this is nice.

It should also be done across the board, for properties too.

If your addon only defines operators or panels, everything is peachy.

> But what I dislike about it is that it forces the scripter
> to
> structure their code in specific ways (a subset of what
> python allows,
> see your mixin comment for example) that may not be most
> appropriate
> to the problem at hand, or to their preferences. 
> _And_ more
> importantly it introduces a new set of (non-obvious) rules
> that the
> scripter has to be aware of beyond just python itself and
> the blender
> python API.

The mixin example is not unheard off in the Python ecosystem. It's basically the recommended solution to defining shared behaviors (see how HTTPServer works and other frameworks in the standard libs).

> > The rule is that only valid well defined classes
> should derive from RNA
> > types, so if you have common behaviors to define, use
> mixin classes.
> 
> But this is a silly and needless restriction.

On the contrary. Having half-defined subclasses of RNATypes breaks the Liskov substitution principle. The verification process done in registration, in essence, is something that you should be doing anyway if you don't want your defined types to be messed up [1]

There's no rational way that you can argue against this outside of "but I don't like it".

[1] I am fully aware that Python lets you break up your type hierarchy left and right and the only thing you'll get as punishment is a runtime error. That is certainly not a very good way to argue that this is a good thing.

>  At the _very least_, if
> we stick with auto-registration we ought to do something
> like using
> decorators to specify what gets registered.  Something
> like:
> 
> @bpy.register
> class MyPanel(bpy.types.Panel):
>     blah blah blah
> 
> At least that way the scripter explicitly knows what is
> going to get
> registered, and can control that.  It would also
> remove some of the
> confusing "magical" feeling of the whole thing, and would
> allow people
> to structure their code as desired/needed.

The only difference with the current system is that you need to manually add a decorator to specify something that you already did by deriving from an RNAType (in a metaphorical sense, it's like you telling blender: "Here, this thing is a new RNAType, name Foo, methods bar, ... And btw, don't forget it's an RNAType, so register it"). One thing deriving from this is that with a decorator, you can skip the later, which means that your RNAType class hierarchy now contains something that Blender doesn't even consider (internally) a full RNAType (which, as far as we know, doesn't currently create any issue, but is not something I'd swear is totally stable).


=== TL/DR ===

Simple rules:

1: Only derive classes from RNATypes when they fulfill the RNATypes restrictions.
2. Only add RNA properties in the Register function.

Notes:
About 1, this covers adding the needed methods and class attributes after definition (whether creating the class with a call to type() or not). Don't do that, you're defining a class that doesn't fulfill its base class contract and then try to fix it after the fact. 

About 2, RNA properties definition can cause issues if registering a module multiple time (whether with the reload all functionality or by unloading and reloading an addon). This is a known issue.

That was certainly much much longer than I originally intended. Apologies for that.

Martin