Created an EventMutator for when you want to mutate an event before reading

[BobG1983]

Objective

• Often in games you will want to create chains of systems that modify some event. For example, a chain of damage systems that handle a DamageEvent and modify the underlying value before the health system finally consumes the event. Right now this requires either:

• Using a component added to the entity
• Consuming and refiring events

Neither is ideal when really all we want to do is read the events value, modify it, and write it back.

Solution

• Create an EventMutator class similar to EventReader but with ResMut and iterators that return &mut so that events can be mutated.

Testing

• I replicated all the existing tests for EventReader to make sure behavior was the same (I believe) and added a number of tests specific to testing that 1) events can actually be mutated, and that 2) EventReader sees changes from EventMutator for events it hasn't already seen.

Migration Guide

Users currently using ManualEventReader should use EventCursor instead. ManualEventReader will be removed in Bevy 0.16. Additionally, Events::get_reader has been replaced by Events::get_cursor.

Users currently directly accessing the Events resource for mutation should move to EventMutator if possible.

[BobG1983]

Worked out the issue, par_read doesn't mark events as seen and thus lets you see events twice. My tests assumed par_read should behave the same as read.

For now I'll change the test.

I'll submit another PR with fixed par_read later.

[BobG1983]

To be clear, it looks like this has always been the behavior of par_read

[BobG1983]

Added the fix to #13821 but only to the par_read for EventMutator and NOT for EventReader figuring either the EventReader (PR #13836) or Added peeking to EventReader (PR #13809) will be taken which already contain the fix.

[nosideeffects]

I am not completely convinced that this addition is a strictly positive improvement in the Events API. The current expectation of most event systems is that each consumer or reader gets an ordered or unordered stream of each event since it started consuming.

Adding the ability for any number of systems to modify an event in the stream for all subsequent readers adds that property that the behavior of the event system is now order-dependent for all systems. I think it conflicts with developer intuition about how a typical event system behaves.

Consuming and re-emitting a separate event I would argue actually is the ideal for adhering to the semantics of the current event system. If the need for these types of event changes proves to be fairly common, I think we would be better off providing a wrapper type that captures the entire intended behavior. For example:

trait EventMutator<From: Event, To: Event> {
  fn mutate_events(&mut self, mutator: FnMut(&From) -> To);
}

where an implementation might contain both the reader for From and writer for To. It would also make the system parameters more explicit about what mutation is actually occurring.

[BobG1983]

The problem with that is the combinatorial complexity for that is awful. DamageEvent becomes DamageAfterArmorEvent becomes DamageAfterArmorWithPoisonEvent etc. so instead I just fire off damage events and chain the systems manually anyway.

[alice-i-cecile]

I think this should exist: we have an existing example for this because users kept asking how to do this. If we can meaningfully simplify and robustify a common pattern we should here.

[nosideeffects]

I would love to see an example of this making your proposed damage system "simpler" while not making it more error-prone. In my opinion, the use of events in this way is making the system more complicated than it needs to be. The combinatorial complexity doesn't go away when you make it implicit, I think it just turns into a foot gun.

[BobG1983]

I disagree.

This a very very common pattern in games, and systems themselves are already order dependent. If I have two systems that mutate the same component I must specify there run order or behavior can “footgun”. So this is already true.

Not only that but I can actually already do this in a roundabout way by directly accessing the events resource this just provides better semantics and less bugs by allowing better tracking of already seeen events while unifying the API.

[nosideeffects]

Then why insist on using events at all instead of a component?

[BobG1983]

It’s more performant, it doesn’t involve adding removing components every frame, and because philosophically it is an event and not a component.

[BobG1983]

Again, every game does something like this.

People have already asked how to do it often enough there is an example.

You can already do it if you know how EventReader and EventWriter are implemented.

Why wouldn’t make it less “arcane” and provide a unified API that matches existing APIs?

[nosideeffects]

If the firing of the first event is an event, then so too are the modifications to whatever value is contained in the event. I think this muddies what the definition of an event is.

For context, I do come from a distributed systems background and not a game development background. I would prefer to see the actual morphology of the event expressible in the API. I don't think providing this in its current state is a good long-term design. If feels like it is circumventing the strengths of Rusts type system in order to keep a status quo.

[BobG1983]

I’m sorry but I don’t understand the concern.

How is this any different from allowing component mutation?

I’m happy to listen I just don’t get it.

[nosideeffects]

Essentially, I want the API of Bevy to move in a direction where more system constraints are verifiable at compile time. Not being able to express order-or-execution constraints with component modification is not something I would say is ideal, it is just hard to solve. I don't really want to see even more of the API that Bevy provides out of the box to be only verifiable at runtime.

I think this change makes the behavior of the event system more ambiguous instead of less ambiguous.

[nosideeffects]

Let's say I update a plugin I use, and it now modifies the value of some emitted event in a shared dependency (or core event) before my system is able to read it. Before, perhaps my systems were order independent from the plugin. Now, however, the entire system has a different pattern of behavior even though the plugins API surface did not change.

[nosideeffects]

Would I need to implement my own event system in order to express immutable events?

[BobG1983]

As an aside this now also fixes the par_read bug mentioned in #13836

[atornity]

nice, LGTM

[BobG1983]

Still waiting on a few reviews.

[github-actions]

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

[alice-i-cecile]

@BobG1983 now that the release is out the door I finally have time to review this :) Very pleased with the design and engineering. Once merge conflicts are resolved I'll merge this in.

[BobG1983]

I'll get those fixed either this evening or tomorrow.

For the migration guide. Do I add it as a comment?

[alice-i-cecile]

As part of the PR description :) I've edited the description to get it started; take a look and revise it as you see fit.

[BobG1983]

Done