#9038 closed patch
EVENTS: EventManager and related cleanup
| Reported by: | lordhoto | Owned by: | lordhoto |
|---|---|---|---|
| Priority: | normal | Component: | --Other-- |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
Hi,
this patch fixes the fact the our DefaultEventManager is much too bloated. It includes the event recorder code, the keymapper use code etc. If a backend would want to create its own specific EventManager but still using some of those features it would either result in code duplication or skipping features, because implementing them would be too much work.
To overcome this fact I added a new "event dispatching API". This API includes the following classes:
- EventSource, a source of events (similar to the (now removed) EventProvider of DefaultEventManager) - EventObserver, a observer of new events, if an observer finds an event wants to process alone, he is allowed to "eat" it - EventMapper, a mapper, which allows mapping certain events to other events (the Keymapper implements this interface for example). - EventDispatcher, a priority based dispatcher, which is catching events from sources, passing them to an registered mapper, and then passes it to the observers. So this is basically the point there events are caught from different subsystems and supplied to different sub systems.
BaseBackend is now an EventSource (instead of EventProvider) and automatically registered with the EventDispatcher of the DefaultEventManager on initialization. Another event source is the "ArtificialEventSource", which is used to process the old "EventManager::pushEvent" API. That all is hidden from the user in DefaultEventManager initialization.
Keymapper being a EventMapper now, makes it easy to assign it to an EventDispatcher. Thus if there's another EventManager subclass other than DefaultEventManager, it only needs to call EventDispatcher::registerMapper and it works just fine.
Another (I think) advantage is that one can now change our virtual keyboard being both an EventSource and an EventObserver. Due to that fact we can even allow our DefaultEventManager and the VKBD to coexist. That means: if the VKBD notices a click inside the VKBD area it will eat that event and thus prevent the running engine from getting the click event. Now the only thing left for a simultaneous engine + vkbd run is the overlay API not being able to display game and overlay at once. That is of course now a different matter. This new code base at least allows the event part to be adapted easily for this use case.
I have another patch lying around which refactors the event recoder into its own class "EventRecoder" and registers it with the global event dispatcher (of EventManager). I'll post that too, but you first need to apply the "events.patch". (NOTE: Actually I have it in even finer grained patches in my git repo, but I think for here those two will do, if we plan to accept the patches, I'll just commit from my git repo).
The "recorder.patch", which moves the event recorder into its own class (EventRecoder), also removes the rather confusing methods "registerRandomSource" and "processMillis" from EventManager. Of course those now are placed inside the EventRecoder class, which is much cleaner IMHO. All engines and the SDL backend are adapted for this change.
Ticket imported from: #2817533. Ticket imported from: patches/1143.
Attachments (2)
Change History (6)
by , 15 years ago
| Attachment: | events.patch added |
|---|
by , 15 years ago
| Attachment: | recorder.patch added |
|---|
Patch against r42180 with applied "events.patch".
comment:1 by , 15 years ago
What you describe sounds very sensible and useful. The monolithic nature of the DefaultEventManager has been a thorn in my eye for some time, so I appreciate it. However, I haven't had time yet to look into the actual patch yet, and may not have a lot of time in the near future. I'll try to at least peek into it soon; but in the meantime, you might want to post to scummvm-devel, pointing to this patch, and ask people to comment here if they want (no point in discussing this on the list, is there?). If nobody objects for say a week, just go ahead with it.
comment:2 by , 15 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → closed |
comment:3 by , 15 years ago
I committed it now, because there was no reaction. Checkout revisions 42717 to 42729 for the event man cleanup. I couldn't commit the event recorder bits yet, because of troubles when committing. I'll do so after sleeping.
comment:4 by , 6 years ago
| Component: | → --Other-- |
|---|
Patch against r42180. (Includes disptacher API, DefaultEventManager cleanup and Keymapper adaption).