Trying to create a function that opens a window, please help.

[Lorcatar]

Alright, this compiles fine but it crashes when it runs. Any suggestions?

#include "SFML/Graphics.hpp"
#include<iostream>
#include<array>

sf::RenderWindow* startWindow()
{
        std::vector<sf::VideoMode> i = sf::VideoMode::getFullscreenModes();
        sf::RenderWindow window(i.front(), "SFML WORKS!", sf::Style::Fullscreen);

        return &window;
}

int main()
{
        sf::RenderWindow *window = startWindow();
        // Create a graphical text to display
        sf::Font font;
        if (!font.loadFromFile("Resources/UbuntuMono-R.ttf"))
        {
                std::cerr << "Can't load font you fucking shit loser.";
                return EXIT_FAILURE;
        }
        sf::Text text("Hello SFML", font, 50);

        while (window->isOpen())
        {
                sf::Event event;

                while (window->pollEvent(event))
                {
                        switch (event.type)
                        {
                        case sf::Event::Closed:
                                window->close();
                                break;
                        }
                }
                window->clear();
                window->draw(text);
                window->display();
        }
}

[Ixrec]

You're returning a pointer to an object just before it gets destroyed.  This is about as undefined as undefined behavior gets.

The simplest/best/safest solution is to default construct the actual window in main(), then pass it by reference to a function that takes care of looking at fullscreen modes and then calls create() on it.

In general, you should avoid using raw pointers (especially owning raw pointers like this one) unless you have a very good reason to, because as you've just discovered, they're very error-prone.

[Lorcatar]

You're returning a pointer to an object just before it gets destroyed.  This is about as undefined as undefined behavior gets.

The simplest/best/safest solution is to default construct the actual window in main(), then pass it by reference to a function that takes care of looking at fullscreen modes and then calls create() on it.

In general, you should avoid using raw pointers (especially owning raw pointers like this one) unless you have a very good reason to, because as you've just discovered, they're very error-prone.

Thank you for explaining so clearly. That solved my problem.

#include "SFML/Graphics.hpp"
#include<iostream>
#include<array>

void startWindow(sf::RenderWindow *window)
{
        std::vector<sf::VideoMode> i = sf::VideoMode::getFullscreenModes();
        window->create(i.front(), "SFML WORKS!", sf::Style::Fullscreen);
}

int main()
{
        sf::RenderWindow window = sf::RenderWindow();
        startWindow(&window);
        // Create a graphical text to display
        sf::Font font;
        if (!font.loadFromFile("Resources/UbuntuMono-R.ttf"))
        {
                std::cerr << "Can't load font you fucking shit loser.";
                return EXIT_FAILURE;
        }
        sf::Text text("Hello SFML", font, 50);

        while (window.isOpen())
        {
                sf::Event event;

                while (window.pollEvent(event))
                {
                        switch (event.type)
                        {
                        case sf::Event::Closed:
                                window.close();
                                break;
                        }
                }
                window.clear();
                window.draw(text);
                window.display();
        }
}

[Ixrec]

Might as well toss out a few more nitpicks while they're easy to fix:

1) When you write this:

sf::RenderWindow window = sf::RenderWindow();

A temporary RenderWindow gets constructed, then RenderWindow "window" gets constructed using copy assignment, then the temporary RenderWindow gets destroyed.  This is kinda wasteful, so just let "window" default construct itself:

sf::RenderWindow window; // yes, this is a default constructor call

2) Passing by pointer is certainly a big improvement over your first attempt, but when I said "pass by reference" I was referring to C++ references, which are often better than raw pointers.  startWindow with references would be:

void startWindow(sf::RenderWindow& window) // &, not *
{
        std::vector<sf::VideoMode> i = sf::VideoMode::getFullscreenModes();
        window.create(i.front(), "SFML WORKS!", sf::Style::Fullscreen); // ., not ->
}

References can't be null, they don't allow unsafe things like pointer arithmetic, and they retain the efficiency bonus you get from passing by pointer instead of (copied) value.  So for the simple use case of efficiently passing arguments, references are generally preferable to pointers.

3)

#include<array>

This is interesting since you're using std::vector and no std::arrays.  Perhaps you should include <vector> instead. =)

[Lorcatar]

Might as well toss out a few more nitpicks while they're easy to fix:

1) When you write this:

sf::RenderWindow window = sf::RenderWindow();

A temporary RenderWindow gets constructed, then RenderWindow "window" gets constructed using copy assignment, then the temporary RenderWindow gets destroyed.  This is kinda wasteful, so just let "window" default construct itself:

sf::RenderWindow window; // yes, this is a default constructor call

2) Passing by pointer is certainly a big improvement over your first attempt, but when I said "pass by reference" I was referring to C++ references, which are often better than raw pointers.  startWindow with references would be:

void startWindow(sf::RenderWindow& window) // &, not *
{
        std::vector<sf::VideoMode> i = sf::VideoMode::getFullscreenModes();
        window.create(i.front(), "SFML WORKS!", sf::Style::Fullscreen); // ., not ->
}

References can't be null, they don't allow unsafe things like pointer arithmetic, and they retain the efficiency bonus you get from passing by pointer instead of (copied) value.  So for the simple use case of efficiently passing arguments, references are generally preferable to pointers.

3)

#include<array>

This is interesting since you're using std::vector and no std::arrays.  Perhaps you should include <vector> instead. =)

Thanks, this is what I have so far now. Tested and everything works, thanks for helping me out!

#include "SFML/Graphics.hpp"
#include<iostream>
#include<vector>

void startWindow(sf::RenderWindow& window)
{
        std::vector<sf::VideoMode> i = sf::VideoMode::getFullscreenModes();
        window.create(i.front(), "SFML WORKS!", sf::Style::Fullscreen);
}

void createText(sf::Text& text, sf::Font& font)
{
        font.loadFromFile("Resources/UbuntuMono-R.ttf");
        text.setFont(font);
        text.setCharacterSize(50);
        text.setString("Hello SFML");
}

int main()
{
        sf::RenderWindow window;
        sf::Font font;
        sf::Text text;

        //create window
        startWindow(window);
       
        // Create a graphical text to display
        createText(text, font);

        while (window.isOpen())
        {
                sf::Event event;

                while (window.pollEvent(event))
                {
                        switch (event.type)
                        {
                        case sf::Event::Closed:
                                window.close();
                                break;
                        }
                }
                window.clear();
                window.draw(text);
                window.display();
        }
}

[Hapax]

You should still be testing to see if the font loading failed (not that I particularly approve of your error message).
Just returning on failing would only leave the function and then continue in main(), although it would still stop you from setting up the text to use that font if it did fail.

Two options here are:

• return a bool and use your function in a similar way to .loadFromFile()
• throw an exception