Getting segfault at RenderWindow method callings

[nischu]

Each time I try to invoke any method of my RenderWindow object, I get a segfault:

At this example at 1st at line 117

#include <SFML/Graphics.hpp>
#include <array>
#include <iostream>
#include <cstdlib>
#include <forward_list>

const int ROWS = 10;
const int COLS = 20;
const int CELL_SIZE = 16;

namespace LifeName {
enum Life { Grass = 1, Cow };
}

class Life : public sf::Drawable
{
public:
    int age;
    sf::Sprite sprite;
    sf::Vector2<int> pos;
    LifeName::Life lifeName;
    bool is_spawn;
public:
    Life() : age(0), is_spawn(false)
    { sprite.setColor(sf::Color(0xff,0xff,0xff,0x00));}
   
    virtual void draw( sf::RenderTarget & rt, sf::RenderStates s) const override
    {
        rt.draw( sprite );
    }
    virtual void live() = 0;
    virtual void spawn( std::array<std::array<int,COLS>,ROWS> & world) const = 0;
};


class Grass : public Life
{
public:
    void draw( sf::RenderTarget & rt, sf::RenderStates s) const override
    {
        rt.draw(sprite);
    }
    void live() override
    {
        if( is_spawn == true) is_spawn = false;
       
        if( age ++ > 10)
        {
           is_spawn = true;
        }
    }
    void spawn(std::array<std::array<int,COLS>,ROWS> & world) const override {}
};
class Animal : public Life
{

};


class Game : public sf::NonCopyable
{
    std::array<std::array<Life*,COLS>,ROWS> m_grid;
    std::forward_list<Life*> m_lifeList;
    sf::Vector2<int> m_cellSize;
    sf::RenderWindow & m_window;
    sf::Texture m_grassTexture;

public:  
    Game() : m_cellSize(CELL_SIZE,CELL_SIZE),
        m_window( *new sf::RenderWindow(sf::VideoMode(COLS*CELL_SIZE,
        ROWS*CELL_SIZE),"Test") )
    {
        m_grassTexture.loadFromFile("/home/nico/develop/c++/sfml/gol/media/64x16_grass.png");
        for ( int row = 0;  row < m_grid.size(); ++row ) {
            for( int col = 0; col < m_grid[row].size();  ++col)
            {
                if( std::rand()%10 == 0) {
                    Life * grassCell = new Grass;
                    m_grid[col][row] = grassCell;
                    grassCell->pos = std::move(sf::Vector2<int>(col, row));
                    m_lifeList.push_front(grassCell);
                    grassCell->sprite.setPosition( col*CELL_SIZE, row*CELL_SIZE );
                    grassCell->sprite.setTexture( m_grassTexture );
                    grassCell->sprite.setTextureRect(
                        sf::Rect<int>(0 ,0, m_cellSize.x, m_cellSize.y )
                    );
                }
                else {
                    m_grid[col][row] = nullptr;
                }
            }
        }
    }
    ~Game()
    {
        delete &m_window;
        for( Life * life : m_lifeList) delete life;
    }
private:
    void draw()
    {            
        for( auto const life : m_lifeList)
        {
            m_window.draw( * life );
        }
    }
    void update()
    {
        for( Life * life : m_lifeList)
            life->live();
     }
     
public:    
    void run()
    {
        std::cerr << "Mark0\n";  //DEBUG
        while( m_window.isOpen() )    // <- Here I get the segfault
        {
            sf::Event event;
            std::cerr << "Mark1\n";  //DEBUG
            while( m_window.pollEvent(event))
            {
                if( event.type == sf::Event::Closed)
                {
                    m_window.close();
                }
            }
            std::cerr << "Mark2\n"; //DEBUG
            m_window.clear( sf::Color::Black );
            draw();
            std::cerr << "Mark3\n"; //DEBUG
            m_window.display();
            update();
            sf::sleep(sf::seconds(1));
        }
    }
};

int main()
{
    Game game;
    game.run();
}
 

[Hapax]

You should read this:
https://stackoverflow.com/questions/16879988/new-vs-new-in-c

[nogoodname]

You're program seemed to segfault on this line.

m_grid[col][row] = nullptr;
 

It's because you're indexing the matrix out of range (which is undefined behaviour) as you got the row and column backwards. You should be doing m_grid[row][col] instead.
Note that [] doesn't do range check, so if you want to range checking you can use .at().

Another issue is this line.

m_window( *new sf::RenderWindow(sf::VideoMode(COLS*CELL_SIZE,
        ROWS*CELL_SIZE),"Test") )
 

Is there a reason why you're initialising m_window is like this? You could simply make m_window not a reference, and just initialise it normally.

m_window( sf::VideoMode(COLS*CELL_SIZE,
        ROWS*CELL_SIZE),"Test")
 

This way you avoid the unnecessary dynamic allocation.

When you do need to use dynamic allocation. It's best to avoid explicit new when possible, as there's usually better alternatives, such as unique_ptr and shared_ptr. Learn about RAII, and you can reduce error-prone manual resource management.

Another issue is that your Life class is has no virtual destructor. Polymorphic base classes need one to prevent undefined behaviour. It's quite simple to so.

class Life : public sf::Drawable
{
public:
//....
    virutal ~Life() = default;
};
 

As usual when you're defining destructors, you should be aware of the rule of 5.


Well that's the major issues, onto the very minor things.

Minor things that you may want to know. Firstly sf::Noncopyable also deletes the move functions as well as copy which may not be what you want. Secondly, it is preferably for the constants at the top (ROWS) to be constexpr instead of const, so you don't get compiler errors down the road. Thirdly, if you want scoped enums you can you enum classes as it also provides strong typing. Fourthly, having both virtual and override in a function declaration is redundant as you cannot override a non-virtual function, so you can simply just have override.