Chris3606 / GoRogue

.NET Standard roguelike library in C#. Features many algorithms and data structures pertinent to roguelike/2D game developers, specifically designed to be minimally intrusive upon the developer's architecture.
MIT License
494 stars 31 forks source link

Current MessageBus Implementation Does Not Fully Support Adding Subscribers During Send #245

Closed Chris3606 closed 2 years ago

Chris3606 commented 3 years ago

The current implementation of MessageBus will crash if an ISubscriber<T> is subscribed to the bus from within an already-subscribed ISubscriber<T>, because the lists of subscribers will be modified during iteration.

This was a somewhat intentional design decision, but is not documented, and may not be viable as a general implementation regardless.

MessageBus could be implemented a number of different ways that would account for this; and each methodology introduces some different properties:

  1. Call ToArray on each subscriber list directly before it's iterated over in Send.

    • Simplest solution
    • Subscribers added during a call to Send will be called during the Send iteration in which they were added, iff the subscriber subscribed to a base class of the current message
    • Subscribers added during a call to Send will not be called during the Send iteration in which they were added if:
      • The subscriber subscribed to the same message type as the current subscriber
      • The subscriber subscribed to a base class of the current subscriber's message type, as viewed from within the actual message's type chain.
    • It's possible to call ToArray on all subscriber lists in the type chain before handling to prevent messages from queuing until after the Send call is complete; however may come with an additional performance penalty
    • Either case requires some performance testing; it may be possible to mitigate the performance cost of ToArray by simply removing the foreach loops, for example
  2. Have MessageBus add messages during a Send call to a queue instead of adding them directly, and actually add them to lists after.

    • More complex to implement; unsubscribes require queueing as well and subscribe/unsubscribe could happen entirely before the subscriber ever actually made it into the refs list
    • If all message types are queued, nested Send calls could produce unexpected behavior. Queueing only message types related to active type trees would be much more complex

AT MINIMUM, the current behavior should be documented. Eventually, if a determination can be reached on generic enough behavior suitable for a library, a nice enhancement would be adding explicit support for subscribes-during-sends.

Chris3606 commented 2 years ago

Fixed in currently released version of GoRogue 3.