diff --git a/CHANGELOG.md b/CHANGELOG.md index b06b3d7ee0..7e962c3622 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,13 @@ Arcade [PyPi Release History](https://pypi.org/project/arcade/#history) page. - Added property setters for `center_x` and `center_y` - Added property setters for `left`, `right`, `top`, and `bottom` - Users can now set widget position and size more intuitively without needing to access the `rect` property + - Property listener can now receive: + - no args + - instance + - instance, value + - instance, value, old value + > Listener accepting `*args` receive `instance, value` like in previous versions. + - Rendering: - The `arcade.gl` package was restructured to be more modular in preparation for other backends such as WebGL and WebGPU diff --git a/arcade/gui/property.py b/arcade/gui/property.py index c9332c3ad4..2bcd5496fc 100644 --- a/arcade/gui/property.py +++ b/arcade/gui/property.py @@ -1,6 +1,8 @@ +import inspect import sys import traceback from collections.abc import Callable +from contextlib import contextmanager, suppress from typing import Any, Generic, TypeVar, cast from weakref import WeakKeyDictionary, ref @@ -9,18 +11,64 @@ P = TypeVar("P") +NoArgListener = Callable[[], None] +InstanceListener = Callable[[Any], None] +InstanceValueListener = Callable[[Any, Any], None] +InstanceNewOldListener = Callable[[Any, Any, Any], None] +AnyListener = NoArgListener | InstanceListener | InstanceValueListener | InstanceNewOldListener + + class _Obs(Generic[P]): """ Internal holder for Property value and change listeners """ - __slots__ = ("value", "listeners") + __slots__ = ("value", "_listeners") def __init__(self, value: P): self.value = value # This will keep any added listener even if it is not referenced anymore # and would be garbage collected - self.listeners: set[Callable[[Any, P], Any] | Callable[[], Any]] = set() + self._listeners: dict[AnyListener, InstanceNewOldListener] = dict() + + def add( + self, + callback: AnyListener, + ): + """Add a callback to the list of listeners""" + self._listeners[callback] = _Obs._normalize_callback(callback) + + def remove(self, callback): + """Remove a callback from the list of listeners""" + if callback in self._listeners: + del self._listeners[callback] + + @property + def listeners(self) -> list[InstanceNewOldListener]: + return list(self._listeners.values()) + + @staticmethod + def _normalize_callback(callback) -> InstanceNewOldListener: + """Normalizes the callback so every callback can be invoked with the same signature.""" + signature = inspect.signature(callback) + + with suppress(TypeError): + signature.bind(1, 1) + return lambda instance, new, old: callback(instance, new) + + with suppress(TypeError): + signature.bind(1, 1, 1) + return lambda instance, new, old: callback(instance, new, old) + + with suppress(TypeError): + signature.bind(1) + return lambda instance, new, old: callback(instance) + + with suppress(TypeError): + signature.bind() + return lambda instance, new, old: callback() + + raise TypeError("Callback is not callable") class Property(Generic[P]): @@ -85,27 +133,23 @@ def set(self, instance, value): """Set value for owner instance""" obs = self._get_obs(instance) if obs.value != value: + old = obs.value obs.value = value - self.dispatch(instance, value) + self.dispatch(instance, value, old) - def dispatch(self, instance, value): + def dispatch(self, instance, value, old_value): """Notifies every listener, which subscribed to the change event. Args: instance: Property instance - value: new value to set + value: new value set + old_value: previous value """ obs = self._get_obs(instance) for listener in obs.listeners: try: - try: - # FIXME if listener() raises an error, the invalid call will be - # also shown as an exception - listener(instance, value) # type: ignore - except TypeError: - # If the listener does not accept arguments, we call it without it - listener() # type: ignore + listener(instance, value, old_value) except Exception: print( f"Change listener for {instance}.{self.name} = {value} raised an exception!", @@ -126,7 +170,7 @@ def bind(self, instance, callback): # Instance methods are bound methods, which can not be referenced by normal `ref()` # if listeners would be a WeakSet, we would have to add listeners as WeakMethod # ourselves into `WeakSet.data`. - obs.listeners.add(callback) + obs.add(callback) def unbind(self, instance, callback): """Unbinds a function from the change event of the property. @@ -136,7 +180,7 @@ def unbind(self, instance, callback): callback: The callback to unbind. """ obs = self._get_obs(instance) - obs.listeners.remove(callback) + obs.remove(callback) def __set_name__(self, owner, name): self.name = name @@ -232,45 +276,49 @@ def __init__(self, prop: Property, instance, *args): self.obj = ref(instance) super().__init__(*args) - def dispatch(self): - self.prop.dispatch(self.obj(), self) + @contextmanager + def _dispatch(self): + """This is a context manager which will dispatch the change event + when the context is exited. + """ + old_value = self.copy() + yield + self.prop.dispatch(self.obj(), self, old_value) @override def __setitem__(self, key, value): - dict.__setitem__(self, key, value) - self.dispatch() + with self._dispatch(): + dict.__setitem__(self, key, value) @override def __delitem__(self, key): - dict.__delitem__(self, key) - self.dispatch() + with self._dispatch(): + dict.__delitem__(self, key) @override def clear(self): - dict.clear(self) - self.dispatch() + with self._dispatch(): + dict.clear(self) @override def pop(self, *args): - result = dict.pop(self, *args) - self.dispatch() - return result + with self._dispatch(): + return dict.pop(self, *args) @override def popitem(self): - result = dict.popitem(self) - self.dispatch() - return result + with self._dispatch(): + return dict.popitem(self) @override def setdefault(self, *args): - dict.setdefault(self, *args) - self.dispatch() + with self._dispatch(): + return dict.setdefault(self, *args) @override def update(self, *args): - dict.update(self, *args) - self.dispatch() + with self._dispatch(): + dict.update(self, *args) K = TypeVar("K") @@ -309,80 +357,86 @@ def __init__(self, prop: Property, instance, *args): self.obj = ref(instance) super().__init__(*args) - def dispatch(self): - """Dispatches the change event.""" - self.prop.dispatch(self.obj(), self) + @contextmanager + def _dispatch(self): + """Dispatches the change event. + This is a context manager which will dispatch the change event + when the context is exited. + """ + old_value = self.copy() + yield + self.prop.dispatch(self.obj(), self, old_value) @override def __setitem__(self, key, value): - list.__setitem__(self, key, value) - self.dispatch() + with self._dispatch(): + list.__setitem__(self, key, value) @override def __delitem__(self, key): - list.__delitem__(self, key) - self.dispatch() + with self._dispatch(): + list.__delitem__(self, key) @override def __iadd__(self, *args): - list.__iadd__(self, *args) - self.dispatch() + with self._dispatch(): + list.__iadd__(self, *args) return self @override def __imul__(self, *args): - list.__imul__(self, *args) - self.dispatch() + with self._dispatch(): + list.__imul__(self, *args) return self @override def append(self, *args): """Proxy for list.append() which dispatches the change event.""" - list.append(self, *args) - self.dispatch() + with self._dispatch(): + list.append(self, *args) @override def clear(self): """Proxy for list.clear() which dispatches the change event.""" - list.clear(self) - self.dispatch() + with self._dispatch(): + list.clear(self) @override def remove(self, *args): """Proxy for list.remove() which dispatches the change event.""" - list.remove(self, *args) - self.dispatch() + with self._dispatch(): + list.remove(self, *args) @override def insert(self, *args): """Proxy for list.insert() which dispatches the change event.""" - list.insert(self, *args) - self.dispatch() + with self._dispatch(): + list.insert(self, *args) @override def pop(self, *args): """Proxy for list.pop() which dispatches the change""" - result = list.pop(self, *args) - self.dispatch() + with self._dispatch(): + result = list.pop(self, *args) return result @override def extend(self, *args): """Proxy for list.extend() which dispatches the change event.""" - list.extend(self, *args) - self.dispatch() + with self._dispatch(): + list.extend(self, *args) @override def sort(self, **kwargs): """Proxy for list.sort() which dispatches the change event.""" - list.sort(self, **kwargs) - self.dispatch() + with self._dispatch(): + list.sort(self, **kwargs) @override def reverse(self): """Proxy for list.reverse() which dispatches the change event.""" - list.reverse(self) - self.dispatch() + with self._dispatch(): + list.reverse(self) class ListProperty(Property[list[P]], Generic[P]): diff --git a/arcade/gui/widgets/__init__.py b/arcade/gui/widgets/__init__.py index 95881b48db..dfe53fdb85 100644 --- a/arcade/gui/widgets/__init__.py +++ b/arcade/gui/widgets/__init__.py @@ -3,6 +3,7 @@ from abc import ABC from collections.abc import Iterable from enum import IntEnum +from types import EllipsisType from typing import TYPE_CHECKING, NamedTuple, TypeVar from pyglet.event import EVENT_HANDLED, EVENT_UNHANDLED, EventDispatcher @@ -509,8 +510,8 @@ def with_padding( def with_background( self, *, - color: None | Color = ..., # type: ignore - texture: None | Texture | NinePatchTexture = ..., # type: ignore + color: Color | EllipsisType | None = ..., + texture: Texture | NinePatchTexture | EllipsisType | None = ..., ) -> Self: """Set widgets background. diff --git a/arcade/gui/widgets/slider.py b/arcade/gui/widgets/slider.py index b17847acd0..12e632b648 100644 --- a/arcade/gui/widgets/slider.py +++ b/arcade/gui/widgets/slider.py @@ -115,8 +115,6 @@ def _apply_step(self, value: float): return value def _set_value(self, value: float): - # TODO changing the value itself should trigger `on_change` event - # current problem is, that the property does not pass the old value to listeners if value < self.min_value: value = self.min_value elif value > self.max_value: diff --git a/arcade/types/rect.py b/arcade/types/rect.py index cac1acab1b..3e2db6d25a 100644 --- a/arcade/types/rect.py +++ b/arcade/types/rect.py @@ -717,16 +717,16 @@ def kwargs(self) -> RectKwargs: checking. See :py:class:`typing.TypedDict` to learn more. """ - return { - "left": self.left, - "right": self.right, - "bottom": self.bottom, - "top": self.top, - "x": self.x, - "y": self.y, - "width": self.width, - "height": self.height, - } + return RectKwargs( + left=self.left, + right=self.right, + bottom=self.bottom, + top=self.top, + x=self.x, + y=self.y, + width=self.width, + height=self.height, + ) # Since __repr__ is handled automatically by NamedTuple, we focus on # human-readable spot-check values for __str__ instead. diff --git a/tests/unit/gui/test_property.py b/tests/unit/gui/test_property.py index 9ac5cefada..9c783e06a1 100644 --- a/tests/unit/gui/test_property.py +++ b/tests/unit/gui/test_property.py @@ -10,19 +10,40 @@ class MyObject: class Observer: call_args = None called = False + count = 0 def call(self): self.call_args = tuple() self.called = True + self.count += 1 - def call_with_args(self, instance, value): + def call_with_instance(self, instance): + """Match expected signature of 2 parameters""" + self.call_args = (instance,) + self.called = True + self.count += 1 + + def call_with_instance_value(self, instance, value): """Match expected signature of 2 parameters""" self.call_args = (instance, value) self.called = True + self.count += 1 + + def call_with_instance_value_old(self, instance, value, old): + """Match expected signature of 2 parameters""" + self.call_args = (instance, value, old) + self.called = True + self.count += 1 + + def call_with_args(self, *args): + self.call_args = args + self.called = True + self.count += 1 - def __call__(self, *args): + def __call__(self, *args, **kwargs): self.call_args = args self.called = True + self.count += 1 def test_bind_callback(): @@ -39,16 +60,44 @@ def test_bind_callback(): assert observer.call_args == tuple() -def test_bind_callback_with_args(): - """ - A bound callback can have 0 or 2 arguments. - 0 arguments are used for simple callbacks, like `log_change`. - 2 arguments are used for callbacks that need to know the instance and the new value. - """ +def test_bind_callback_only_once(): observer = Observer() my_obj = MyObject() - bind(my_obj, "name", observer.call_with_args) + bind(my_obj, "name", observer.call) + bind(my_obj, "name", observer.call) + + assert not observer.call_args + + # WHEN + my_obj.name = "New Name" + + assert observer.call_args == tuple() + assert observer.count == 1 + + +def test_bind_callback_accepts_instance(): + observer = Observer() + + my_obj = MyObject() + bind(my_obj, "name", observer.call_with_instance) + + assert not observer.call_args + + # WHEN + my_obj.name = "New Name" + + assert observer.call_args == (my_obj,) + + # Remove reference of call_args to my_obj, otherwise it will keep the object alive + observer.call_args = None + + +def test_bind_callback_accepts_instance_value(): + observer = Observer() + + my_obj = MyObject() + bind(my_obj, "name", observer.call_with_instance_value) assert not observer.call_args @@ -61,11 +110,28 @@ def test_bind_callback_with_args(): observer.call_args = None +def test_bind_callback_accepts_instance_value_old(): + observer = Observer() + + my_obj = MyObject() + bind(my_obj, "name", observer.call_with_instance_value_old) + + assert not observer.call_args + + # WHEN + my_obj.name = "New Name" + + assert observer.call_args == (my_obj, "New Name", None) + + # Remove reference of call_args to my_obj, otherwise it will keep the object alive + observer.call_args = None + + def test_bind_callback_with_star_args(): observer = Observer() my_obj = MyObject() - bind(my_obj, "name", observer) + bind(my_obj, "name", observer.call_with_args) # WHEN my_obj.name = "New Name" @@ -152,12 +218,12 @@ def test_gc_keeps_bound_methods(): bind(obj, "name", observer.call) - assert len(MyObject.name.obs[obj].listeners) == 1 + assert len(MyObject.name.obs[obj]._listeners) == 1 del observer gc.collect() - assert len(MyObject.name.obs[obj].listeners) == 1 + assert len(MyObject.name.obs[obj]._listeners) == 1 def test_gc_keeps_temp_methods(): @@ -170,8 +236,8 @@ def callback(*args, **kwargs): bind(obj, "name", callback) - assert len(MyObject.name.obs[obj].listeners) == 1 + assert len(MyObject.name.obs[obj]._listeners) == 1 del callback - assert len(MyObject.name.obs[obj].listeners) == 1 + assert len(MyObject.name.obs[obj]._listeners) == 1