Moving character in given direction C++ SFML

Question

I have written simple input manager which is used for me to detect key press, and fire assigned function. For example I can do something like that:

auto own_pointer = std::make_shared<Player>(*this);
inputControl.addKeyToCheck( sf::Keyboard::H, std::function<void( Player& )>( &Player::sayHello ),own_pointer );

and in my loop manager, I check input managers, and if H is pressed, then function sayHello will be fired.

I have written moving my player with this input manager.

auto own_pointer = std::make_shared<Player>(*this);
inputControl.addKeyToCheck( sf::Keyboard::A, std::function<void( Player& )>( &Player::moveLeft ),own_pointer );
inputControl.addKeyToCheck( sf::Keyboard::D, std::function<void( Player& )>( &Player::moveRight ), own_pointer );
inputControl.addKeyToCheck( sf::Keyboard::S, std::function<void( Player& )>( &Player::moveDown ), own_pointer );
inputControl.addKeyToCheck( sf::Keyboard::W, std::function<void( Player& )>( &Player::moveTop ), own_pointer );

where for example moveTop is just

void Player::moveTop()
{
 this->getComponent<Velocity>()->y -= speed * mv::constants::mob::DEFAULT_SPEED;
}

Generally I am so proud of my InputManager, it works everywhere else but there is really bad to use 4 different function to control moving. Have somebody any advice how to deal with that style problem?

Input manager

 /**
 * @brief Class which manage keys' input
 */
 template <class T>
 class InputManager
 {
 /* ===Objects=== */
 public:
 protected:
 private:
 //when you press KEY you execute FunctionWrapper
 std::map < sf::Keyboard::Key, FunctionPointerWrapper_t<T>> keyData;
 /* ===Methods=== */
 public:
 /**
 * @brief Checks if keys have been clicked
 */
 void update();
 /**
 * @bried adds key to database
 */
 bool addKeyToCheck( sf::Keyboard::Key key, std::function<void( T& )> function, std::shared_ptr<T> object );
 /**
 * @brief remove key from database
 */
 bool eraseKey( sf::Keyboard::Key key );
 protected:
 private:
 };
 template <class T>
 void InputManager<T>::update()
 {
 for ( auto&var : keyData )
 if ( sf::Keyboard::isKeyPressed( var.first ) )
 var.second.function( *var.second.object );
 }
 template <class T>
 bool InputManager<T>::addKeyToCheck( sf::Keyboard::Key key, std::function<void( T& )> function, std::shared_ptr<T> object )
 {
 keyData.emplace( key, FunctionPointerWrapper_t<T>( function, object ) );
 return true;
 }
 template <class T>
 bool InputManager<T>::eraseKey( sf::Keyboard::Key key )
 {
 keyData.erase( key );
 return false;
 }

• user673679: I'm not sure this question is a good fit for CodeReview. If you posted the input manager class we could suggest general improvements, some of which might cover what you're asking. However, for a specific question like this you might have better luck on gamedev.stackexchange.com .
• VirtualUser: this question is only about style, so I think that it is good. Ok, I am going to edit question
• Mast: Can you provide a usage example of how your input manager is invoked and in what manner?
• VirtualUser: @Mast it is just invoked by inputManager.update() in main game loop.

Answer 1

FunctionPointerWrapper_t seems unnecessary; we can store a std::function<void()> in the map directly. To create this function, we can either use std::bind:

auto player = std::make_shared<Player>(*this);
inputControl.addKeyToCheck( sf::Keyboard::H, std::bind(&Player::sayHello, player) );

or a lambda function:

auto player = std::make_shared<Player>(*this);
inputControl.addKeyToCheck( sf::Keyboard::H, [=] () { player->sayHello(); } );

Note that both of these examples internally create a copy of the shared_ptr.

---

For cutting down on the number of named functions in the player class, we can route the functionality to a single move function, something like:

void Player::move(Vector2f const& speed) { getComponent<Velocity>() += speed * mv::constants::mob::DEFAULT_SPEED; }

We can then use std::bind, or lambda functions as above:

inputControl.addKeyToCheck( sf::Keyboard::W, std::bind(&Player::move, player, Vector2f(0.f, -1.f)) );
inputControl.addKeyToCheck( sf::Keyboard::W, [=] () { player->move(Vector2f(0.f, -1.f)); });

Where Vector2f(0.f, -1.f) can be replaced by whatever the actual desired type / value is. (We could also do the multiplication outside of the move function, if it's not common between the functions).

We still need to define 4 functions, but there's less overhead in the Player class.

To simplify it further, we'd probably want to define some sort of axis mapping, so we could do something like:

inputControl.addAxis(Axis(sf::Keyboard::W, sf::Keyboard::S, maxSpeed, minSpeed), [=] (float axisValue) { player->moveVertically(axisValue); });

(and perhaps even allow a 2d-axis to be defined mapping 4 keys to one function call) but that might be a fair amount of extra work.

---

General comments:

• InputManager::addKeyToCheck and InputManager::eraseKey should probably not return a fixed true / false without checking if they actually succeeded.