Am I implementing acceleration and movement correctly?

[Veritas]

Hello everyone, this is the first time I'm writing to this forum and the first time I'm trying to make a game using c++ so I apologize in case I am oblivious to some known game development concepts. Anyway the first thing I tried to address was movement handling. Instead of having to litter the main procedure with variables, I thought I should make a wrapper class possibly deriving from the sprite class in order to handle movement and general functions better.
Below is a small example-class showing what I came up. Is this a good approach for general movement? The real class will of course address other things like collision and camera view but I would like some feedback first to see if this is a good approach. Take note that I'm using the 1.6 library because of some technical reasons and since this is an educational project I thought it shouldn't matter much. In the example I'm also deriving from the Shape class since I don't really need any of the Drawable class functionalities.

Here is the main file:

#include <SFML/Graphics.hpp>
#include "Object.h"

int main(int argc, char** argv) {
   
    double frt;
    sf::RenderWindow App(sf::VideoMode(800, 600, 32), "SFML Graphics");
    sf::Event Event;
    Object Rect = sf::Shape::Rectangle(0, 0, 10, 10, sf::Color::Red);
    Rect.bottomBoundary(590);
    Rect.rightBoundary(790);
    Rect.gravity(500);
    Rect.SetPosition(0,590);
    Rect.onGround(true);
    while (App.IsOpened()) {
       
        frt = App.GetFrameTime();

        if (App.GetInput().IsKeyDown(sf::Key::Left)) {
            if (Rect.horizontalSpeed() == 0) {
                Rect.horizontalSpeed(-5);
            }
            Rect.acceleration(-500);
        }
        else if (App.GetInput().IsKeyDown(sf::Key::Right)) {
            if (Rect.horizontalSpeed() == 0) {
                Rect.horizontalSpeed(5);
            }
            Rect.acceleration(500);
        }
        else {
            if (Rect.horizontalSpeed() > 0) {
                Rect.acceleration(-500);
            }
            else if (Rect.horizontalSpeed() < 0) {
                Rect.acceleration(500);
            }
        }
           
        if (Rect.horizontalSpeed() < 0.10 && Rect.horizontalSpeed() > -0.10) {
            Rect.acceleration(0);
            Rect.horizontalSpeed(0);
        }

        if (App.GetInput().IsKeyDown(sf::Key::Up) && Rect.onGround()) {
            Rect.jump(590);
            Rect.onGround(false);
        }
     
        while (App.GetEvent(Event)) {

            if (Event.Type == sf::Event::Closed || Event.Key.Code == sf::Key::Escape) {
                App.Close();
            }
           
         }  

        Rect.updateMovement(frt);
        App.Clear();
        App.Draw(Rect);    
        App.Display();
    }

    return EXIT_SUCCESS;
}
 

And here is the wrapper class:

#include <SFML/Graphics.hpp>
#include "Object.h"
#include <math.h>

    Object::Object(const sf::Shape& other) :
        sf::Shape(other) , _gravity(0), _accel(0), _uBound(0), _bBound(0),
        _lBound(0), _rBound(0), _vSpeed(0), _hSpeed(0), _onGround(false) {}

    void Object::gravity(double gravity) {
        _gravity = gravity;
    }
    double Object::acceleration() {
        return _accel;
    }
    void Object::acceleration(double accel) {
        _accel = accel;
    }
    void Object::upperBoundary(double bound) {
        _uBound = bound;
    }
    void Object::bottomBoundary(double bound) {
        _bBound = bound;
    }
    void Object::leftBoundary(double bound) {
        _lBound = bound;
    }
    void Object::rightBoundary(double bound) {
        _rBound = bound;
    }
    void Object::verticalSpeed(double speed) {
        _vSpeed = speed;
    }
    void Object::horizontalSpeed(double speed) {
        _hSpeed = speed;
    }
    void Object::jump(double meters) {
        _vSpeed = -std::sqrt(2*_gravity*meters);
    }
    double Object::horizontalSpeed() {
        return _hSpeed;
    }
    double Object::verticalSpeed() {
        return _vSpeed;
    }
    bool Object::onGround() {
        return _onGround;
    }
    void Object::onGround(bool onGround) {
        _onGround = onGround;
    }
    void Object::updateMovement(double frt) {
        if (!_onGround) {
            _vSpeed += _gravity * frt;
        }
        if (GetPosition().y + _vSpeed * frt >= _bBound) {
            SetY(_bBound);
            _vSpeed = 0;
            _onGround = true;
        }
        if (GetPosition().y +_vSpeed * frt <= _uBound) {
            SetY(_uBound);
            _vSpeed = 0;
        }        
        if (GetPosition().x + _hSpeed * frt >= _rBound) {
            SetX(_rBound);
            _hSpeed = 0;
        }
        if (GetPosition().x + _hSpeed * frt <= _lBound) {
            SetX(_lBound);
            _hSpeed = 0;
        }
        _hSpeed += _accel * frt;
        Move(_hSpeed * frt, _vSpeed * frt);
       
    }
 

[Ixrec]

The main thing to get right at an early stage like this is to avoid coupling the inputs and actions too tightly to each other.  Your use of updateMovement() is a very good first step to this, and it might even be all you need.  So yes, you are definitely on the right track.

One way you could decouple them further is to replace lines like Rect.acceleration(-500); with something abstract like Rect.nextDirection(Rect::Left); and then let the rectangle itself decide in updateMovement() whether "move left" means -500 acceleration or some other value.  Whether this will be a beneficial abstraction or an overcomplication depends on how complicated you plan on making your game.

Also, instead of writing all those getters and setters, just make all the bound/speed/etc members public.  The only time it's worth writing that kind of boilerplate is when you're making a library for other people to use, and need the freedom to change your getters/setters to something non-trivial in the future without breaking other people's code.

[Veritas]

The main thing to get right at an early stage like this is to avoid coupling the inputs and actions too tightly to each other.  Your use of updateMovement() is a very good first step to this, and it might even be all you need.  So yes, you are definitely on the right track.

One way you could decouple them further is to replace lines like Rect.acceleration(-500); with something abstract like Rect.nextDirection(Rect::Left); and then let the rectangle itself decide in updateMovement() whether "move left" means -500 acceleration or some other value.  Whether this will be a beneficial abstraction or an overcomplication depends on how complicated you plan on making your game.

Also, instead of writing all those getters and setters, just make all the bound/speed/etc members public.  The only time it's worth writing that kind of boilerplate is when you're making a library for other people to use, and need the freedom to change your getters/setters to something non-trivial in the future without breaking other people's code.

Thanks for the reply! The reason I did not abstract away acceleration is because I thought it may be useful to be able to change the acceleration and the velocity of the object independently. So if I just want to move the object with a constant speed I just need to set the horizontal / vertical speed variables and if I ever need to change the acceleration of the object say because of environmental factors this can also be easily done. I tried to go with how real acceleration works so if I use -500 as the acceleration, the acceleration really is 500 pixels/second^2 to the left and the speed is a direct translation on pixels/second. So far I think it works pretty well, I especially like the jump example and how physics can help you calculate the exact jump height. By the way, I am actually thinking of wrapping movement to a function using states so I can easily set up some movement defaults and change between them on the go. As for encapsulation, I guess it is a habit I picked up when trying to learn programming on my own and it's kind of hard to shake :p

[Ixrec]

That's certainly a far better habit to have than not encapsulating anything =)

[Nexus]

Another crucial point of abstraction: Use vectors, not separate coordinates. Use rectangles, not 4 bounds.

Your code will become significantly simpler and more expressive. Start with sf::Vector2 where you have X and Y coordinates. For more complex operations, my implementation Thor.Vectors could help you.

[Veritas]

Another crucial point of abstraction: Use vectors, not separate coordinates. Use rectangles, not 4 bounds.

Your code will become significantly simpler and more expressive. Start with sf::Vector2 where you have X and Y coordinates. For more complex operations, my implementation Thor.Vectors could help you.

I won't be using bounds on the actual class that's for sure. I was mostly concerned about acceleration in the example, and I just needed something to limit the rect's movement. Now about vectors, I thought I should use them but I am not sure how it's going to work when I want the vertical and horizontal movements to be separate. As it is now, I can just set the upward acceleration and speed for instance to handle jumping, without changing the horizontal one. Abstracting the speeds to a vector will require me to handle both of them at once and I'm not sure if this will create more complications than it's worth. By the way, what coordinates are you referring to ?

EDIT: After some thought, using a vector should be very handy when using projectiles. I guess I could keep the separate syntax and add the vectors on top ?

[Nexus]

I don't see any problem.

sf::Vector2f gravity(0, 10);                     // constant
sf::Vector2f userInput = ...;                    // depending on user input
sf::Vector2f acceleration = gravity + userInput; // result

// Euler integration, for every time step dt
velocity += dt * acceleration;
position += dt * velocity;

With coordinates, I meant X and Y. Of course, you can still access them directly with vectors, but the whole point is to do that as rarely as possible.

[Veritas]

I don't see any problem.

sf::Vector2f gravity(0, 10);                     // constant
sf::Vector2f userInput = ...;                    // depending on user input
sf::Vector2f acceleration = gravity + userInput; // result

// Euler integration, for every time step dt
velocity += dt * acceleration;
position += dt * velocity;

With coordinates, I meant X and Y. Of course, you can still access them directly with vectors, but the whole point is to do that as rarely as possible.

Thanks for the fast reply, I quite like the approach.

EDIT: After some research it seems that deriving from the Sprite class is considered bad practice and it is advised to use a renderer class to handle graphics. What do you guys think?

[Nexus]

EDIT: After some research it seems that deriving from the Sprite class is considered bad practice and it is advised to use a renderer class to handle graphics. What do you guys think?

I agree. In general, think twice before using inheritance, it's often misused for convenience. You should read about the LSP.

There have been a lot of threads about how to render things and how to separate graphics and logics in the past, maybe you could search for them.

[Veritas]

I did read some other topics but this is a broad subject and I don't seem to end up with a concrete answer.
After reading the LSP page and rethinking things a bit, a game object is not a type of sprite so I shouldn't inherit from it. Instead the sprite is a part of the game object so I should have a sprite object as a member of the class? In this case inheriting from Drawable should be a better choice since most movable game objects are drawables in nature. Of course one could separate the drawable and the logic part of an object and treat them as different properties. I am not sure which solution is "better" or at least prevents complications as the code size increases.

[Nexus]

I did read some other topics but this is a broad subject and I don't seem to end up with a concrete answer.

There's not a single one.

Inheriting sf::Drawable can be a good place to start. For bigger projects, having game logic and graphics in separate classes may pay off; it allows things such as changing the rendering in one place (not dozens), or even switching the graphics library without affecting the whole project. It also makes entities lightweight, if they only have to store a few attributes such as position, health, etc.; and it increases overall modularity, simplifying maintenance and debugging.

[Veritas]

I did read some other topics but this is a broad subject and I don't seem to end up with a concrete answer.

There's not a single one.

Inheriting sf::Drawable can be a good place to start. For bigger projects, having game logic and graphics in separate classes may pay off; it allows things such as changing the rendering in one place (not dozens), or even switching the graphics library without affecting the whole project. It also makes entities lightweight, if they only have to store a few attributes such as position, health, etc.; and it increases overall modularity, simplifying maintenance and debugging.

Do you know any project that works with separating the graphics and the logic of which I can read the source code? Seeing an example will help me a lot and it might prove beneficial in other things relating to the development.

[Nexus]

You could have a look at the game jam code that eXpl0it3r and I developed. We had to rush quite a bit and thus didn't do things the perfect way, but it should illustrate the principle. Keep in mind that it's just one possible approach, for a game of bigger scale we could have used a component-based approach, where drawable entities might have had a graphics component.

And please don't use full quotes if the answer is just above

[Veritas]

Will do. Sorry for the quote, I am used to more active forums where posts are almost never next to each other so the habit just kind of kicked in.