Closed tzachshabtay closed 6 years ago
I might be missing something here, but the practical purpose of this system is still not clear to me. As far as I understand this, the idea of claiming event is strictly connected with precise order of sending that event, because as soon as there is any uncertainty in the order, the usefulness of claiming becomes dubiuos.
If there is no actual control over subscribers ordering, what are the use cases for the claim event? I have a concern that this might be an imperfect theoretical solution for something that will be solved differently in practice.
But there is control with the CallbackPriority
parameter. So you'll have one subscriber with a low priority and one subscriber with a high priority, and you're guaranteed that the high priority subscriber will get the event first.
I doubt that somebody in practice will need more than 3 levels of sorting.
Imho the problem is not so much that the priority levels are limited, but that they are explicitly set with 3 predefined values. For example, someone writes a class library, where classes explicitly subscribe to events with highest priority, and you do not want them to have highest priority, how could you fix that without modifying the library code? Or vice-versa, I am using built-in engine's GUI classes, which all have default subscriptions, how may I increase their priority?
It's just that I am not sure how well this particular solution will solve the actual practical problem: controlling subsystems in your game, the way they receive events, when they receive them and when they don't, and so on. I've already changed my own mind several times on a solution that my game needs, and still not completely sure what's the best way to organize this. So, I have a concern that this feature may not be as useful in the end, but still add certain "overhead".
This is why I was asking what are the use cases for "claim event", maybe it is something different from what I am thinking about?
Sorry, I realize I sound bit chaotic there.
What I'd like to propose is to create a number of practical use cases ("on paper", or in the actual code) and see whether this "claim event" feature is actually working in these cases.
Well, one practical use case we know of is the one you had for your game, and this PR can solve it (right?). I don't have any more practical use cases in mind, but this PR aims to target scenarios similar to what you had, a conflict between 2 subscribers. If both subscribers are from your game, than you have full control on who goes first. If one of the subscribers is from the engine with default priority, then while you can't increase their priority, you can put your event in the low priority which guarantees the engine events come first or in high priority which guarantees the engine events come last.
Intuition tells me that this will account for most scenarios in which you need something like this.
If you have a conflict with a high priority library subscriber, then yes, this won't solve it, and you'll have to find another solution.
Well, if you think it will work by default...
I have an uneasy feeling about this, to be honest. This adds another layer of complexity, but does not work as a universal solution. Which means that if one will have a custom method to control events sending, this feature will be a "dead weight".
Unfortunately I have nothing to suggest as an alternative for engine API right now. In my game eventually I used a workaround similar to claim event, but there I had a very explicit control over order in which events are dispatched.
Yes, while I agree it's not a universal solution, I think in practice this will work fine for the vast majority of cases, and the added complexity is small.
I'm merging it in, we can always revisit it later if we come up with something better.
Resolves #150
ClaimEventToken
in the callback on which we can calltoken.Claimed = true;
which will stop the event propagating to the rest of the subscribers for that event.CallbackPriority
parameter (which can beLow
,Normal
orHigh
- default toNormal
) to all event subscriptions which can determine the order in which the events are received. This is a compromise, as you don't get individual control for rearranging subscribers along the list: that compromise was made mostly for performance reasons. As we also have uniqueness checks on the subscriptions having both uniqueness, indexing and thread-safeness would require a structure that can hurt performance badly (sorted dictionary with a lock comes to mind as one possibility here). Meanwhile, the 3 priorities solution should be good enough to most if not all cases in practice, and can be simply implemented with 3 "bucket" collections.