benmoran56 / esper

An ECS (Entity Component System) for Python
MIT License
537 stars 67 forks source link

event handler as function #69

Closed Felecarpp closed 2 years ago

Felecarpp commented 2 years ago

Hey ! Thanks for this greet library ! In the README.rst I can read:

An event handler can be a function or class method.

That works fine with methods but with functions I get this error:

Python 3.10.4 (main, Mar 25 2022, 19:56:11) [GCC 10.2.1 20201203] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import esper
>>> def foo():
...     pass
... 
>>> esper.set_handler("foo", foo)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "~/.local/lib/python3.10/site-packages/esper/__init__.py", line 79, in set_handl
er
    event_registry[name].add(_WeakMethod(func, _make_callback(name)))
  File "/usr/lib/python3.10/weakref.py", line 52, in __new__
    raise TypeError("argument should be a bound method, not {}"
TypeError: argument should be a bound method, not <class 'function'>
>>> 
j-fdion commented 2 years ago

Testing if the function is a method type would fix the error.

def set_handler(name: str, func) -> None:
    if name not in event_registry:
        event_registry[name] = set()

    if isinstance(func, types.MethodType):
        event_registry[name].add(_WeakMethod(func, _make_callback(name)))
    else:
        event_registry[name].add(_ref(func, _make_callback(name)))

or

def set_handler(name: str, func) -> None:
    if name not in event_registry:
        event_registry[name] = set()

    if inspect.ismethod(func):
        event_registry[name].add(_WeakMethod(func, _make_callback(name)))
    else:
        event_registry[name].add(_ref(func, _make_callback(name)))

I don't know which is better.

benmoran56 commented 2 years ago

Thanks for opening the ticket, and thanks for the suggestions j-fdion. I'll push out a fix shortly.

benmoran56 commented 2 years ago

Looks like isinstance is about 45% faster, so lets go with that.

benmoran56 commented 2 years ago

New 2.1 release was pushed out with the bugfix. Thanks!