Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: Simplifying SFML  (Read 12605 times)

0 Members and 2 Guests are viewing this topic.

Alf P. Steinbach

  • Newbie
  • *
  • Posts: 5
    • View Profile
    • Email
Simplifying SFML
« on: March 06, 2014, 06:39:35 am »
I was looking for a portable C++ framework for some simple simulation of planets/bodies dancing around in space, which was meant for some writings. I tried SDL but it seemed too low level. SFML however seems all right so far (I just worked through some of the tutorials).

What I want to expose of code is just the logical model, with a minimum of support code.

So I factored out the support code so that a main program (working code), with antialised drawing and support for window resizing without stretching, looks like this, where xsf is my namespace for this:

#include <xsf/all.h>        // xfs::*, fs::*

class Moving_ball
    : public xsf::Model
{
using Circle = xsf::shape::Circle;
private:
    Circle          ball_{ Circle::Params()
        .radius( 10.0 )
        .fillcolor( sf::Color::Green )
        };
    xsf::Size<>     velocity_{ 30*3.14159f, 30*2.71828f };

    void on_update( sf::Time const time_now, sf::Time const dt ) override
    {
        XSF_INTENTIONALLY_UNUSED( time_now );

        float const seconds = dt.asSeconds();
        ball_.move( seconds*velocity_ );

        sf::Vector2f const position = ball_.getPosition();
        if( position.x < 0 && velocity_.x < 0 )     { velocity_.x *= -1; }
        if( position.x > 100 && velocity_.x > 0 )   { velocity_.x *= -1; }
        if( position.y < 0 && velocity_.y < 0 )     { velocity_.y *= -1; }
        if( position.y > 100 && velocity_.y > 0 )   { velocity_.y *= -1; }
    }

public:
    Moving_ball()
    { drawables_.insert( &ball_ ); }
};

auto main()
    -> int
{
    Moving_ball model;
    xsf::Window window( "SFML works, yay!", &model );
    xsf::Controller( &model ).process_events();
}
 

But maybe others have done things in the same direction before, and there is already some C++ support framework for SFML that reduces code to something like the above?

Cheers,

- Alf

PS: This thing is still only at the stage of exploring the basics of SFML, and wrapping it in C++. I just think it's a good idea to get feedback before one has done too much! Both about directions, technicalities and alternatives.

PPS: The support code, complete as-of-this-posting (I have not yet even split it up in separate headers):

#pragma once
// Copyright © 2014 Alf P. Steinbach.

#include <SFML/Graphics.hpp>

#include <algorithm>    // std::count_if
#include <functional>   // std::function
#include <set>          // std::set

#define XSF_INTENTIONALLY_UNUSED( arg )   (void) arg; struct arg;

namespace xsf {

    //---------------------------------------- Geometry:

    template< class Number = float >
    class Point
        : public sf::Vector2< Number >
    {
    using Base = sf::Vector2< Number >;
    public:
        Point(): Base( 0, 0 ) {}
        Point( Number const x, Number const y ): Base( x, y ) {}
    };

    template< class Number = float >
    class Size
        : public sf::Vector2< Number >
    {
    using Base = sf::Vector2< Number >;
    public:
        Size(): Base( 0, 0 ) {}
        Size( Number const x, Number const y ): Base( x, y ) {}
    };

    inline
    auto antialiased()
        -> sf::ContextSettings
    {
        sf::ContextSettings settings;

        settings.antialiasingLevel = 8;
        return settings;
    }


    //---------------------------------------- Time:

    inline
    auto abs( sf::Time const& time )
        -> sf::Time
    { return (time < sf::Time()? -time : time); }

    inline
    auto sign( sf::Time const& time )
        -> int
    { return (time < sf::Time()? -1 : time == sf::Time()? 0 : +1); }


    //---------------------------------------- Model:

    class Model
    {
    friend class Controller;
    private:
        sf::Time    max_dt_;

        // dt, for "delta of time", is the time span since the previous update.
        virtual
        void on_update( sf::Time const time_now, sf::Time const dt )
            = 0;

    protected:
        std::set< sf::Drawable const* > drawables_;

        void update_to( sf::Time const time_now, sf::Time const dt )
        {
            sf::Time const abs_dt   = abs( dt );
            if( abs_dt <= max_dt_ )
            {
                on_update( time_now, dt );
            }
            else
            {
                bool const      is_positive     = (dt > sf::Time());
                sf::Time const  previous_time   = time_now - dt;

                sf::Time elapsed;
                for( elapsed = max_dt_;  elapsed < abs_dt;  elapsed += max_dt_ )
                {
                    if( is_positive )
                    {
                        on_update( previous_time + elapsed, max_dt_ );
                    }
                    else
                    {
                        on_update( previous_time - elapsed, -max_dt_ );
                    }
                }
                elapsed -= max_dt_;
                if( elapsed < abs_dt )
                {
                    sf::Time const remainder = abs_dt - elapsed;
                    on_update( time_now, (is_positive? remainder : -remainder) );
                }
            }
        }

        void set_max_dt( sf::Time const& new_max_dt )
        { max_dt_ = new_max_dt; }

    public:
        auto max_dt() const -> sf::Time { return max_dt_; }

        auto drawables() const
            -> std::set< sf::Drawable const* > const&
        { return drawables_; }

        virtual ~Model() {}

        Model()
            : max_dt_( sf::seconds( 0.1f ) )
        {}
    };

    namespace shape {

        class Circle
            : public sf::CircleShape
        {
        using Base = sf::CircleShape;
        public:
            class Params
            {
            friend class Circle;
            private:
                sf::Color       fillcolor_      { sf::Color::White };
                int             n_points_       { 90 };
                Point<float>    position_       { 0, 0 };
                float           radius_         { 100 };

            public:
                using Outer_class = Circle;

                auto fillcolor( sf::Color const& value )
                    -> Params&
                { fillcolor_ = value; return *this; }

                auto n_points( int const value )
                    -> Params&
                { n_points_ = value; return *this; }

                auto position( Point<float> const& value )
                    -> Params&
                { position_ = value; return *this; }

                auto radius( float const value )
                    -> Params&
                { radius_ = value; return *this; }
            };

            void move_to( Point<float> const& position )
            { Base::setPosition( position ); }

            Circle( float const radius, int const n_points = 90 )
                : Base( radius, n_points )
            {}

            Circle( Params const& params )
                : Base( params.radius_, params.n_points_ )
            {
                Base::setFillColor( params.fillcolor_ );
                Base::setPosition( params.position_ );
            }
        };

    }  // namespace shape


    //---------------------------------------- Rendering:

    class Controller;

    class Window
        : public sf::RenderWindow
    {
    friend class Controller;
    private:
        Model const*    p_model_;

        // Called by controller's event processing loop.
        static auto current_windows()
            -> std::set<Window*>&
        {
            static std::set<Window*> the_windows;
            return the_windows;
        }

        // Called by controller's event processing loop.
        virtual void on( sf::Event const event )
        {
            switch( event.type )
            {
                case sf::Event::Closed:
                {
                    close();
                    break;
                }
                case sf::Event::Resized:
                {
                    // Avoid stretching of graphics to window size.
                    sf::FloatRect const visible_area(
                        0, 0, (float)event.size.width, (float)event.size.height
                        );
                    setView( sf::View( visible_area ) );
                    break;
                }
            }
        }

        virtual void render_model()
        {
            if( p_model_ != nullptr )
            {
                for( auto p_drawable : p_model_->drawables() )
                {
                    draw( *p_drawable );
                }
            }
        }

        // Called by controller's event processing loop.
        virtual void render()
        {
            clear();
            render_model();
        }

    public:
        ~Window() override
        { current_windows().erase( this ); }

        explicit Window(
            char const* const   title,
            Model const* const  p_model = nullptr,
            Point<int> const&   size    = Point<int>( 320, 200 )
            )
            : sf::RenderWindow(
                sf::VideoMode( size.x, size.y ),
                title,
                sf::Style::Default,
                antialiased()
                )
            , p_model_( p_model )
        {
            sf::Window::setVerticalSyncEnabled( true );
            current_windows().insert( this );
        }
    };


    //---------------------------------------- Control:

    class Controller
    {
    public:
        using Event_processing_func = std::function< void( Window&, sf::Event const& ) >;

        static
        void default_event_processing( Window&  window, sf::Event const& event )
        { window.on( event ); }

        using Rendering_func = std::function< void( Window& ) >;

        static
        void default_rendering( Window& window )
        { window.render(); }

    private:
        sf::Clock   clock_;
        sf::Time    previous_time_;
        Model*      p_model_;

        void update()
        {
            sf::Time const current_time  = clock_.getElapsedTime();
            if( p_model_ != nullptr )
            {
                p_model_->update_to( current_time, current_time - previous_time_ );
            }
            previous_time_ = current_time;
        }

        void dispatch_events( Event_processing_func const process_event )
        {
            sf::Event event;
            for( auto const p_window : Window::current_windows() )
            {
                while( p_window->pollEvent( event ) )
                {
                    process_event( *p_window, event );
                }
            }
        }

        auto render_and_count_open_windows( Rendering_func const render )
           -> int
         {
            int n = 0;
            for( auto const p_window : Window::current_windows() )
            {
                if( p_window->isOpen() )
                {
                    render( *p_window );
                    p_window->display();
                    ++n;
                }
            }
            return n;
        }

    public:
        void process_events(
            Rendering_func const        render          = default_rendering,
            Event_processing_func const process_event   = default_event_processing
            )
        {
            for( ;; )
            {
                update();
                dispatch_events( process_event );

                int const n_open_windows = render_and_count_open_windows( render );
                if( n_open_windows == 0 ) { break; }
            }
        }

        Controller( Model* p_model = nullptr )
            : p_model_( p_model )
        {}
    };

}  // namespace xsf
 

eXpl0it3r

  • SFML Team
  • Hero Member
  • *****
  • Posts: 11034
    • View Profile
    • development blog
    • Email
AW: Simplifying SFML
« Reply #1 on: March 06, 2014, 08:11:49 am »
It's definitely an interesting approach, however "Simplifying SFML" might suggest that you rewrote SFML, which is not the case, you just built your own framework with SFML.

Is the "automation" really worth the effort in the end? How do you go about complex entity systems? How would you use multiple objects with the current system?

As for the code, I find the "auto func() -> type" a bit misused, e.g. the main function just returns int, there's no way it would change, thus the auto keyword is very artificial. ;)
Official FAQ: https://www.sfml-dev.org/faq.php
Official Discord Server: https://discord.gg/nr4X7Fh
——————————————————————
Dev Blog: https://duerrenberger.dev/blog/

Alf P. Steinbach

  • Newbie
  • *
  • Posts: 5
    • View Profile
    • Email
Re: AW: Simplifying SFML
« Reply #2 on: March 06, 2014, 09:36:37 am »
It's definitely an interesting approach, however "Simplifying SFML" might suggest that you rewrote SFML, which is not the case, you just built your own framework with SFML.

Oh yes, maybe I should have written "SFML use".

Is the "automation" really worth the effort in the end? How do you go about complex entity systems? How would you use multiple objects with the current system?

I don't know what "complex entity systems" are, but using multiple objects are trivial: in the same way as shown for single object. That is, the model then just contains multiple objects, and the std::set of display object pointers then has two or more pointers instead of just one. This code also supports multiple windows, with the app terminating when no open window is left, although it does not support OpenGL with multiple windows.

I tried OpenGL but discovered that (at least with SFML) it can only draw in one window at a time, very odd. Not sure how to best deal with that with multiple windows?

As for the code, I find the "auto func() -> type" a bit misused, e.g. the main function just returns int, there's no way it would change, thus the auto keyword is very artificial. ;)

I use `auto` as  a single function syntax. With an exception for void functions. In addition to being a single unified syntax, which IMHO is an advantage in and of itself, this supports rapid scanning of code since function name and return type appears in fixed places, in particular regardless of length of complex template stuff.

Is there something like this framework already?

Thanks!,

- Alf


Tank

  • SFML Team
  • Hero Member
  • *****
  • Posts: 1486
    • View Profile
    • Blog
    • Email
Re: Simplifying SFML
« Reply #3 on: March 06, 2014, 10:47:41 am »
As much as I tried, but I find your code extremely hard to read. :|

Mario

  • SFML Team
  • Hero Member
  • *****
  • Posts: 879
    • View Profile
Re: AW: Simplifying SFML
« Reply #4 on: March 06, 2014, 10:53:33 am »
As for the code, I find the "auto func() -> type" a bit misused, e.g. the main function just returns int, there's no way it would change, thus the auto keyword is very artificial. ;)

I use `auto` as  a single function syntax. With an exception for void functions. In addition to being a single unified syntax, which IMHO is an advantage in and of itself, this supports rapid scanning of code since function name and return type appears in fixed places, in particular regardless of length of complex template stuff.

That's the job of typedefs IMO.

But let's ignore that part.

Overall, the idea is interesting, but I don't think it's something for the base framework. Plus I'm not sure how you'd use that kind of code having multiple things to show or move etc.

I understand the general idea or concept behind this (display things with as little syntax as possible), but at the same time your API seems rather restricted and cumbersome to use (at least for me).

How would you write some sample program. Let's say a Pong clone (i.e. two paddles and a ball)?
« Last Edit: March 06, 2014, 11:04:07 am by Mario »

Groogy

  • Hero Member
  • *****
  • Posts: 1469
    • MSN Messenger - groogy@groogy.se
    • View Profile
    • http://www.groogy.se
    • Email
Re: Simplifying SFML
« Reply #5 on: March 06, 2014, 10:54:46 am »
As much as I tried, but I find your code extremely hard to read. :|

Yeah I have to agree, the code was really hard to follow, not from its complexity but from the coding standard you have.

I think the problem you reached with several windows is that each window is its own context.
how do you mean though one at a time? Did you try to render to each window in its own separate thread?
Because correct me if wrong but I think that isn't supported until OpenGL 4.x and DirectX 11

Also to me it just looks like you've made a MVC-Architecture with a lot of restrictions to it. Having something like that in the base of SFML would just restrict people in how they do something. Since SFML isn't an engine I don't see a need for that.
« Last Edit: March 06, 2014, 10:58:19 am by Groogy »
Developer and Maker of rbSFML and Programmer at Paradox Development Studio

Alf P. Steinbach

  • Newbie
  • *
  • Posts: 5
    • View Profile
    • Email
Re: AW: Simplifying SFML
« Reply #6 on: March 06, 2014, 02:27:39 pm »
As for the code, I find the "auto func() -> type" a bit misused, e.g. the main function just returns int, there's no way it would change, thus the auto keyword is very artificial. ;)

I use `auto` as  a single function syntax. With an exception for void functions. In addition to being a single unified syntax, which IMHO is an advantage in and of itself, this supports rapid scanning of code since function name and return type appears in fixed places, in particular regardless of length of complex template stuff.

That's the job of typedefs IMO.

But let's ignore that part.

Overall, the idea is interesting, but I don't think it's something for the base framework. Plus I'm not sure how you'd use that kind of code having multiple things to show or move etc.

I understand the general idea or concept behind this (display things with as little syntax as possible), but at the same time your API seems rather restricted and cumbersome to use (at least for me).

How would you write some sample program. Let's say a Pong clone (i.e. two paddles and a ball)?

The following just shows how to animate 3 objects, a ball and two paddles, i.e. no game logic. Disregarding the collision detection (here only walls are checked) I think it's pretty simple, yes?

#include <xsf/all.h>        // xsf::*, sf::*

#include <array>
using std::array;

class Pong_game
    : public xsf::Model
{
using Circle = xsf::shape::Circle;
template< class Item, size_t > friend class array;

private:
    enum{ width = 200, height = 100, middle_x = width/2 };

    struct Ball
    {
        Circle              body_       { Circle::Params()
            .radius( 10.0 )
            .fillcolor( sf::Color::Green )
            };
        xsf::Size<>         velocity_   { 30*3.14159f, 30*2.71828f };
    };

    struct Paddle
    {
        sf::RectangleShape  body_       { xsf::Size<>{5, 20} };
        float               speed_y     { 27.3f };

        Paddle( xsf::Point<> const position )
        {
            body_.setPosition( position );
            body_.setFillColor( sf::Color::Red );
        }
    };

    Ball                ball_;
    array<Paddle, 2>    paddle_;

    void on_update( sf::Time const time_now, sf::Time const dt ) override
    {
        XSF_INTENTIONALLY_UNUSED( time_now );

        float const seconds = dt.asSeconds();
        ball_.body_.move( seconds*ball_.velocity_ );

        sf::Vector2f const position = ball_.body_.getPosition();
        if( position.x < 0 && ball_.velocity_.x < 0 )       { ball_.velocity_.x *= -1; }
        if( position.x >= width && ball_.velocity_.x > 0 )  { ball_.velocity_.x *= -1; }
        if( position.y < 0 && ball_.velocity_.y < 0 )       { ball_.velocity_.y *= -1; }
        if( position.y >= height && ball_.velocity_.y > 0 ) { ball_.velocity_.y *= -1; }

        for( int i = 0;  i < 2;  ++i )
        {
            float const v = paddle_[i].speed_y;
            paddle_[i].body_.move( {0, seconds*v} );

            auto const position = paddle_[i].body_.getPosition();
            if( position.y < 0 && v < 0 )       { paddle_[i].speed_y *= -1; }
            if( position.y >= height && v > 0 ) { paddle_[i].speed_y *= -1; }
        }
    }

public:
    Pong_game()
        : paddle_( { Paddle({10, 10}), Paddle({ width - 10, height - 10 }) } )
    {
        drawables_.insert( {&ball_.body_, &paddle_[0].body_, &paddle_[1].body_} );
    }
};

auto main()
    -> int
{
    Pong_game model;
    xsf::Window window( "SFML works, yay!", &model );
    xsf::Controller( &model ).process_events();
}
 


Oder?

Since I just started there are lots of things not implemented, most of what's needed for Real Usage(TM), but in particular I noticed now -- I hadn't thought of it -- a need to address what's drawn on top of what, i.e. what's drawn first and last. With current code that's pretty arbitrary, selected by the order of pointers in the set of drawables. How do people go about addressing that, general z-order?

Cheers,

- Alf

Alf P. Steinbach

  • Newbie
  • *
  • Posts: 5
    • View Profile
    • Email
Re: Simplifying SFML
« Reply #7 on: March 06, 2014, 02:59:23 pm »
As much as I tried, but I find your code extremely hard to read. :|

Yeah I have to agree, the code was really hard to follow, not from its complexity but from the coding standard you have.

I think the problem you reached with several windows is that each window is its own context.
how do you mean though one at a time? Did you try to render to each window in its own separate thread?
Because correct me if wrong but I think that isn't supported until OpenGL 4.x and DirectX 11

Also to me it just looks like you've made a MVC-Architecture with a lot of restrictions to it. Having something like that in the base of SFML would just restrict people in how they do something. Since SFML isn't an engine I don't see a need for that.

Yes it's MVC, or rather MWC :-) It's not about imposing restrictions though, but about providing defaults, such as default antialiasing, default smooth circles, default event processing, etc. There are some language limitations that I don't think can be reasonably worked around, such as for named arguments for initialization (Boost parameters is a hassle, and my own earlier C++03 options classes too complex, I don't have time to rework that for C++11 now), but at least "boilerplate code", the code that's almost the same in every intro example, is I think default-able in reasonable ways.

Re "in the base of SFML" no I don't think that's a good idea. This is a thin layer of code that sits on top of SFML. It exposes SFML for direct use much as some GUI frameworks expose the underlying API.

Cheers, & thanks for your reactions,

- Alf

Mario

  • SFML Team
  • Hero Member
  • *****
  • Posts: 879
    • View Profile
Re: Simplifying SFML
« Reply #8 on: March 06, 2014, 09:17:52 pm »
Ah, now I got it - guess I misunderstood your initial proposal.

So you wanted to abstract away window creation, event/update handling, and drawing using MVC?

Think this is a bit too complex for something that's meant as a simple interface, i.e. I'd rather use some easy to use base class where you can provide an update and/or drawing function, especially considering this is probably the part most (game programming) newbies fail (e.g. timesteps) and you won't need a separated view, you could replace (or you could just switch the whole base class to move to another system).

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: Simplifying SFML
« Reply #9 on: March 07, 2014, 10:27:33 am »
The idea of MVC is nice, there have also been several game engines for SFML that use a similar approach. But I always wonder how scalable and generic these are, because most are tailored for a specific game and have specific requirements.

Also, ideally the model wouldn't have any SFML-related data or functionality at all (except maybe time and vectors), which doesn't seem to be the case for your pong example. Actually, I see no separation of graphics and logics at all. What I would consider separate is, for example:
  • A class for logics and one for graphics. The logics class holds an opaque pointer to the graphics class, and a renderer reads it
  • A class for logics and one for graphics, but they don't reference each other directly, but through a map with an index or identifier
  • A logic class and a renderer class that has draw() overloads for different logic objects, and constructs the graphics on-the-fly

I agree that the code is very hard to read. That has to do with the unusual placement of newlines and whitespaces, as well as the use of under_lines convention, which makes it more difficult to recognize identifiers as a whole (especially in the presence of operators).

Example:
inline
auto sign( sf::Time const& time )
    -> int
{ return (time < sf::Time()? -1 : time == sf::Time()? 0 : +1); }
 
vs.
inline int sign(sf::Time time)
{
    return time < sf::Time::Zero ? -1
         : time == sf::Time::Zero ? 0
         : +1;
}
(or use an if or the non-conditional sign trick).

Furthermore, you're very inconsistent with the placement of curly braces; sometimes everything is on one line, sometimes you distribute it across multiple lines with braces on their own line, sometimes they're in the middle of a multi-line block...

Other than that, the code looks quite good, maybe some minor remarks:
  • Don't use top-level const qualifiers for parameters. In char const* const title, the second const is useless, as it only says that the copy of the pointer (local to the function) may not be changed. Same for sf::Time const time_now, don't overuse const where it doesn't contribute to semantics.
  • Use static_cast instead of C casts
  • Instead of giving classes like Point a default template parameter for float, I'd rather do it like SFML and provide typedefs for int and float. Something like Point<> that is used so often seems weird.
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

Alf P. Steinbach

  • Newbie
  • *
  • Posts: 5
    • View Profile
    • Email
Re: Simplifying SFML
« Reply #10 on: March 07, 2014, 05:44:44 pm »
The idea of MVC is nice, there have also been several game engines for SFML that use a similar approach. But I always wonder how scalable and generic these are, because most are tailored for a specific game and have specific requirements.

Also, ideally the model wouldn't have any SFML-related data or functionality at all (except maybe time and vectors), which doesn't seem to be the case for your pong example. Actually, I see no separation of graphics and logics at all. What I would consider separate is, for example:
  • A class for logics and one for graphics. The logics class holds an opaque pointer to the graphics class, and a renderer reads it
  • A class for logics and one for graphics, but they don't reference each other directly, but through a map with an index or identifier
  • A logic class and a renderer class that has draw() overloads for different logic objects, and constructs the graphics on-the-fly

This suggestion of wrapping SFML, of abstracting away its coupling of "logic" and "presentation", and just assuming that I'm able to do such a thing, is quite a compliment. Thanks!  :)

However, while I've done similar small frameworks before, I don't have the time to go into that, sorry. Also I think it would be a good idea with some experience in games programming before trying to address the problems of that area, or even guessing what the problems are! And while I know a lot about general GUI programming, C++ and software development in general, I know next to zilch about games programming.  :(

I agree that the code is very hard to read. That has to do with the unusual placement of newlines and whitespaces, as well as the use of under_lines convention, which makes it more difficult to recognize identifiers as a whole (especially in the presence of operators).

Example:
inline
auto sign( sf::Time const& time )
    -> int
{ return (time < sf::Time()? -1 : time == sf::Time()? 0 : +1); }
 
vs.
inline int sign(sf::Time time)
{
    return time < sf::Time::Zero ? -1
         : time == sf::Time::Zero ? 0
         : +1;
}
(or use an if or the non-conditional sign trick).

Furthermore, you're very inconsistent with the placement of curly braces; sometimes everything is on one line, sometimes you distribute it across multiple lines with braces on their own line [...]

As it happens the spacing and naming convention I use is mostly the same as in the standard library and Boost library code, modulo my use of auto function syntax. And there is no alternative to auto function syntax for lambdas and for some template functions, so one can't avoid having to deal with it. So if any of this feels unreadable or uncomfortable, then I think it can be a good idea to force oneself to get used to it, so as to be able to deal effectively with "standard" library code and modern code using lambdas etc.

But I do now tend to write a one-liner body on a single line, yes.

As I see it that's not "inconsistent". On the contrary, running the code through the free AStyle code formatter with option style=--allman, which generally ensures consistency, retained all the one-liner function bodies. AStyle did however replace one one-liner statement if( n_open_windows == 0 ) { break; } with four lines and indenting, which to my eyes reduce the readability of the function as a whole -- but I guess it depends on what one's used to. ;-)

Anyway, I recommend AStyle or some similar formatting tool to deal with formatting that one finds uncomfortable, or that is simply inconsistent or lacking. Nowadays even Visual Studio has code formatting. And both AStyle and Visual Studio's formatting can be configured to your liking via a host of options.


, sometimes [the curly braces are] in the middle of a multi-line block...

I suspect that here you're misinterpreting C++11 curly braces initializers.


Other than that, the code looks quite good, maybe some minor remarks:
  • Don't use top-level const qualifiers for parameters. In char const* const title, the second const is useless, as it only says that the copy of the pointer (local to the function) may not be changed. Same for sf::Time const time_now, don't overuse const where it doesn't contribute to semantics.
  • Use static_cast instead of C casts
  • Instead of giving classes like Point a default template parameter for float, I'd rather do it like SFML and provide typedefs for int and float. Something like Point<> that is used so often seems weird.

• Regarding const,
the same rationale that applies for using const on local variables, applies to using it at top level for formal arguments. Namely, it reduces the number of things that can vary and that one has to keep track of for understanding the code. Note that for ordinary application programming this is the sole purpose of const.

I fail to understand what you mean by "doesn't contribute to semantics", sorry.

Maybe you meant "doesn't contribute to the function's formal type" (signature plus return type), which is technically true, but which I then don't understand the relevance of?

• Regarding use of static_cast,
you're right that C style casts set a bad example, even when they're "safe". I should not have used just (float) to suppress sillywarnings, mea culpa! However, static_cast gets a bit too verbose for my taste in such cases, so (when thinking about it) I would rather choose to e.g. multiply by 1.0f or add 0.0f just to force a conversion.

• Regarding default parameter for templates,
I did consider a typedef like Point_f. However, the syntax Point<> very clearly says that this is the default, the natural choice for the library, while Point_f is no different from e.g. Point_i. Besides, I'm adverse to type-prefixes and suffixes... ;-)
 
Cheers, & thanks for you feedback!,

- Alf

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
Re: Simplifying SFML
« Reply #11 on: March 07, 2014, 06:23:27 pm »
Quote
As it happens the spacing and naming convention I use is mostly the same as in the standard library and Boost library code
Yes, and both standard library and Boost implementations (not necessarily usage) are often totally unreadable.

Quote
And there is no alternative to auto function syntax for lambdas and for some template functions, so one can't avoid having to deal with it.
True, but that's no reason to use it everywhere. When consistency is an argument, you should probably start with curly braces :P

Quote
I suspect that here you're misinterpreting C++11 curly braces initializers.
No, I meant the following inconsistency:
// {} on separate lines, multiple statements on separate lines
inline
    auto antialiased()
        -> sf::ContextSettings
    {
        sf::ContextSettings settings;

        settings.antialiasingLevel = 8;
        return settings;
    }

// {} on same line, multiple statements in one line
                auto radius( float const value )
                    -> Params&
                { radius_ = value; return *this; }

// both {} and statement on same line
if( position.y > 100 && velocity_.y > 0 )   { velocity_.y *= -1; }
 

Quote
the same rationale that applies for using const on local variables, applies to using it at top level for formal arguments. Namely, it reduces the number of things that can vary and that one has to keep track of for understanding the code. Note that for ordinary application programming this is the sole purpose of const.
That is true when you do it in the function definition, but when it's in the declaration and thus part of the interface (since people or tools like Doxygen inspect headers), you're cluttering the API with information that is irrelevant to the user. The caller of a function doesn't bother whether a copy is changed by the implementation. Don't simply use const because you can; keep it for the important cases (namely pointer-to-const or reference-to-const parameters).

Quote
However, static_cast gets a bit too verbose for my taste in such cases
That's true. Here, casts are necessary, but in general too many static_casts are a good indication that one is casting too often, and that the conversion might be worth encapsulating.

Quote
I did consider a typedef like Point_f. However, the syntax Point<> very clearly says that this is the default, the natural choice for the library, while Point_f is no different from e.g. Point_i. Besides, I'm adverse to type-prefixes and suffixes... ;-)
Another user might not know what's the default... and even for default values I find it strange to type always <> ;)
If you don't like suffixes, you can also use typedefs FloatPoint and IntPoint or simply Point and IntPoint (when the former is the default).

Keep in mind that it was just my impression from reading your code, and apparently there are some people who have problems with its readability. But you don't have to change your code style because of us :)
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development: