Terasology: Reacting to `OnAddedComponent` and `OnChangedComponent`

[jdrueckert]

The Issue

Currently, systems can (only) react to events individually. For instance, a System might react to OnChangedComponent as follows:

@ReceiveEvent(components = {AComponent.class})
public void reactTo(OnChangedComponent event, EntityRef character) {
  doSomething(character.getComponent(AComponent.class));
}

At the start of the game, AComponent is often only added, but not changed yet. With the above implementation, this will result in doSomething not being executed. While this _can_ be desired in some cases, often it is not.
For instance, the QuiverUIClientSystem in CombatSystem listens to CharacterHeldItemComponent being changed to update the hud and show an ammo slot. At the start of the game CharacterHeldItemComponent is added but not changed, which results in the ammo slot not being shown even if the selected item would need it.

Currently Feasible Solutions

Listening to changes to unrelated components

A very simple way to counter this is to simply listening to changes to an unrelated component that is bound to change at the beginning of the game:

@ReceiveEvent(components = {BComponent.class, AComponent.class})
public void reactTo(OnChangedComponent event, EntityRef character) {
  // BComponent is never used here
  doSomething(character.getComponent(AComponent.class));
}

In the QuiverUIClientSystem this can be, for instance, CharacterComponent. This component is likely to change at the start of the game and will result in doSomething being executed. However, this also means that reactTo is called _every time_ CharacterComponent changes. Furthermore, we cannot be sure that a component is indeed changed at the beginning resulting in the desired behaviour.

Additionally Reacting to OnAddedComponent

A rather nice pattern that is currently already possible to use is to react to both OnAddedComponent _and_ OnChangedComponent and let them call a shared handler function:

private void handleAddedOrChangedAComponent(character) {
  doSomething(character.getComponent(AComponent.class));
}

@ReceiveEvent(components = {AComponent.class})
public void reactTo(OnChangedComponent event, EntityRef character) {
  handleAddedOrChangedAComponent(character);
}

@ReceiveEvent(components = {AComponent.class})
public void reactTo(OnAddedComponent event, EntityRef character) {
  handleAddedOrChangedAComponent(character);
}

This pattern is a bit lenghty, though, so we might want to consider adding an alternative.

Potential Alternatives

Additional OnAddedOrChangedComponent Event

One possible alternative could be an additional event that "combines" OnAddedComponent and OnChangedComponent and can be used in exactly the cases where we want a system to react to both. In other cases the individual events can still be used. This would lead to the following:

@ReceiveEvent(components = {AComponent.class})
public void reactTo(OnAddedOrChangedComponent event, EntityRef character) {
  doSomething(character.getComponent(AComponent.class));
}

The question with this alternative is where to draw the line. Do we also need OnActivatedOrAddedComponent and OnActivatedOrAddedOrChangedComponent events and where does that stop?

Listening to Multiple Events

@ReceiveEvent can already listen to multiple components as we saw in the "unrelated component" example above:

@ReceiveEvent(components = {BComponent.class, AComponent.class})

So how about extending ReceiveEvent to also be able to listen to multiple events?

@ReceiveEvent(events = {OnChangedComponent.class, OnAddedComponent.class}, components = {AComponent.class})
public void reactTo(Event event, EntityRef character) {
  doSomething(character.getComponent(AComponent.class));
}

In the case of indeed listening to multiple events, this would probably limit the usage of event inside the function body, but often we're not interested in any information the event provides, but only in the fact that it happened.
Further questions / discussion points for this alternative:
1. The semantics of ReceiveEvent's parameters events and components would be different:
- events would encode "or" (one of the listed)
- components would encode "and" (all of the listed)

This might become a source of confusion for contributors.

1. Currently (afaik) we would need to use a common supertype of the listed events as the parameter in the function signature. Often this will probably be Event as in the example above. However, if we indeed are not interested in the event information at all, we could consider to enable dropping the event parameter like we can drop the component parameter if we list components as parameter for ReceiveEvent.

[jdrueckert]

Pinging @immortius @IsaacLic @Cervator and @skaldarnar for discussion.
Relates to https://github.com/Terasology/CombatSystem/pull/50 @stefaniamak

[immortius]

I don't recall the detail's of Terasology's current event system so much. I know in the new gestalt event system I have the concept of Event inheritence, so you could have a base ComponentEvent which is inherited by OnChangedComponent and OnAddedComponent, etc. And then if you listened to the base event you would receive all child events. It doesn't have filtering for particular child events at the moment though, but I think it broadly covers this sort of situation?

That said, the correct sort of parent event would need to exist. The new gestalt system parents these sort of events off of a Lifecycle event parent.

Looking over the alternatives being suggested, I do like having the option of an events filter on the ReceiveEvent annotation... presumable the basic contract would be the method would only be called for events that are one of the listed types or a child thereof. In the presence of an event filter the Event parameter could then be made optional. If the event parameter is included all listed events would have to be children of it, and if the event parameter is not included then there would have to be at least one filter event. Internally the event engine may produce multiple handlers for the same method for different event types.

Incidentally, a small reminder that

@ReceiveEvent(components = {AComponent.class})
public void reactTo(OnChangedComponent event, EntityRef character) {
  AComponent a = character.getComponent(AComponent.class);
}

is equivalent of

@ReceiveEvent
public void reactTo(OnChangedComponent event, EntityRef character, AComponent a) {
}

[Cervator]

So when I started reading this, remembering the chat for the actual issue that spawned it, I got the event filter thing into my head pretty quick but with a potential twist: registering an event handler for _either_ of the desired events _from a given source_ but just once. I don't know if that makes any sense, yet it has been a frequent headache when you want to react to a particular thing that could get triggered multiple times. Like "I want to know whenever a thing I care about changes, either when it is created or saved etc" which would often end up in one of those multi-handlers with shared logic - but also possibly reacting to the same cause multiple times (like a thing both being created/changed/saved - but really it was the same action)

I'm not sure if it is a good example but another one is on _Block_ changed vs on _Blocks_ changed. Sometimes you batch some logic, with a system able to handle those happenings more efficiently. But then you either end up needing to event handle both separately elsewhere, or you need to throw both events. In this example it may be you just have to go to the plural option alone (better to redesign how you're using the events?), even if you only change one block. But if both events are thrown yet you only need to react to the single underlying thing that happened you might want a handler than can trigger on both but only actually run once, skipping the second execution if it is otherwise identical (when throwing multiple events from a single action maybe you could just add the same generated id to both of them?).

That said the options above all sound good to me, and what I added may make more sense in my head than in reality. We won't get to use the very latest Gestalt in Terasology for a while, but at least we could prepare better by leaning in whichever direction it takes.

[immortius]

Mmmm, that is a trickier problem in general.

In the new gestalt's handling of lifecycle events, besides stripping them back from 5 to 3 (dropping to OnAdded, OnRemoved and OnChanged), it now has logic so that between processing events it will collate events together on the same object based on what is happening - so if a component is changed multiple times only one OnChanged event will be queued, or if a component is added, changed and removed then OnRemove is sent. If a component is removed and then readded, only an OnChanged event is sent. That sort of thing. But this really needs a custom queuing an preprocessor for those sorts of events to pull off.

Generally this is also where the event inheritance can be leveraged. If you have OnBlocksChanged and OnBlockChanged, you could have the single block change event inherit the multi-block changed event, so it can trigger both. Although really I would recommend only having the multi-block changed event, and if I was redoing things I would try vastly reducing the use of block entities and probably send block changed events against chunk entities instead.

[skaldarnar]

I'd say that for now (while Terasology is with _gestalt v5_) we can go for the variant of Additionally Reacting to OnAddedComponent which two event handlers for OnChangedComponent and OnAddedComponent with a third method holding the actual implemenation.
Not necessarily a pretty solution, but a one that is working!