Not designed to be inherited from?

I just stumbled upon this issue which looks remarkably similar to one I met not long ago, while dealing with DirectFrame objects.

The problem is that as I attach an object of a custom class to a NodePath object (via the attachNewNode() method) it looks as if the ancestor’s class instance gets attached instead of the instance of the new custom class. This leads to the situation where using NodePath.node() fetches the ancestor class instance instead of the new class’ instance. (See the example code at the end of the message).

I know that as a workaround I can set a python tag on the NodePath object to retrieve the instance of the correct class (see the two commented-out lines) but what I’d like to understand is what’s happening under the hood, as this behaviour is not quite what I’d expect. It’s almost as if these classes were not designed to be inherited from. Is that the case?

from pandac.PandaModules import TextNode, NodePath

class MyTextNode(TextNode):
    
    def __init__(self, aName):
        TextNode.__init__(self, aName)
        self.aVariable = 0.0
    
myTextNode = MyTextNode("myTextNode")
aNodePath = NodePath()
myTextNodePath = aNodePath.attachNewNode(myTextNode)
#myTextNodePath.setPythonTag("myTextNode", myTextNode)

print(type(myTextNode)) ## As expected!
print(type(myTextNodePath)) ## As expected!
print(type(myTextNodePath.node())) ## Should be MyTextNode!
#print(type(myTextNodePath.getPythonTag("myTextNode"))) ## As expected!

That’s approximately right. More accurately, inheriting from C++ classes is allowed, but you have to understand the limitations.

The C++ classes do not exactly exist in the Python namespace. They can’t; they’re C++ objects, not Python objects. Instead, we create a Python wrapper class that has the same name as the C++ class, and all of the same methods as the C++ class. When you call one of the methods on the Python wrapper, it turns around and calls the underlying C++ method of the same name. Thus, it looks like you’re actually dealing directly with the C++ object, even though you’re really dealing with a Python object.

When you inherit from a C++ class, you are actually inheriting from the Python wrapper class. You can’t actually inherit from the C++ class itself, since you’re writing a Python class, not a C++ class.

This means that whenever you create an instance of your new inherited class, you’re creating an instance of the C++ class, the Python wrapper, and your Python inherited class. But then if you pass a pointer of your instance to some C++ method, all it receives is a pointer to the C++ class. So if you take your new “node” class and store it in the scene graph, you are really only storing the underlying C++ object in the scene graph–the Python part of the object gets left behind. This makes sense, because the C++ structures can only store pointers to C++ objects, not Python objects.

So, when you pull the node out of the scene graph later, it creates a new Python wrapper around it and returns that new wrapper. Now all you have is the original C++ node–it’s not your wrapped class any more, it’s just wrapped with a generic Python wrapper. You can, of course, get back to the original wrapped class using setPythonTag(), as you observed.

David

Thank you David, that was a very clear and useful explanation. Much appreciated.

Manu

Out of your post David I’ve decided to add an item to the appendix of the manual. It uses much of what you wrote and expands upon it, with example code and some narrative to put it into context. It’d be good if you and others could proof-read it for correctness, but any feedback (or direct change!) is welcome really. You can find it here and is already in the table of contents.

Hope it helps!

Looks great! You should add a note about creating circular dependencies, though, and maybe even carefully mention unref().

Good point. I’ll get it done in the next 24-48 hours. Thank you!

Hmmm… upon further reflection I must ask for assistance!

The concern on circular dependencies is that even when two objects are tied together when one is deleted it doesn’t get garbage collected because the other one still has a reference to it, isn’t it?

Now, the normal behaviour to delete a node from the scene graph is to use NodePath.removeNode(), which in the case of a Subclass of a PandaNode will remove the PandaNode rather than its subclass. But as the subclass is stored on the PandaNode -if- there are no other references to the subclass the subclass will get garbage-collected too, python’s standard behaviour.

So, where do you see the potential for problematic circular dependencies?

I don’t think NodePath.removeNode() removes the underlying PandaNode - it just removes the reference to it.

>>> a = PandaNode("a")
>>> b = NodePath(a)
>>> b.removeNode()
>>> a
PandaNode a

See? It still exists after a removeNode().

The issue is if you have a class that inherits from PandaNode that stores itself in a tag, you get a circular dependency - the reference count never reaches zero. For example:

from pandac.PandaModules import PandaNode
import gc, sys

class A(PandaNode):
  def __init__(self):
    PandaNode.__init__(self, "A")
    self.setPythonTag("classobj", self)

Ainst = A()
print sys.getrefcount(Ainst) - 1
del Ainst

Here’s exactly what happens:
First, an instance of A is created (thus also a reference to that instance). So, the constructor adds another reference to that instance and stores it as “classobj” tag on itself.
So, by the time of the print statement, you have 2 references. One is ‘Ainst’, the other is stored as python tag on that object.
Then, I decrease the reference count by deleting the “Ainst” reference. Then the reference count becomes 1 - so it will not get destructed. There’s your memory leak right there! The PandaNode is still somewhere in memory, but we can’t access it anymore.
Python does have a cycle detector (gc.collect()) but I kind of doubt it will work with Panda’s C++ classes.
So, I see two possible solutions:

(1) Explicitly clearPythonTag(“classobj”) before you destroy the last reference. Putting this in the destructor won’t work - because the destructor will not be called until the reference count has reached zero!

(2) Manually decrease the reference count after you setPythonTag it. Dangerous - before you clear the python tag, you’ll need to increase it again. I don’t really trust this solution.

PS. I added -1 after the print statement because sys.getrefcount always prints one reference more than you’d expect - since you’re creating another reference by the time you pass Ainst as argument.

Well, this is standard Python behaviour: the garbage collector makes no guarantees of when an unreferenced object actually gets collected. It only does eventually. Otherwise, what would be the difference between detachNode() and removeNode()? The manual explicitly says that the first method only place the node in a limbo, the second actually deletes the node.

But I understand what you are saying concerning circular dependencies. I guess the Pandonic (!) way to do it is to add a destroy() method to the subclass to clear the python tag. Identically named methods are used on DirectGUI objects for seemingly the same reason, so it’s probably best to stick to that convention.

Well, try placing a gc.collect() call there. That won’t make a difference. The PandaNode still exists because there is a reference to it.
See the source code of removeNode() - it will remove the PandaNode, but only when there are no other references. So basically it decreases the reference count and removes it from the scene graph. detachNode() just doesn’t clear the reference at all.

No, because I left a reference to it. Otherwise you’d end up with a bogus pointer.

Well, then the manual needs to be corrected.

I was actually thinking about this inherit problem not long ago.

What if we made a scene graph data structure like a dict that would store all the C++ class -> python class mapping when its inserted into the scene graph. Then make the different functions that return C++ classes from the scene graph check the data structure and return you the same python class that you put in?

Basically it would require an extra wrapper around the functions that deal with adding and getting stuff from scene graph so that they can record what python classes belong to what C++ classes.

The GC part needs to be though out more though.

Ideas like this have been proposed in the past. I’m not entirely opposed, but I am a little bit against it, on the principle that it doesn’t solve the fundamental problem, and in fact only hides it a little more deeply.

The fundamental problem is that we are shoehorning two languages together, and pretending that it’s all one language. This works well up to a point. Until a new developer reaches that point, he blissfully continues working in Python, believing he is working entirely with Python classes. Eventually, though, he’s going to cross the line where it stops working, and then he will scratch his head for a while, and maybe come to the forums and write a post asking why it didn’t do what he expected when he did X.

Right now, that line is right where you subclass a C++ class (either node or NodePath) and then try to pull your subclassed object back out the scene graph.

If we implement treeform’s proposal, all it does is push the line a little further back. It means that you’ll be able to transparently pull your subclassed objects out of the scene graph, and so the new developer will be kept in the dark about the Python/C++ seam just a little while longer. Eventually, though, he’s going to try to do something else that won’t be supported (“hey, how come it doesn’t work when I overload getName()?”) and he will scratch his head for a while, and maybe come to the forums and write a post.

So, I’m not sure that this transparent remapping proposal would actually help anyone in the long run.

David

I appreciate the philosophical issue David is raising.

Personally, now that I understood the problem and I’m aware of the limitations, I’m ok with what’s available. That been said, if this is a recurring issue there might be a case to move forward along the lines Treeform has described. “Pushing the line a little further back” is not a bad thing even if it’s just a little. Progress is still progress no matter if it’s small. And then who knows, from that new point in the codescape somebody might get an idea on how to progress further.

To address David’s concerns: would it be possible to implement Treeform’s idea as a parallel option rather than the only way available? This way it’d be up to the developer’s choice if to use the currently available functionality (which by the sound of it would remain slightly more high performance) or Treeform’s “enhanced” version.

You could probably get the line up to (but not including) multiple inheritance

from Employer.MustUseOrGetFired import GameLogicNode
# something like: class GameLogicNode(object): ...
from pandac.PandaModules import PandaNode

class RuntimeObject(GameLogicNode,PandaNode):
  def __init__(self,*args,**kwargs):
    super(GameObject,self).__init__(*args,**kwargs)
    # and the pandora's box is now open

Even if the object can use tags to coerce the C++ object and self together the super() is probably not going to get all the init()s called… and if just one class fails to super() all the other classes that do will break… to super or not to super (and break those that do)… a pandora’s box indeed… (especially considering the religiously-fanatic evangelism some Pythoneers have for and against super() and multiple inheritance and MRO and mixins etc.)

I’m going to gander a gut-feeling based guess that the wrapper does not play nicely with the cooperative/MRO/C3 stuff in Python.

PS: In the example our poor developer is obviously trying to shoehorn some employer’s game engine object and make the state changes on the game engine node reflect on the superimposed PandaNode and present a unified interface that satisfies the host (ie: game logic) engine and render on screen via Panda’s scene graph.