Separation of Event Management from Window Base to a custom type

[AzuxirenLeadGuy]

Summary of PR

This PR introduces a new approach to SFML event management. Prior to this, the C# events within the WindowBase class are required to be subscribed to .NET events within the WindowBase class. This PR makes the class WindowBase to hold an object of IEventMan, and the WindowBase.DispatchEvents() now passes all events to this IEventMan object. Thus, the developer can now define their own event management logic using Dependancy Injection of their own custom type which implements this interface.

The interface IEventMan defines two methods.

public interface IEventMan
{
    void PrepareFrame();          // To be called once every frame, before events are processed
    void HandleEvent(Event eve);  // Passes the event to the custom logic
}

and the class WindowBase is modifed as

public class WindowBase : ObjectBase
{
    public readonly IEventMan SfmlEventManager;
    ...

    public void DispatchEvents()
    {
        SfmlEventManager.PrepareFrame();
        while (PollEvent(out Event e))
        {
            SfmlEventManager.HandleEvent(e);
        }
    }

Thus, a custom logic for event handling can be provided as shown

public struct SimpleEventPasser : IEventMan
{
    private readonly List<Event> _events;
    public SimpleEventPasser() => _events = new();
    public void HandleEvent(Event eve)
    {
        switch (eve.Type)
        {
            case EventType.Closed:
            case EventType.LostFocus:
            case EventType.GainedFocus:
            case EventType.KeyPressed:
            case EventType.KeyReleased:
                _events.Add(eve);
                break;
            default: // Filtering out all other events
                break;
        }
    }
    public void PrepareFrame() => _events.Clear();
    public IEnumerable<Event> ProcessEvents() => _events;
}
public static class Program
{
    public static void Main()
    {
        var event_man = new SimpleEventPasser();
        var window = new RenderWindow(
            new(640, 480),
            "Custom Event Manager example",
            Styles.Default,
            event_man
        );
        while (window.IsOpen)
        {
            window.Clear(Color.Black);
            window.DispatchEvents();
            window.Display();
            foreach (var e in event_man.ProcessEvents())
            {
                System.Console.WriteLine(e.ToString());
                if (e.Type == EventType.Closed)
                {
                    window.Close();
                }
            }
        }
    }
}

The above example is also added in a new project in the examples folder.

The original .NET event based implementation is separated to a class SubscribeManager. This is set to be the default object created for WindowBase. This does make it to be a breaking change, and the examples are also modified to support them.

Also, as a small bonus, the dummy values in the constructors are resolved. Since the constructors of base class WindowBase now also accept a type of IEventMan, all public and protected constructors can be differentiated by the order of the arguements.

Details of commits

The first commit defines IEventMan, along with modifying the WindowBase class. The original implementation of event handling is moved to the class SubscribeManager.

The second commit modifies the constructor to accept an argument of the type IEventMan, which would be used by the WindowBase. This commit also resolves the public and protected constructors so that the dummy variables are not needed.

The third commit modifies the examples to build sucessfully. Also, a new example custom_eventman is added. All examples are tested on linux, by running them in net6.0 framework.

[eXpl0it3r]

Thus, the developer can now define their own event management logic using Dependancy Injection of their own custom type which implements this interface.

And why is this preferable?

I feel like this is a very subjective change and don't really see why we have to facilitate this hook in SFML.Net.
Does this allow something, that you currently cannot achieve with the event based subscription?

[AzuxirenLeadGuy]

And why is this preferable?

Consider a game with multiple scenes, and the game gets transitioned to each scene without any fixed order. Thus for each scene, the .NET events should be registered (and unregistered) during these transitioned. Of course it is possible, but if the game makes use of multiple events (say KeyPressed, KeyReleased, MouseButtonScrolled and MouseButtonReleased), it gets cumbersome to register and unregister the events for each scene. Alternatively, If a single large method is made for all the scenes, (which provides a switch for all states), this approach mandates updating this large method for any addition or removal of scenes in the game.

Also, if I want my logic to consider multiple types of SFML events together in a single method, the NET event approach is not good for this, (unless I store the events in some buffer).

The core issue, in my opinion here is that the event management is tightly coupled to the window in this case. The PR makes use of dependency injection to resolve this.

There are additional benefits as well. I don't always want to use, say the MouseMoved SFML event and thus the .NET events hooks within the WindowBase is not carried in my type when I don't need to.

I feel like this is a very subjective change and don't really see why we have to facilitate this hook in SFML.Net.
Does this allow something, that you currently cannot achieve with the event based subscription?

Yes I agree this is a bit subjective, but I honestly feel there are more gains than losses in this approach. The original approach is still available, and can be used for simple games, whereas the developers who are looking for their own custom approach can also do so without issues.

[AzuxirenLeadGuy]

The example build is failing for the single VB.Net project examples/visualbasic. I would fix it myself, but I don't know programming in Visual Basic. The code where the event hooks are referred,

    Sub App_Closed(ByVal sender As Object, ByVal e As EventArgs) Handles window.Closed

Here, window.Closed should be replaced with the VB equivalent of the C# code (window.SfmlEventManager as SFML.Window.SubscribeManager).Closed.

[eXpl0it3r]

Consider a game with multiple scenes, and the game gets transitioned to each scene without any fixed order. Thus for each scene, the .NET events should be registered (and unregistered) during these transitioned. Of course it is possible, but if the game makes use of multiple events (say KeyPressed, KeyReleased, MouseButtonScrolled and MouseButtonReleased), it gets cumbersome to register and unregister the events for each scene. Alternatively, If a single large method is made for all the scenes, (which provides a switch for all states), this approach mandates updating this large method for any addition or removal of scenes in the game.

I feel like there are a lot better patterns to address this, than changing how SFML handles events, especially in C#. You could for example build that dispatcher you integrated into SFML on top of SFML and then distribute events as needed.
You could have a message bus that distributes the events into the locations you need it. Or you could build a inheritance hierarchy with onMouseMoved or similar functions and have the event dispatching in the base class. Or automate the registration/unregistration in some base class somehow. Or many other ways.

Additionally, your problem sounds to me like your scenes are too closely coupled to events. Why do scenes have to directly handle SFML events? Shouldn't they handle more generic events and then you have again one place to handle SFML events from where you dispatch input events?

Also, if I want my logic to consider multiple types of SFML events together in a single method, the NET event approach is not good for this, (unless I store the events in some buffer).

You can certainly register the same method on two different events, not sure that's very advisable though. When you have type specific information and you make it type unspecific again, just to branch anew to be type specific.

The core issue, in my opinion here is that the event management is tightly coupled to the window in this case. The PR makes use of dependency injection to resolve this.

But events are tightly couple to the window. Events without a window doesn't make sense.

---

I'm still not convinced of this change:

• It's "solves" a very subjective problem that has many other solutions
• We muddy the API by providing multiple ways to do the same thing
• It hooks "deep" into SFML.Net and opening up for potential errors - what if someone implements the Event Manager wrong?

[AzuxirenLeadGuy]

I feel like there are a lot better patterns to address this, than changing how SFML handles events, especially in C#. You could for example build that dispatcher you integrated into SFML on top of SFML and then distribute events as needed.
You could have a message bus that distributes the events into the locations you need it. Or you could build a inheritance hierarchy with onMouseMoved or similar functions and have the event dispatching in the base class. Or automate the registration/unregistration in some base class somehow. Or many other ways.

Yes, I agree there are many ways to solve this. I would argue that all these ways involve adding to the layers of abstraction for solving the problem (of handling the events), in contrast to my approach to removing one layer, but I understand that this is opinionated.

Additionally, your problem sounds to me like your scenes are too closely coupled to events. Why do scenes have to directly handle SFML events? Shouldn't they handle more generic events and then you have again one place to handle SFML events from where you dispatch input events?

I am not sure if I understand this part. The problem here is not on scenes being tightly coupled to the SFML events. The game window listens on the SFML events, exposes them to the scene on which scene must change state or transition to another scene. A scene can take the sequence of SFML events it wants to listen on, and sure, I could add layers of abstraction here between the SFML events and my custom game scenes, but this is forced on me because of how the window is dispatching the events. Again, I may have not understood your point.

Also, if I want my logic to consider multiple types of SFML events together in a single method, the NET event approach is not good for this, (unless I store the events in some buffer).

You can certainly register the same method on two different events, not sure that's very advisable though. When you have type specific information and you make it type unspecific again, just to branch anew to be type specific.

Oh that's a good point I hadn't considered.

But events are tightly couple to the window. Events without a window doesn't make sense.

Yes, the window must expose the events, but the issue here is that the logic of dispatching of those events is tightly bound to the window. In the C++ library, this is not a problem, because the WindowBase::pollEvent() simply returns the event, leaving the developer on their own to handle it. In SFML.Net, the developer must move around the .NET event hooks.

I'm still not convinced of this change:

• It's "solves" a very subjective problem that has many other solutions
• We muddy the API by providing multiple ways to do the same thing
• It hooks "deep" into SFML.Net and opening up for potential errors - what if someone implements the Event Manager wrong?

For the second point, I'd say that I am not providing multiple ways to do the same thing. I am providing an option to replace the sealed way of doing the thing.
No arguments against the first and last points. I understand and respect your decision if you feel this PR is unnecessary.

[eXpl0it3r]

Yes, I agree there are many ways to solve this. I would argue that all these ways involve adding to the layers of abstraction for solving the problem (of handling the events), in contrast to my approach to removing one layer, but I understand that this is opinionated.

Yes, I can absolutely see that and if it was all one code base, I could also see providing such hooks, but I don't see it justifying to modify SFML, especially for a relatively narrow use case.

sure, I could add layers of abstraction here between the SFML events and my custom game scenes, but this is forced on me because of how the window is dispatching the events. Again, I may have not understood your point.

It's a bit of a strong word go with "forced on me". Nobody forced you to uses scene objects, you could also just do it in one big loop 😉
I don't think it's productive to "blame" libraries for their provided abstraction, when they don't fit into your chosen design. Nor is it productive trying to change all the libraries to make them follow your design.

For the second point, I'd say that I am not providing multiple ways to do the same thing. I am providing an option to replace the sealed way of doing the thing.

If I can provide two answers to the question of "how can I handle events in SFML.Net", I'm providing multiple ways. Yes, one is a deep hook and the other one is using a "default implementation", but it's still multiple ways for which one needs to write/provide justifications for when to use what.

I understand and respect your decision if you feel this PR is unnecessary.

Thank you for bringing it up, having a discussion and also respecting the decision! 🙂

If you have an ideas on how to improve the design of SFML.Net, maybe you can leave some comments over on #262