Error when trying to use Player's metaclass with SQLAlchemy's

[Mahi]

Here's the error:

Code: Select all

[SP] Caught an Exception:
Traceback (most recent call last):
  File '..\addons\source-python\packages\source-python\events\listener.py', line 92, in fire_game_event
    callback(game_event)
  File '..\addons\source-python\plugins\testplugin\testplugin.py', line 17, in on_jump
    player = TPlayer.from_userid(e['userid'])
  File '..\addons\source-python\packages\source-python\players\entity.py', line 82, in from_userid
    return cls(index_from_userid(userid))
  File '<string>', line 2, in __init__
  File '..\addons\source-python\packages\site-packages\sqlalchemy\orm\instrumentation.py', line 335, in _new_state_if_none
    if hasattr(instance, self.STATE_ATTR):
  File '..\addons\source-python\packages\source-python\entities\entity.py', line 93, in __getattr__
    for server_class in self.server_classes:
  File '..\addons\source-python\packages\source-python\entities\entity.py', line 204, in server_classes
    yield from server_classes.get_entity_server_classes(self)
  File '..\addons\source-python\packages\source-python\entities\classes.py', line 128, in get_entity_server_classes
    if entity.classname in self._entity_server_classes:

Boost.Python.ArgumentError: Python argument types in
    None.None(TPlayer)
did not match C++ signature:
    None(class IServerUnknown *)

Here's the code:

Syntax: Select all

from sqlalchemy import Column, Integer
from sqlalchemy.ext.declarative import declarative_base
from events import Event
from players.entity import Player

Base = declarative_base()

class Meta(type(Player), type(Base)):
    pass

class TPlayer(Player, Base, metaclass=Meta):
    __tablename__ = 'player'
    id = Column(Integer, primary_key=True)

@Event('player_jump')
def on_jump(e):
    player = TPlayer.from_userid(e['userid'])

Am I doing something wrong or is this an issue with SP? How could I avoid this?

[L'In20Cible]

Adding a dummy __init__ to your metaclass seems to works for me:

Syntax: Select all

class Meta(type(Player), type(Base)):
    def __init__(*args, **kwargs):
        pass

[Mahi]

That's because it's blocking Player and Base metaclasses' .__init__'s from being called :P

Edit: To clarify, I obviously need the two base metaclasses' __init__'s to be called.

[satoon101]

I'm not sure if you have figured this out yet, but you should know that only the first found __init__ is going to get called anyway. This simple test shows what I mean:

Syntax: Select all

>>> class Test(object):
	def __init__(self):
		print('Test.__init__')

>>> class Test2(object):
	def __init__(self):
		print('Test2.__init__')

>>> class Test3(Test, Test2):
	pass

>>> Test3()
Test.__init__
<__main__.Test3 object at 0x037F3AD0>
>>> class Test3(Test2, Test):
	pass

>>> Test3()
Test2.__init__
<__main__.Test3 object at 0x031E1670>

The same is true for metaclasses.

So, you might need to overwrite __init__ in Meta to call both __init__ methods with their respective arguments. I believe the error you were encountering was due to arguments being passed to Player's metaclass that really should only be passed to Base's metaclass.

I have tested a bit this late evening (early morning), but cannot fully figure it out. I have the Player __init__ correct, I believe, but the Base __init__ I cannot quite figure out. Here is where I left off:

Syntax: Select all

from sqlalchemy import Column, Integer
from sqlalchemy.ext.declarative import declarative_base
from events import Event
from players.entity import Player

Base = declarative_base()


class Meta(type(Player), type(Base)):
    def __init__(self, name, bases, odict):
        type(Player).__init__(type(Player), name)
        type(Base).__init__(type(Base), name, (Base, ), odict)


class TPlayer(Player, Base, metaclass=Meta):
    __tablename__ = 'player'
    id = Column(Integer, primary_key=True)

@Event('player_spawn')
def _player_spawn(game_event):
    player = TPlayer.from_userid(game_event['userid'])

[Mahi]

Thanks for the answer!
I'm currently on a trip so I can't test this, but yeah I also thought it had something to do with the inits' arguments being mixed up.

you should know that only the first found __init__ is going to get called anyway.

This is why there should always be a super() call in overriddable methods! super() is not actually calling your parent class, it's calling your child's parent class:

Syntax: Select all

>>> class A:
    def __init__(self):
        super().__init__()
        print('A was called')
        
>>> class B:
    def __init__(self):
        super().__init__()
        print('B was called')
        
>>> class C(A, B):
    def __init__(self):
        super().__init__()
        print('C was called')
        
>>> c = C()
B was called
A was called
C was called

So all the __init__'s should be called if coded "properly". Here's a good video about super() from Raymond Hettinger, he explains its edge cases really well:

[Mahi]

Bump :( Still running into the same issue, no matter how I mix the inits, nothing works...

[satoon101]

Sorry, I did a little more work on this that day, but still never found a solution. I will try to work on it a bit tonight after work.

[Doldol]

100% a SourcePython Issue. And it's not because you're using metaclasses.

Syntax: Select all

from events import Event
from players.entity import Player

class My1Player(Player):
	def __init__(self, *args, **kwargs):
		super().__init__(*args, **kwargs)

class My2Player(Player):
	def __init__(self, *args, **kwargs):
		self.lol = "lolme"
		super().__init__(*args, **kwargs)

@Event('player_jump')
def on_jump(e):
	player1 = My1Player.from_userid(e['userid'])
	print(player1)
	player2 = My2Player.from_userid(e['userid'])
	print(player2)

Result:

Code: Select all

<grav.grav.My1Player object at 0xd42acca0>

[SP] Caught an Exception:
Traceback (most recent call last):
  File '../addons/source-python/packages/source-python/events/listener.py', line 92, in fire_game_event
    callback(game_event)
  File '../addons/source-python/plugins/grav/grav.py', line 17, in on_jump
    player2 = My2Player.from_userid(e['userid'])
  File '../addons/source-python/packages/source-python/players/entity.py', line 82, in from_userid
    return cls(index_from_userid(userid))
  File '../addons/source-python/plugins/grav/grav.py', line 10, in __init__
    self.lol = 'lolme'
  File '../addons/source-python/packages/source-python/entities/entity.py', line 113, in __setattr__
    for server_class in self.server_classes:
  File '../addons/source-python/packages/source-python/entities/entity.py', line 200, in server_classes
    yield from server_classes.get_entity_server_classes(self)
  File '../addons/source-python/packages/source-python/entities/classes.py', line 128, in get_entity_server_classes
    if entity.classname in self._entity_server_classes:

Boost.Python.ArgumentError: Python argument types in
    None.None(My2Player)
did not match C++ signature:
    None(IServerUnknown*)


You can't functionally subclass Player.

[satoon101]

And if you call super().__init__() prior to setting a new attribute in your 2nd class, it works fine...

Syntax: Select all

class My2Player(Player):
	def __init__(self, *args, **kwargs):
		super().__init__(*args, **kwargs)
		self.lol = "lolme"

So, that is absolutely not the issue here.

[Ayuto]

The problems are caused by the implementation of Entity.__getattr__ and Entity.__setattr__. If you use BaseEntity instead of Player, it seems to work fine.

[Mahi]

The problems are caused by the implementation of Entity.__getattr__ and Entity.__setattr__. If you use BaseEntity instead of Player, it seems to work fine.

So this is an SP problem for sure? Should I put up an issue on GitHub

[Ayuto]

Please fully test with BaseEntity at first. For me it was loading without any errors (and after commenting out __getattr__ and __setattr__ of the Entity class). But I did not test the functionality of SQL Alchemy and the Entity class. So, we should make sure that it's working at first.

[Mahi]

I've done some further testing, and this is 99% the issue. The following code works fine and creates a database of all jumping players with boolean value of 1 to the _player column:

Syntax: Select all

from sqlalchemy import create_engine, Boolean, Column, Integer
from sqlalchemy.orm import sessionmaker
from sqlalchemy.ext.declarative import declarative_base

from events import Event
from entities.entity import BaseEntity
from players.entity import Player


engine = create_engine('sqlite:///:memory:')
Base = declarative_base()
Session = sessionmaker(bind=engine)


class Meta(type(Player), type(Base)):
    pass


class SqlEnt(BaseEntity, Base, metaclass=Meta):
    __tablename__ = 'player'
    id = Column(Integer, primary_key=True)
    _player = Column(Boolean)


Base.metadata.create_all(engine)


@Event('player_jump')
def on_jump(e):
    player = Player.from_userid(e['userid'])
    ent = SqlEnt(player.index)
    ent._player = ent.is_player()
    session = Session()
    session.add(ent)
    session.commit()

However, change BaseEntity to Entity and it stops working with the __getattr__ and __setattr__ errors.

[satoon101]

We started working on a fix for this this morning, but we realized that any fix for this would mask the same error happening when someone forgets to call super().__init__ in their inheriting class. That is certainly not something we would want to do.

However, I do have a workaround for your specific use-case:

Syntax: Select all

from sqlalchemy import create_engine, Boolean, Column, Integer
from sqlalchemy.orm import sessionmaker
from sqlalchemy.ext.declarative import declarative_base

from events import Event
from entities.entity import BaseEntity
from players.entity import Player


engine = create_engine('sqlite:///:memory:')
Base = declarative_base()
Session = sessionmaker(bind=engine)


class Meta(type(Player), type(Base)):
    pass


class SqlEnt(Player, Base, metaclass=Meta):
    __tablename__ = 'player'
    id = Column(Integer, primary_key=True)
    _player = Column(Boolean)
    _is_initialized = False

    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self._is_initialized = True

    def __getattr__(self, attr):
        if self._is_initialized:
            return super().__getattr__(attr)
        else:
            return BaseEntity.__getattr__(self, attr)

    def __setattr__(self, attr, value):
        if self._is_initialized:
            super().__setattr__(attr, value)
        else:
            BaseEntity.__setattr__(self, attr, value)


Base.metadata.create_all(engine)


@Event('player_jump')
def on_jump(e):
    player = Player.from_userid(e['userid'])
    ent = SqlEnt(player.index)
    ent._player = ent.is_player()
    session = Session()
    session.add(ent)
    session.commit()

I know it's slightly dirty and probably not ideal, but it does work.

[Mahi]

Thanks, I'll take a look at the workaround tomorrow! :)

We started working on a fix for this this morning, but we realized that any fix for this would mask the same error happening when someone forgets to call super().__init__ in their inheriting class. That is certainly not something we would want to do.

If a better solution exists, surely it would be better. However, I don't see too much of a problem with this? If you're creating an entity subclass, you probably should initializate the entity. Besides, super().__init__ should almost always be called in subclasses anyways, right? Else you might end up with issues when multi-inheriting, etc.

So what's so bad with forcing a subclass to call it's base class's init, shouldn't that always be done?

[satoon101]

The problem is that it would be masked. As in, if you fail to call super().__init__, it would still 'work', but you would likely run into other issues later that would be a lot harder to debug.

Definitely, if a better solution presents itself, that will accommodate this issue and not mask the other, we will surely add it. I have yet to see or come up with a fix that would do this.

[iPlayer]

Take a look at how threading.Thread works:

Syntax: Select all

from threading import Thread

class MyThread(Thread):
    def __init__(self):
        print("We don't call super's __init__!")

    def run(self):
        print("Running the thread...")

thread = MyThread()
thread.start()

Code: Select all

We don't call super's __init__!
Traceback (most recent call last):
  File "<pyshell#10>", line 1, in <module>
    thread.start()
RuntimeError: thread.__init__() not called

They add an self._initialized field in __init__ and then assert / check for it in almost every method.

[Mahi]

Bumping the thread due to starting to work on this again :D

We started working on a fix for this this morning, but we realized that any fix for this would mask the same error happening when someone forgets to call super().__init__ in their inheriting class. That is certainly not something we would want to do.

I'm not so sure why we would want to avoid this? If you're subclassing something that uses custom metaclass functionality and has a lot of stuff going on, you can't just ignore its __init__... The heck, even if it's not doing anything fancy you shouldn't just randomly avoid the super() call!

Here's an article from Python developer Raymond Hettinger who recommends always using super(): https://rhettinger.wordpress.com/2011/0 ... red-super/

Edit: Also, already the fact that you must call super().__init__() before setting any attributes sounds like a design flaw. Not that anyone would need to set attributes before it, but I don't know of any other package ever having this issue. Emphasis on sounds like and I don't know, I could be wrong