Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design Proposal: Event extensions #694

Open
pathunstrom opened this issue Feb 5, 2024 · 0 comments
Open

Design Proposal: Event extensions #694

pathunstrom opened this issue Feb 5, 2024 · 0 comments

Comments

@pathunstrom
Copy link
Collaborator

Event extensions are really cool and powerful and I like teaching about them, but they've got a bunch of roughness around the edges.

Setting them up:

  1. You need to have a handle to the Engine, and we limit access to the global object intentionally.
  2. In general, it's an API designed to be used by Systems.
  3. You need to create a custom __init__ with their specifically quirky __init__ signature, then call engine.register after setting up the system proper.

Typing woes:

  1. Extensions by definition extend the type, so no immutable events.
  2. It's hard to type events extended by multiple objects.

The rest of this issue describes a potential design to address many of these issues.

Events are going to be a protocol: To be a base event, you need to have a scene attribute and a context attribute:

class Event(Protocol):
    scene: ppb.Scene | None
    context: dict | None

Any additional core event data is defined by the object that defines the event. We'll use Update as our example:

@dataclasses.dataclass(frozen=True)
class Update:
    time_delta: int
    scene: ppb.Scene | None = None
    context: dict | None = dataclasses.field(default_factory=dict)

Now, when we go to extend an event in the engine, instead of passing the original event around, we pass an empty dictionary. The dictionary should be modified in place, but we can also support returning a new dictionary as well.

If a dict is returned, we must assume that the intent is to replace the existing context dict.

We pass this dict or the replaced dict to the next extension in order.

ExtensionCallback = Callable[[dict], dict | None]

After we have hydrated our context dictionary, we now assemble a copy of the Event object in the engine. On first pass, I think this means using dataclass.replace() but open to ideas on how to do this more robustly.

dispatch_event = pre_processed_event.replace(scene=self.current_scene, context=context_dict)

Optionally, we can choose (or write) a "frozen_dict" operation to freeze the context_dict as well.

Benefits:

  1. We don't need to worry about users accidentally shooting themselves in the foot by overwriting fields.
  2. We don't need specialized type hints for a extended event type.
  3. We can teach using TypedDict for specifying the fields you care about out of a dictionary.

The second half of this proposal is fixing the Systems API for extending events. Our general design philosophy is allowing as much declarative configuration as possible, and I strongly believe that's the right path for this. The eventual goal is something akin to:

class MySystem(ppb.systemslib.System):
    extend_events = {
        ...: self.my_extension_callback
    }

While this method won't work directly (methods aren't defined at class definition time), something similar would be great.

I assume something akin to callback_or_string = Callable[[dict], dict | None] | str that will then call get_attr(SomeSystem, callback_or_string)(context_dict) when callback_or_string is a string.

The goal is that extending events is as simple as everything else, with better guard rails and typing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant