Hey

I'm posting this in the beginner forums just because I think this problem is going to turn out to be some silly little mistake i've made

Overview of my setup:

I have a 'Particle' class. And inside another class I have an array of them: (header file)

#define PARTICLE_COUNT 10

protected:
    Particle particles_m[PARTICLE_COUNT];

.

The particles have a default constructor that puts everything in a valid state. Later on I call Particle::Create() on all of the array to initialize them (not sure if this is relevant but inluding anyway just in case..)

My problem is that whenever I call Particle::Draw() and Particle::Update(), all of the instance's members are garbage data. Checked with debugger. The bit that's confusing me is that if I go up one level in the call stack and examine the array of Particles, everything seems ok and the instance's members are fine and how they should be. But one level down into the actual method in the Particle class and my debugger shows me that everything is garbage data. How is this happening?

Here's my particle class declaration if it's any help:

class Particle
{
public:

    Particle();
    Particle(sf::Color colour_p, float x_p, float y_p, float x_speed_p, float y_speed_p); // Calls Create()

    void Create(sf::Color colour_p, float x_p, float y_p, float x_speed_p, float y_speed_p);
    bool IsDead();

    void Update();
    void Draw(Engine* engine_p);

protected:


    sf::Color colour_m;
    float size_m; // Length of the sides of the square
    float x_m, y_m, x_speed_m, y_speed_m;



    // *This bit is sorta half finished, not called by any other methods yet*
    inline void GenerateColourMod(sf::Uint8& colour_byte_p);
    void GenerateSizeMod(float& size_p);
    float GenerateSpeedMod();


};

I think it's worth noting that there is nothing being allocated on the heap.

Thanks for the help, let me know if you need any more code.

The bug is most likely in the code you didnt show. Wild guess: maybe calling the method from a wrongly casted pointer or out of bounds of the array.

Btw., I would use a static const instead of the define, because the preprocessor doesnt know about C++ scopes.

It'd help to give the implementation of it in the cpp file.

If you're attempting to use stack allocated data and it's showing up as garbage inside a method that usually implies you're trying to use a local copy of the data that wasn't intentional, or perhaps you're not setting the class member like you think you are.

You're not debugging using a Release build, right?

You're not debugging using a Release build, right?

Thankfully not..

Here's the dump of weather.h (declares classes Weather AND Particle)

/*
    
    weather.h

    Weather
    A particle system controller that the level class uses to control weather effects

*/


#include "base.h"
#include "engine.h"

#include <SFML/System.hpp>




#define PARTICLE_COUNT 10

#define SPAWN_TIMER_LIMIT 1000

#define PARTICLE_COLOUR_VARIATION 7
#define PARTICLE_MAX_SIZE 20



// PARTICLE_MAX_SIZE denotes the max size (in pixels) that a particle can be (square shape)
// SPAWN_TIMER_LIMIT is in milliseconds






enum WEATHER_TYPE {W_NONE, W_RAIN, W_SNOW, W_SAND};







class Particle
{
public:

    Particle();
    Particle(sf::Color colour_p, float x_p, float y_p, float x_speed_p, float y_speed_p); // Calls Create()

    void Create(sf::Color colour_p, float x_p, float y_p, float x_speed_p, float y_speed_p);
    bool IsDead();

    void Update();
    void Draw(Engine* engine_p);

protected:


    sf::Color colour_m;
    float size_m; // Length of the sides of the square
    float x_m, y_m, x_speed_m, y_speed_m;
    



    void GenerateColourMod(sf::Uint8& colour_byte_p);
    void GenerateSizeMod(float& size_p);
    float GenerateSpeedMod();


};







class Weather
{
public:

    // Constructor and pointer setting
    Weather();
    void SetEngine(Engine* engine_p);


    // Weather control methods
    void StartWeather(WEATHER_TYPE type_p); // Starts the drawing and simulation of a specific type of weather system
    void StopWeather(); // Stops the current weather system being simulated (if any)

    // Returns the type of weather system being simulated (W_NONE if no weather is being simulated)
    WEATHER_TYPE IsPlaying()
    {
        if(running_m)
            return current_weather_type_m;
        else
            return W_NONE;
    }


    // Loop methods
    void Draw();
    void Update();


protected:

    // The pointer to the engine (should be checked at the top of every method that uses this)
    Engine* engine_m;

    // The current weather system being simulated (W_NONE if no weather is being simulated)
    WEATHER_TYPE current_weather_type_m;

    // If the simulation is running
    bool running_m;

    // The array of particles
    Particle particles_m[PARTICLE_COUNT];

    // Particle spawn and update variables
    sf::Clock particle_spawn_timer_m;
    unsigned int particle_count_m;

private:
};

.

And the dump of particle.cpp

// implements the Particle class in weather.h

#include "weather.h"
#include <cstdlib>



Particle::Particle()
{
    colour_m = sf::Color::Red;

    size_m = 1.0f;

    x_m = 0.0f;
    y_m = 0.0f;
    x_speed_m = 0.0f;
    y_speed_m = 0.0f;
}

Particle::Particle(sf::Color colour_p, float x_p, float y_p, float x_speed_p, float y_speed_p)
{
    Create(colour_p, x_p, y_p, x_speed_p, y_speed_p);
}

void Particle::Create(sf::Color colour_p, float x_p, float y_p, float x_speed_p, float y_speed_p)
{
    // Set all values
    colour_m = colour_p;
    size_m = 1.0f;
    x_m = x_p;
    y_m = y_p;
    x_speed_m = x_speed_p;
    y_speed_m = y_speed_p;


    // Mutate the colour slightly
    GenerateColourMod(colour_m.r);
    GenerateColourMod(colour_m.g);
    GenerateColourMod(colour_m.b);
    GenerateColourMod(colour_m.a);

}





bool Particle::IsDead()
{
    if(x_m > 640+64 || y_m > 480+64)
        return true;
    else
        return false;
}






void Particle::Update()
{
    // Move particle
    x_m += x_speed_m;
    y_m += y_speed_m;
}





void Particle::Draw(Engine* engine_p)
{   

    sf::RectangleShape particle(sf::Vector2f(size_m, size_m));  // set size
    particle.setPosition(x_m, y_m); // set position
    particle.setFillColor(colour_m); // set colour

    engine_p->sfml_window_m.draw(particle);

}













////////////////////////
// Randomisation Code //
////////////////////////



void Particle::GenerateColourMod(sf::Uint8& colour_byte_p)
{
    // Convert the 8 bit value to 16 bit 
    // (This is to handle cases where the value becomes bigger than 255)
    sf::Uint16 colour_byte_16 = (sf::Uint16)colour_byte_p;

    // Add +/- PARTICLE_COLOUR_VARIATION to the value
    colour_byte_16 = rand() % (PARTICLE_COLOUR_VARIATION*2);
    colour_byte_16 -= PARTICLE_COLOUR_VARIATION;

    // Restrict the value to unsigned 8 bit limits
    if(colour_byte_16 < 0)
        colour_byte_16 = 0;
    if(colour_byte_16 > 255)
        colour_byte_16 = 255;

    // Convert back to 8 bit and set the original reference
    colour_byte_p = (sf::Uint8)colour_byte_16;
}



void Particle::GenerateSizeMod(float& size_p)
{
    size_p *= rand() % PARTICLE_MAX_SIZE;
}


float Particle::GenerateSpeedMod()
{
    // TODO: Finish this
    return 1.0f;
}

.

Also weather.cpp

Note that I don't simply loop through the particle array. There's some weird trickery to get the particles to look like they're spawning in one by one.

The SPAWN_TIMER_LIMIT macro defines the time (in milliseconds) it takes for a new particle to spawn

// implements the Weather class in weather.h

#include "weather.h"
#include <cstdlib>

#include <SFML/System.hpp>


Weather::Weather()
{
    engine_m = NIL;

    current_weather_type_m = W_NONE;
}


void Weather::SetEngine(Engine* engine_p)
{
    IsPointerNull((void*)engine_p,"Weather::SetEngine()");

    engine_m = engine_p;
}






void Weather::StartWeather(WEATHER_TYPE type_p)
{
    running_m = true;
    current_weather_type_m = type_p;


    sf::Color col;
    // Commented out for debug
    /*
    switch(current_weather_type_m)
    {
        case W_RAIN:
            col.r = 27;
            col.g = 126;
            col.b - 224;
        break;

        case W_SNOW:
            col.r = 227;
            col.g = 227;
            col.b - 227;
        break;

        case W_SAND:
            col.r = 214;
            col.g = 203;
            col.b - 118;
        break;
    }
    */
    col = sf::Color::Blue;


    for(unsigned int c=0; c<PARTICLE_COUNT; c++)
    {
        particles_m[c].Create(col,0,0,4,4);
    }

    
    // Restart spawn timer
    particle_spawn_timer_m.restart();
    particle_count_m = 0;


}


void Weather::StopWeather()
{
    running_m = false;
    particle_count_m = 0;
}





void Weather::Update()
{
    if(IsPointerNull((void*)engine_m,"Weather::Update()"))
        return;

    if(!running_m)
        return;


    if(particle_count_m < PARTICLE_COUNT)
    {
        sf::Time el = particle_spawn_timer_m.getElapsedTime();
        if(el.asMilliseconds() >= SPAWN_TIMER_LIMIT)
        {
            particle_count_m ++;
            //if(particle_count == PARTICLE_COUNT)
            //{
                //std::cout << "ERROR PARTICLE COUNT";
                //particle_count_m = 
            //}
            particle_spawn_timer_m.restart();
        }
    }



    for(unsigned int p=0; p<particle_count_m; p++)
    {
        particles_m[p].Update();
    }
}




void Weather::Draw()
{
    if(IsPointerNull((void*)engine_m,"Weather::Draw()"))
        return;

    if(!running_m)
        return;

    
    for(unsigned int p=0; p<particle_count_m; p++)
    {
        particles_m[p].Draw(engine_m);
    }
}

.

I feel bad just dumping code on everyone lol. If you need any clarifications on anything just ask.

Hi. Do you use a std::vector or list. If so you need a copy construct in your class.

What's the declaration of particles_m? If your able to use C++11, does this help:

Weather::Weather(): particles{} 
{
    engine_m = NIL;

    current_weather_type_m = W_NONE;
}

Why are you adding _m after everything?!? it just make the code harder to read.

If you really like this notation you shoud at least put the m before, like this ---> m_particles, for example, but imo, this is better:

struct float2 {
    float x,y;
};
 
float2 Position, Speed;

than

float x_m, y_m, x_speed_m, y_speed_m;

That would make you're code wayyyyyy easier to read afterward.

Ex.

void Particle::Update()
{
    // Move particle
    Position.x += Speed.x;
    Position.y += Speed.y;
}

//or even better with operator overloading

Position += Speed;

Also

engine_m = NIL;   

NIL? I suggest using NULL instead.

Can't see anything wrong with the code, except maybe:

engine_p->sfml_window_m.draw(particle);

wouldn't be better to pass the particle address instead? Like:

engine_p->sfml_window_m.draw(&particle);

My guess is you probably have a bad pointer somewhere, but who knows, since i can't debug it i can't really say.

Why are you adding _m after everything?!? it just make the code harder to read.

Point taken, thanks. I had problems in previous projects distinguishing member variables and arguments so I thought i'd make it clear what type all variables were. You're right about the readability though, 'x_m' and 'y_m' just look stupid -.- I'll think of a better idea for my next project.

As for the bug, I managed to solve it by changing the container from a raw array to an std::vector No idea why this worked but a few lines changed and bam. Particles.. I think I must have been missing something.

Anyway, thanks for everyone's help and time. I appriciate it.

the vector allocates memory in the "free store," which means your bug MAY still be there, and its not showing up (yet)

that is, unless there actually was something wrong with that simple array, which i highly doubt :)

honestly, i would go back to that array and investigate further

one thing i would do if i got stuck was write verification functions that id put on the backside of every class in the engine

if your engine is constructed in such a way, you could verify the contents of everything at all times

it would be really slow, but lots of interesting things could turn up :)

how would you know?

well for starters, you create 10 particles and set some values

copy those values to a heap allocated memory segment, then just before changing the values in the particles verify that the old contents got through a frame untouched

if they didn't you can start verifying more and more places in the engine, until you start to narrow down where the overwrite is happening

of course this all is just for the unlucky case where there is an actual overwrite, which it may not be

you never know though :)