Enumeration for events

[AzuxirenLeadGuy]

What this PR does

Adds a method EnumerateEvents to window. This returns an enumeration of "raw" events (that is, IEnumerable<Event>), which can be used in a foreach block of C#. It also includes a bool parameter to dispatch the events as well.

public IEnumerable<Event> EnumerateEvents(bool dispatchEvent=true) {...}

This allows a C# developer to handle the raw events similar to how it is done in C++ if they like to. I feel it can be a good alternative to using events, since the PollEvent() is protected.

File(s) changed

Only one file

src/SFML.Window/Window.cs

Usage example

// If the user wishes to handle events themselves
foreach(Event e in window.EnumerateEvents(false))
{
  if(e.Type==EventType.KeyPressed || e.Type==EventType.KeyReleased)
  {
    var key = e.Key;
    ... // handle event 
  }
}

// If the user wishes to dispatch using C# events, and describe/debug the events
foreach(Event e in window.EnumerateEvents(true))
{
  Console.WriteLine(e.Type);
}

[DemoXinMC]

I'm more or less okay with the idea of allowing a user to get an IEnumerable<Event> personally. Ultimate a switch can be a quicker way to implement event handling rather than requiring a bunch of lambdas/functions.

However, I don't like the idea of the optional dispatchEvents argument. In my mind, if there's going to be an optional argument, it should be to whether or not to preserve the events that were polled for and should default to false. That said, doing this would require changes to both DispatchEvents() and Window itself to add a List<Event> member.

[DemoXinMC]

Also, maybe I'm wrong here, but the proposed implementation doesn't seem to return a properly formed IEnumerable<Event>? It seems like it would just return the first Event in the queue with an IEnumerable wrapper that wouldn't be useful for an of the things IEnumerable usually does?

[AzuxirenLeadGuy]

Also, maybe I'm wrong here, but the proposed implementation doesn't seem to return a properly formed IEnumerable<Event>? It seems like it would just return the first Event in the queue with an IEnumerable wrapper that wouldn't be useful for an of the things IEnumerable usually does?

I think my implementation is correct. I tested it as such in examples/window/Program.cs, modifying the method SimpleWindow.Run() as shown

public void Run()
{
    var mode = new SFML.Window.VideoMode(800, 600);
    var window = new SFML.Graphics.RenderWindow(mode, "SFML works!");
    window.SetVerticalSyncEnabled(false);
    window.SetFramerateLimit(1);
    // Setting a low FPS so I can easily invoke multiple events in a single frame
    window.KeyPressed += this.Window_KeyPressed;

    var circle = new SFML.Graphics.CircleShape(100f)
    {
        FillColor = SFML.Graphics.Color.Blue
    };

    // Start the game loop
    while (window.IsOpen)
    {
        window.Clear();
        // Process events
        foreach(var e in window.EnumerateEvents(true))
        {
            System.Console.WriteLine($"Got event {e.Type} ");
        }
        window.Draw(circle);

        // Finally, display the rendered frame on screen
        window.Display();
        System.Console.WriteLine("Frame complete!");
    }
}

I get output like

... 
Got event MouseMoved 
Frame complete!
Got event MouseButtonPressed 
Got event MouseButtonReleased 
Got event KeyPressed 
Got event TextEntered 
Got event KeyPressed 
Got event TextEntered 
Frame complete!
Got event KeyReleased 
Got event KeyReleased 
...

I can invoke multiple events before the end of a single frame, rather than only the first event. I hope I understood you correctly.

[DemoXinMC]

I think my implementation is correct. I tested it as such in examples/window/Program.cs, modifying the method SimpleWindow.Run() as shown

Looks like you're correct. yield return is a special case used for this purpose.

I still have mixed thoughts about the dispatchEvents argument. With the current implementation, .EnumerateEvents() removes the Event from the "queue," but this is basically unavoidable without a separate container being added to Window. If someone else wants to weigh in on this and tell me I'm wrong, I'm certainly willing to be swayed.

[eXpl0it3r]

API design-wise I don't really want to introduce an additional way to do the same thing. It muddies the API usage a bit, as you now have to choose between one or the other, or even worse mix the approaches.

Any specific advantages of this approach or disadvantages or the dispatch approach?

[DemoXinMC]

Any specific advantages of this approach or disadvantages or the dispatch approach?

The ability to use a simple switch block instead of requiring event handlers does have some appeal. Particularly in cases where someone might have different event processing needs dependent on program state, connecting and disconnecting event handlers can become incredible clunky.

[AzuxirenLeadGuy]

@DemoXinMC

Any specific advantages of this approach or disadvantages or the dispatch approach?

The ability to use a simple switch block instead of requiring event handlers does have some appeal. Particularly in cases where someone might have different event processing needs dependent on program state, connecting and disconnecting event handlers can become incredible clunky.

There are other cases also, like when one wishes to use different events (like all events related with Joystick or Mouse), or if the order of events is needed to be considered by the game. Sure, it is possible to do it with the dispatch approach, but using the current one makes it more convenient.

However, I don't like the idea of the optional dispatchEvents argument. In my mind, if there's going to be an optional argument, it should be to whether or not to preserve the events that were polled for and should default to false. That said, doing this would require changes to both DispatchEvents() and Window itself to add a List<Event> member.

For keeping the optional argument as false, what I had in mind was in cases where the user is not using any of the window's event handlers at all, and thus not waste calls for the dispatch events as it would be unneeded. I do agree that it would remove the event from the window's "queue"; but if the user requires it, they can just call the ToArray() on the IEnumerable object to store it.

Event[] event_array = window.EnumerateEvents(false).ToArray();

This array would be handled directly by the user, and not by the window itself. But yes, the window can no longer dispatch those events, so the user must now handle them on their own.

@eXpl0it3r

API design-wise I don't really want to introduce an additional way to do the same thing. It muddies the API usage a bit, as you now have to choose between one or the other, or even worse mix the approaches.

That is a good point. I guess the current way to go about it would be to separate this implementation in another window type, but I am not sure if that is the best way. I will give it some more thought.

[AzuxirenLeadGuy]

Okay, there are 2 approaches that I have thought of now, other than making another Window or EventHandler type/class.

Approach 1

Overload the DispatchEvents with a parameter to return an enumerable.

So now there would be the following methods in the Window type:

void DispatchEvents();

Enumerable<Event> DispatchEvents(bool call_event_handlers);

Since it is to be ensured that both the approaches do not mix, this could be a good design since now a user would be mindful which version of DispatchEvents they are calling.

For the enumeration approach, the real purpose of the parameter here is actually to overload the function with the same name, and setting it to false is only for users who don't want to waste calls to the inner CallEventHandler() . However, this can make it non-obvious for beginners what to choose.

---

Approach 2

Change the original DispatchEvents to return an enumerable after calling the event handlers for each event.

Enumerable<Event> DispatchEvents();

This will not change the existing code that uses the SFML package, as the DispatchEvents() method can still be used as it was originally, or it can be used to iterate by users if they wish to.

// Usual Approach
DispatchEvents(); // All event handlers called

// New Approach
foreach(Event e in DispatchEvents()) // All event handlers called
{
    // Also iterating thorugh the event and using switch if the user desires
}

Of course, a user should only choose one of these ways, since calling the DispatchEvents() once would remove the Events from being accessed by the window again. I would prefer this approach as it doesn't deal with any optional parameter.

---

Both these approaches would be much easier than separating the enumeration logic outside the Window. Let me know what you guys think.