Multiple calls to GetInput extension lead to new glfw input contexts

[Pyrdacor]

Summary

Multiple calls to the GetInput extension will create a new glfw input context each time. Therefore event registering on other than the first input context will not raise the events.

Steps to reproduce

• Platform: Any
• Framework Version: Any

1. Create one method for initialize keyboard events and one for initialize mouse events.
2. In each method use GetInput() to retrieve the input context and add event handlers for keyboard events (first method) and mouse events (second method).
3. Mouse event callbacks are not called as the used input context is not considered by Silk. Only the first one is.

The problem is the method GetInput below. It creates a new instance of the input context instead of reusing the existing one if present.

    public class GlfwInputPlatform : IInputPlatform
    {
        public bool IsApplicable(IWindow window)
        {
            return window is GlfwWindow;
        }

        public IInputContext GetInput(IWindow window)
        {
            // the cast isn't needed, but is used to make sure that what we've got is actually a GlfwWindow
            return new GlfwInputContext((GlfwWindow)window); // <- Problem, should reuse context when called for the second time
        }
    }

Possible solutions

• Reuse context on second and subsequent calls
• Documentation that each time it is called a new context is created (in this case rename to CreateInput)
• Make Silk register each of the created contexts and ensure that all of them are handled

I would prefer first fix.

[Perksey]

This is by design, you should really only be calling GetInput once and keeping track of the input context.

[Perksey]

Although I guess we could change it, idk @Ultz/silk-working-group what do you think?

[Pyrdacor]

If you want to keep it this way, then this has to be documented better. Moreover a method labeled "GetXYZ" should have no side-effects by design. I would not expect that a new context is created. This should be renamed to CreateInput imho. With CreateInput I would expect a new context. But even then this context should be registered and it is not, or am I missing something?

[Perksey]

Yeah I 100% agree with you. The only reason we purposely made it so that it creates a new context is we wanted to avoid people going window.GetInput().blah for every single input call as that’s a code smell.

[Perksey]

I’ll put it up to an working group vote now.

[Pyrdacor]

You can rename it to CreateInput to avoid your mentioned case.

[Perksey]

This was prematurely closed as this change was not implemented in InputWindowExtensions.