3
\$\begingroup\$

I have created a flocking simulation based on Craig Reynolds Boids. Could you give me some feedback on my code? I am a newbie and would really appreciate some criticism.

Files:

  • Boid - this is the representation of a bird like object.

  • Flock - this controls and manages the flock of boids.

Flock.h:

#include "Boid.h"
#include <vector>
#include <random>
#include <chrono>
#ifndef FLOCK_H
#define FLOCK_H

class Flock
{
private:
    std::vector<Boid> m_boidStorage;
    sf::RenderWindow* m_winPtr = nullptr;
    sf::Texture* m_t;

    std::uniform_real_distribution<float> m_uniform;
    int m_seed;
    std::mt19937 m_randomEngine;
    std::normal_distribution<float> m_gaussian;

    float maxForce;
    float maxVelocity;
    float minimumSeperation;


    void Alignment(float multiplier);
    void Cohesion(float multiplier);
    void Seperation(float multiplier);
    void Move();
    void MassCalculate();

    float AbsBoidDistance(const Boid& boid1, const Boid& boid2);
    sf::Vector2f VecBoidDistance(const Boid& boid1, const Boid& boid2);
    float VectorMagnitude(const sf::Vector2f& vec);
    void LimitMagnitude(sf::Vector2f& vec, const float& magnitude);
    void SetMagnitude(sf::Vector2f& vec, const float& magnitude);
    bool CompareMemLocs(const Boid& boid1, const Boid& boid2);

    void Borders(Boid& boid);


public:
    enum FORCECONSTANTS
    {
        KA = 0, KC, KS
    };
    float kA;
    float kS;
    float kC;

    Flock(sf::RenderWindow* winPtr);
    void Display();
    void Update();
    void ChangeConstants(FORCECONSTANTS x, bool direction);
    void AddBoid(bool external);

};

#endif

Flock.cpp: Originally, I had calculated the Cohesion, Seperation and Alignment Forces by individual functions, however merging the algorithms together in to a single function, called void Flock::MassCalculate() has allowed me to increase performance by reducing the number of checks.

#include "Flock.h"
#include "Constants.h"
#include <iostream>

Flock::Flock(sf::RenderWindow* winPtr)
    :m_winPtr(winPtr)
{
    minimumSeperation = 40;
    maxForce = 0.01;
    maxVelocity = 0.75;
    kA = 1.4;
    kC = 1.5;
    kS = 1.7;

    m_seed = std::chrono::steady_clock::now().time_since_epoch().count();
    m_randomEngine.seed(m_seed);
    std::normal_distribution<float> igaussian(WIDTH / 2, 0.2);
    std::uniform_real_distribution<float> uniformDist(-1, 1);


    m_uniform = uniformDist;
    m_gaussian = igaussian;


    for (int i = 0; i < TOTAL_BOIDS; i++)
    {
        AddBoid(false);
    }
}
void Flock::AddBoid(bool external)
{
    if (external)
    {
        Boid b1{ sf::Vector2f{float(WIDTH / 2),float(HEIGHT / 2)}, sf::Vector2f{0,0} };
        m_boidStorage.push_back(b1);
        maxVelocity += 0.05;
    }
    else
    {
        Boid b2(sf::Vector2f{ m_gaussian(m_randomEngine), m_gaussian(m_randomEngine) }, sf::Vector2f{ float(m_uniform(m_randomEngine)), float(m_uniform(m_randomEngine)) });
        m_boidStorage.push_back(b2);
    }

}
sf::Vector2f Flock::VecBoidDistance(const Boid& boid1, const Boid& boid2)
{
    sf::Vector2f b1 = boid1.boid.getPosition();
    sf::Vector2f b2 = boid2.boid.getPosition();
    return b2 - b1;
}
float Flock::AbsBoidDistance(const Boid& boid1, const Boid& boid2)
{
    sf::Vector2f b1 = boid1.boid.getPosition();
    sf::Vector2f b2 = boid2.boid.getPosition();
    return sqrt(pow((b2.x - b1.x), 2) + pow((b2.y - b1.y), 2));
}
float Flock::VectorMagnitude(const sf::Vector2f& vec)
{
    return sqrt(pow(vec.x, 2) + pow(vec.y, 2));
}
void Flock::SetMagnitude(sf::Vector2f& vec, const float& newmagnitude)
{
    float currentMag = VectorMagnitude(vec);
    vec /= currentMag;
    vec *= newmagnitude;
}
void Flock::LimitMagnitude(sf::Vector2f& vec,const float& limit)
{
    float magnitude = VectorMagnitude(vec);
    if (magnitude > limit)
    {
        SetMagnitude(vec, limit);
    }
}
bool Flock::CompareMemLocs(const Boid& boid1, const Boid& boid2)
{
    return &boid1 != &boid2;
}

void Flock::Borders(Boid& boid)
{
    if (boid.boid.getPosition().x < -boid.m_radius)
    {
        boid.boid.setPosition(sf::Vector2f{ WIDTH + boid.m_radius, boid.boid.getPosition().y });
    }
    if (boid.boid.getPosition().x > WIDTH + boid.m_radius)
    {
        boid.boid.setPosition(sf::Vector2f{ 0 - boid.m_radius, boid.boid.getPosition().y });
    }
    if (boid.boid.getPosition().y < -boid.m_radius)
    {
        boid.boid.setPosition(sf::Vector2f{ boid.boid.getPosition().x, HEIGHT + boid.m_radius });
    }
    if (boid.boid.getPosition().y > HEIGHT + boid.m_radius)
    {
        boid.boid.setPosition(sf::Vector2f{ boid.boid.getPosition().x, 0 - boid.m_radius });
    }
}
void Flock::Display()
{
    Boid B(sf::Vector2f{ WIDTH / 2,HEIGHT / 2 }, sf::Vector2f{ 0,0 });
    m_winPtr->clear();
    for (const Boid& boid : m_boidStorage)
    {

        m_winPtr->draw(boid.boid);
    }
    m_winPtr->display();
}
void Flock::Move()
{
    for (Boid& boid : m_boidStorage)
    {
        boid.Move();
        Borders(boid);
    }
}

void Flock::Update()
{
    MassCalculate();

    Move();
    Display();

}


void Flock::MassCalculate()
{
    float alignmentperceptionradius = 50;
    float cohesionperceptionradius = 50;
    float seperationperceptionradius = 50;
    for (Boid& thisBoid : m_boidStorage)
    {
        unsigned int alignmentCount = 0;
        unsigned int cohesionCount = 0;
        unsigned int seperationCount = 0;

        sf::Vector2f AlignmentForce = {};
        sf::Vector2f CohesionForce = {};
        sf::Vector2f SeperationForce = {};
        sf::Vector2f Distance = {};

        for (const Boid& otherBoid : m_boidStorage)
        {
            float d = AbsBoidDistance(thisBoid, otherBoid);
            if (CompareMemLocs(thisBoid, otherBoid) && d < alignmentperceptionradius)
            {
                AlignmentForce += otherBoid.m_Velocity;
                alignmentCount++;
            }
            if (CompareMemLocs(thisBoid, otherBoid) && d < cohesionperceptionradius)
            {
                CohesionForce += otherBoid.boid.getPosition();
                cohesionCount++;
            }
            if (CompareMemLocs(thisBoid, otherBoid) && d < seperationperceptionradius)
            {
                Distance = thisBoid.boid.getPosition() - otherBoid.boid.getPosition();
                Distance /= (d*d);
                SeperationForce += Distance;
                seperationCount++;
            }


        }
        if (alignmentCount > 0)
        {
            AlignmentForce /= float(alignmentCount);
            SetMagnitude(AlignmentForce, maxVelocity);
            AlignmentForce = AlignmentForce - thisBoid.m_Velocity;
            LimitMagnitude(AlignmentForce, maxForce);
        }
        if (cohesionCount > 0)
        {
            CohesionForce /= float(cohesionCount);
            CohesionForce = CohesionForce - thisBoid.boid.getPosition();

            SetMagnitude(CohesionForce, maxVelocity);
            CohesionForce = CohesionForce - thisBoid.m_Velocity;
            LimitMagnitude(CohesionForce, maxForce);
        }
        if (seperationCount > 0)
        {
            SeperationForce /= float(seperationCount);
            SetMagnitude(SeperationForce, maxVelocity);
            SeperationForce = SeperationForce - thisBoid.m_Velocity;
            LimitMagnitude(SeperationForce, maxForce);
        }


        AlignmentForce *= kA;
        thisBoid.AddForce(AlignmentForce);
        CohesionForce *= kC;
        thisBoid.AddForce(CohesionForce);
        SeperationForce *= kS;
        thisBoid.AddForce(SeperationForce);
    }

}
void Flock::ChangeConstants(FORCECONSTANTS x, bool direction)
{
    if (x == FORCECONSTANTS::KA)
    {
        (direction) ? kA += 0.1 : kA -= 0.1;
        std::cout << "kA: " << kA << std::endl;
    }
    if (x == FORCECONSTANTS::KC)
    {
        (direction) ? kC += 0.1 : kC -= 0.1;
        std::cout << "kC: " << kC << std::endl;
    }
    if (x == FORCECONSTANTS::KS)
    {
        (direction) ? kS += 0.1 : kS -= 0.1;
        std::cout << "kS: " << kS << std::endl;
    }

}

Boid.h

#include <SFML/Graphics.hpp>
#ifndef  BOID_H
#define BOID_H
struct Boid
{
    sf::CircleShape boid;
    float m_radius;

    sf::Vector2f m_Velocity;
    sf::Vector2f m_Force;
    Boid(const sf::Vector2f& init_Pos, const sf::Vector2f init_Vel);
    void AddForce(const sf::Vector2f& force);
    void Move();
};
#endif // ! BOID_H

Boid.cpp:

#include "Boid.h"
Boid::Boid(const sf::Vector2f& init_pos, const sf::Vector2f init_vel)
    :m_Velocity(init_vel)
{
    m_radius = 5;
    boid.setPosition(init_pos);
    boid.setFillColor(sf::Color::White);
    boid.setOutlineThickness(0);
    boid.setOutlineColor(sf::Color::White);
    boid.setPointCount(50);
    boid.setRadius(m_radius);
    boid.setOrigin(m_radius, m_radius);
}
void Boid::AddForce(const sf::Vector2f& force)
{
    m_Force += force;
    m_Velocity += force;
}
void Boid::Move()
{

    boid.move(m_Velocity);
}

Main.cpp:

#include "Flock.h"
#include <string>
#include "Constants.h"
#include <iostream>

#include <iomanip>
int main()
{
    std::string myTitle = "Boids";
    sf::RenderWindow myWindow(sf::VideoMode(WIDTH, HEIGHT), myTitle, sf::Style::Default);

    Flock f(&myWindow);
    while (myWindow.isOpen())
    {
        sf::Event evnt;
        while (myWindow.pollEvent(evnt))
        {
                switch (evnt.type)
                {
                    case sf::Event::EventType::Closed:
                    {
                        myWindow.close();
                    }
                    case sf::Event::EventType::KeyPressed:
                    {
                        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Key::Space))
                        {
                            f.AddBoid(true);
                        }
                        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Key::Num1))
                        {

                            f.ChangeConstants(f.KA, true);
                        }
                        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Key::Num2))
                        {
                            f.ChangeConstants(f.KA, false);
                        }
                        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Key::Num3))
                        {
                            f.ChangeConstants(f.KC, true);
                        }
                        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Key::Num4))
                        {
                            f.ChangeConstants(f.KC, false);
                        }
                        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Key::Num5))
                        {
                            f.ChangeConstants(f.KS, true);

                        }
                        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Key::Num6))
                        {
                            f.ChangeConstants(f.KS, false);
                        }
                    }
                }
        }

        f.Update();
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Hi, in the future please make sure that your title is a description of the purpose of your code, as I edited for you. Can you also indicate in your question: does your code work (a screenshot may be helpful for graphics)? A description of Craig Reynolds Boids may help reviewers understand the mechanism of your code as well. \$\endgroup\$ – L. F. Apr 3 at 12:03
3
\$\begingroup\$

In all, this is a nice, solid effort, especially for a self-avowed "newbie." Keep up the good work! With that, here are some things that may help you improve your program.

Use SFML more fully

I would suggest that it would be a good idea to have the Flock class derive from both the sf::Transformable and sf::Drawable classes. Your Flock::Display() would then become an implementation of Flock::draw. See the SFML tutorial on creating entities for more details.

Think carefully about inheiritance

The Boid class is currently this:

struct Boid
{
    sf::CircleShape boid;
    float m_radius;

sf::Vector2f m_Velocity;
sf::Vector2f m_Force;
Boid(const sf::Vector2f& init_Pos, const sf::Vector2f init_Vel);
void AddForce(const sf::Vector2f& force);
void Move();
};

I would make several changes to this:

struct Boid : public sf::CircleShape
{
    float m_radius = 2;
    sf::Vector2f m_Velocity;
    sf::Vector2f m_Force;
    Boid(const sf::Vector2f& init_Pos, const sf::Vector2f init_Vel);
    void AddForce(const sf::Vector2f& force);
};

I've changed the Boid to inherit from sf::CircleShape which cleans things up. For example, lines like these:

sf::Vector2f b1 = boid1.boid.getPosition();
sf::Vector2f b2 = boid2.boid.getPosition();

are now a bit simpler with this:

sf::Vector2f b1 = boid1.getPosition();
sf::Vector2f b2 = boid2.getPosition();

I also moved the default value for m_radius into the declaration rather than the constructor. Finally, I removed the Move function entirely, since we can now use sf::CircleShape::move directly:

void Flock::Move()
{
    for (Boid& boid : m_boidStorage)
    {
        boid.move(boid.m_Velocity);
        Borders(boid);
    }
}

However, see the next suggestion.

Think carefully about class responsibilities

It seems to me that Borders should be a function of Boid rather than of Flock. Just pass in width and height and I think you'll find it a much nicer looking piece of code. Also, Borders is a noun rather than a verb which makes it a somewhat misleading name for a function. I'd call it Wrap instead. Similarly, the only uses of m_randomEngine, m_uniform, m_gaussian are within AddBoid which strongly suggests to me that they would be better as static members of Boid and used for a default constructor of Boid.

Put default values in the class definition

Rather than assigning values in the constructor, such as for kA, kB, etc. it's better to put those into the class definition. If you do that and provide a default Boid constructor as mentioned above, your Flock constructor could look like this:

Flock::Flock(sf::RenderWindow* winPtr) :
    m_boidStorage(TOTAL_BOIDS),
    m_winPtr(winPtr)
{
}

This also simplifies AddBoid by removing the need for a passed parameter.

Use emplace_back instead of push_back where appropriate

We don't really need to separately construct a Boid and push it onto the std::vector. Instead, we can do both in a single step:

void Flock::AddBoid()
{
    static const sf::Vector2f middle{ WIDTH / 2.0, HEIGHT / 2.0 };
    static const sf::Vector2f zero{ 0, 0 };
    m_boidStorage.emplace_back(middle, zero);
    maxVelocity += 0.05;
}

This assumes that the previous suggestion is also used.

Watch out for missing breaks

A common error in C++ is to forget to put a break; at the bottom of each case to prevent it from falling through. There is such a missing break; in Main.cpp.

Use const where appropriate

Helper functions such as VectorMagnitude don't modify the underlying Flock object and so should be const. However, in this case, see the next suggestion instead.

Omit helper functions from the interface

There's no need to use VectorMagnitude outside of the Flock implementation code, so I'd recommend that it (and the similar functions) be non-class member static functions instead.

Eliminate unused functions

The VecBoidDistance is never used and can be eliminated.

Consider refactoring using objects

The MassCalculate function has a lot of nearly duplicate code. I would be inclined to refactor that into three Influence objects with Influence being a pure virtual base class that would do all of the common stuff and two virual functions updateForce and applyForce.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I really appreciate the feedback. I will implement these right away! Edward, I am in desperate need of a mentor since none of my friends know how to code. Would you be so kind so as to help me improve my code in the future? I would really, really appreciate it. I understand if you do not want to do this however. \$\endgroup\$ – jasbindra00 Apr 4 at 13:17
  • \$\begingroup\$ That's what we're here for. You might also participate in the 2nd monitor if you have questions. \$\endgroup\$ – Edward Apr 4 at 13:22
  • \$\begingroup\$ Edward in your point "Omit helper functions from the interface", where would you put these functions instead? In main.cpp, or a separate header called "helper.h"/ any other spot? \$\endgroup\$ – jasbindra00 Apr 4 at 13:43
  • \$\begingroup\$ Leave them exactly where they are and make them static. You don't need a header file for those. \$\endgroup\$ – Edward Apr 4 at 13:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.