Use last frame state of keys to extend sf::Input

[Chikubi]

What I propose, is to have the ability to get the Event::KeyPressed and Event::KeyReleased out of the sf::Input.

Because sometimes you want to get just the state, and sometimes you want to know if it was just pressed or just released. With this extension I could have all the input-related checks in the same place, and event-specific stuff in another.

This thing worked really well when I was using SDL in my own engine. I coded a little InputManager class to handle that stuff, it was really easy.

After I have switched over to SFML (I liked it a lot and it suited for my needs better than SDL too), I searched for similar feature but never found it, so, naturally, I decided to code it on my own.

I tried to do it the elegant way, by inheriting from sf::Input and sf::RenderWindow, overloading a couple of functions and such, but to my disappointment I found that all members of both of these classes were kept private.

Long story short I just messed with the source code and got it to work this way.

array of bools became array of Uint8 (same in memory afaik anyway)

here are the states I used

Code: [Select]

enum KeyStates{
NOKEY      = 1,
KEYPRESSED = 2,
//modifiers
KEYCHANGED = 4,
KEYDOWN    = KEYPRESSED | KEYCHANGED,
KEYUP      = NOKEY | KEYCHANGED
};

Don't ask me why NOKEY isn't 0 and KEYPRESSED isn't 1 so they could fit in the same bit. This code is a bit old, and I remember when I designed it there was some kind of reason for this but I don't remember it now.

Honestly I don't really care if I gain anything by saving 1 bit

The design of the enums doesn't matter anyway. What does matter is the KEYCHANGED bit; it indicates that the state of the key has changed from the last frame.

Finally I can get the info about pressing and releasing of the keys AND their state WITH the modifier keys(shift,alt,ctrl) in one line of code.

Well at the current time it does require one little call from the Window::Display() method so I can flush the KEYCHANGED states, but it works.

So, this topic has actually two feature requests:

• Extend the Input class
• Give up the private crap

I mean, what is the point of using OOP in designing a library if the user can't subclass and overload a couple of methods to suit his needs without recompiling everything?

It's either me missing some vital point behind making members private or it's just a thing that was overlooked or something.

Anyway, to keep haters off my tail I want to make one thing clear: I love SFML and I think it is awesome. However if I'm not mistaken, some parts of it could be improved a little.

What I changed to make this work:

I added methods to sf::Input:
Uint8 GetKeyState(Key::Code KeyCode) const;
void FlushKeychangedStates();

This goes in Input::OnEvent:

Code: [Select]

case Event::KeyPressed :
if(myKeys[EventReceived.Key.Code] == NOKEY) {
myKeys[EventReceived.Key.Code] = KEYDOWN;
}
else {
myKeys[EventReceived.Key.Code] = KEYPRESSED;
}
break;

case Event::KeyReleased :
if(myKeys[EventReceived.Key.Code] == KEYPRESSED) {
myKeys[EventReceived.Key.Code] = KEYUP;
}
else {
myKeys[EventReceived.Key.Code] = NOKEY;
}
break;

This in Input::ResetStates

Code: [Select]

for (int i = 0; i < Key::Count; ++i) {
        myKeys[i] = NOKEY;
//myKeysCurrent[i] = NOKEY;
//myKeysLast[i] = NOKEY;
}

and

Code: [Select]

void Input::FlushKeychangedStates()
{
for (int i = 0; i < Key::Count; ++i) {
if(myKeys[i] & KEYPRESSED) {
myKeys[i] = KEYPRESSED;
}
else if(myKeys[i] & NOKEY) {
myKeys[i] = NOKEY;
}
//myKeysCurrent[i] = NOKEY;
//myKeysLast[i] = NOKEY;
}
}

and this goes at the end of Window::Display():
myInput.FlushKeychangedStates();

Now I didn't test this extensively (yet), but it seems like it's working. The same concept however was working really good with SDL.

So, what are your thoughts about this?

BTW, the code is specific to 1.6. I looked in the latest 2.0 but it's almost the same there, private and just the key states.

Also, I implemented this only for keyboard(and mouse, later), because I don't really need that functionality for joystick currently. But it seems pretty simple to do the same for the joystick.

Edit: Didn't seem to notice this thread http://www.sfml-dev.org/forum/viewtopic.php?t=2719 *facepalm*
But... What does it change? The features are so simple to implement and are very useful IMO.

[Nexus]

I tried to do it the elegant way, by inheriting from sf::Input and sf::RenderWindow

That's not necessarily elegant. Inheritance is often abused.

I mean, what is the point of using OOP in designing a library if the user can't subclass and overload a couple of methods to suit his needs without recompiling everything?

It's either me missing some vital point behind making members private or it's just a thing that was overlooked or something.

I believe you have a wrong understanding of OOP. The way of thinking, where object-oriented programming and extensibility can only be achieved through inheritance, is typical for languages like Java, but only one possibility in multi-paradigm C++.

In my opinion, Laurent (the SFML developer) had a good reason to make those members private. Protected access is only useful, if inherited classes are really desired and need access to the base class – that is, by design. If the public interface covers the necessary interactions, there is no need to expose the internal implementation. Breaking encapsulation (whether it's by making members public or protected) is generally a bad idea, as it binds the user to a concrete implementation, making future changes difficult.

So, what are your thoughts about this?

You should try to find a less intrusive solution to achieve the desired functionality. What if you wrap sf::Input (write a class containing sf::Input as member and forward functions)?

[Chikubi]

That's not necessarily elegant. Inheritance is often abused.

That sounds like a typical "protect the fools from themselves" approach or something. We're all reasonable people here, I mean. Why make things inconvenient for us, just to protect someone from something he might or might not do? I mean in C++ is already easy to shoot your foot off. If I screw up and next update breaks my code, well, I'm the one responsible for fixing it.

I believe you have a wrong understanding of OOP. The way of thinking, where object-oriented programming and extensibility can only be achieved through inheritance, is typical for languages like Java, but only one possibility in multi-paradigm C++.

I never used Java
Well I never said extensibility could only be achieved through inheritance, but it's a nice way to do it without changing the original sources. Nothing bad in that too, but, you know, more convenient.

In my opinion, Laurent (the SFML developer) had a good reason to make those members private. Protected access is only useful, if inherited classes are really desired and need access to the base class – that is, by design. If the public interface covers the necessary interactions, there is no need to expose the internal implementation. Breaking encapsulation (whether it's by making members public or protected) is generally a bad idea, as it binds the user to a concrete implementation, making future changes difficult.

While I agree that L probably did have a good reason, I'm still convinced that it's the "protecting the fool" stuff. I might be wrong. But I mean, again, if the user binds himself to a particular implementation, it's his responsibility to deal with future changes. I accept this responsibility.

You should try to find a less intrusive solution to achieve the desired functionality. What if you wrap sf::Input (write a class containing sf::Input as member and forward functions)?

That's just great. I don't think that iterating over the IsKeyDown member every frame ~300 times is a good idea.

I'm still convinced that either having access to the data via some function like GetData(char **data) or something, or just having the members protected is a better solution than iterating over the poor IsKeyDown member ~300 times

IMO SFML is really good at being simple, but it would benefit by exposing some internal stuff to more experienced folks and/or more willing to mess with it.

[panithadrum]

If I understand, you want an input class that let's you know if a key was just pressed, just released and if it is hold?

I've done this time ago. It's pretty simple, just have two array of bool (one for key pressed and other for key released). The hold state is actually returning sf::Input::IsKeyDown.

Now just take care of reseting the states every frame, then pass the sf::Event(s) to the wrapper class to update the pressed and released states and that's all!

[Chikubi]

If I understand, you want an input class that let's you know if a key was just pressed, just released and if it is hold?

I've done this time ago. It's pretty simple, just have two array of bool (one for key pressed and other for key released). The hold state is actually returning sf::Input::IsKeyDown.

Now just take care of reseting the states every frame, then pass the sf::Event(s) to the wrapper class to update the pressed and released states and that's all!

Ok... Did you read the first post at all?