I need a code review.

[magneonx]

Hello! Can I ask your help? Actually this code works. I have a goal in making this smooth though. Also I wanted some kind of not-so-harsh criticism regarding this, I am starting game programming and I think I need this kind of common game feature. What this does is to pan a map just like in RTS and also to zoom into the map.

I need some suggestions or recommendations for this one.

If I could get the green sign I will be soon draw the layout for this and implementing this mechanic soon.

THank you very much!!!

#include<iostream>
using std::cout;
using std::endl;
using std::cerr;

#include<string>
using std::string;

#include<SFML/Graphics.hpp>
#include<SFML/Window.hpp>


int main()
{
    const int WIDTH  = 1028;
    const int HEIGHT = 768;
    sf::RenderWindow window( sf::VideoMode( WIDTH , HEIGHT , 32 ) , "SFML Window" );

    sf::Texture bg;

    bool isBgLoaded = bg.loadFromFile("background.jpg");

    if( !isBgLoaded )
        exit( 1 );

    sf::Sprite background( bg );

    sf::Vector2f center  ( WIDTH / 2 , HEIGHT / 2 );
    sf::Vector2f halfsize( 800 , 800 );

    sf::View view( center , halfsize );

    float pan_speed = 0.8f;
    const int MAX_SCROLL = 27;
    int scroll_count = 0;

    int actual_width  = view.getSize().x;
    int actual_height = view.getSize().y;
    const int resize_rate   = 20;

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

        int x = sf::Mouse::getPosition( window ).x;
        int y = sf::Mouse::getPosition( window ).y;

        x = ( x < 0 ) ? 0 : x;
        y = ( y < 0 ) ? 0 : y;

        x = ( x > WIDTH  ) ? WIDTH  : x;
        y = ( y > HEIGHT ) ? HEIGHT : y;

        //cout << "( " << x << " , " << y << " ) " << endl;

        if( view.getViewport().top >= 0 and view.getViewport().left < HEIGHT )
        {
            if( x <= 50  )
            {
                view.move( -pan_speed , 0 );
            }

            if( y <= 50 )
            {
                view.move( 0 , -pan_speed );
            }

            if( x >= WIDTH-50 and x <= WIDTH )
            {
                view.move( pan_speed , 0 );
            }

            if( y >= HEIGHT-50 and y <= HEIGHT )
            {
                view.move( 0 , pan_speed );
            }
        }

        while( window.pollEvent( event ) )
        {
            if( event.type == sf::Event::Closed )
            {
                window.close();
            }
            else if( event.type == sf::Event::KeyPressed )
            {
                if( event.key.code == sf::Keyboard::Key::Escape )
                {
                    window.close();
                }
            }

            if( event.type == sf::Event::MouseWheelMoved )
            {
                if( event.mouseWheel.delta > 0 and scroll_count <= MAX_SCROLL )
                {
                    cout << "Going up!!!" << endl;
                    scroll_count++;
                    //view.zoom( 0.7 );

                    actual_height -= resize_rate;
                    actual_width  -= resize_rate;


                    view.setSize( actual_width , actual_height );
                }

                if( event.mouseWheel.delta < 0 and scroll_count > 0 )
                {
                    cout << "Going down!!!" << endl;
                    scroll_count--;
                    //view.zoom( 1.9 );

                    actual_height += resize_rate;
                    actual_width  += resize_rate;

                    view.setSize( actual_width , actual_height );
                }

                cout << "Scroll Count : " << scroll_count << endl;
                cout << "View size : ( " << view.getSize().x << " x " << view.getSize().y << " ) " << endl;

            }
        }

        /***/

        window.clear( sf::Color( 0 , 0 , 0 ) );
        window.draw( background );

        window.setView( window.getDefaultView() );
        /** Draw some interface here! */

        window.display();

    }

    return 0;
}

 

[Nexus]

No unnecessary variables:

bool isBgLoaded = bg.loadFromFile("background.jpg");
if( !isBgLoaded )
// ->
if (!bg.loadFromFile("background.jpg"))

Never use std::exit()! It performs no cleanup, destructors are not invoked.

exit( 1 );
// ->
return 1;

Use standard library functions from <algorithm>:

x = ( x < 0 ) ? 0 : x
// ->
x = std::max(x, 0);

Do you really use alternative keywords? Just be aware that Visual Studio doesn't support them, so your code is non-portable.

and
// ->
&&

Make variables that don't change constant. Or no local variables at all, but be consistent.

float pan_speed = 0.8f;
// ->
const float pan_speed = 0.8f;

Get rid of unused include directives.

#include<string>
// ->
 

Be consistent with your code style. Some constants are ALL_CAPS, some standard_style. Personally, I would use ALL_CAPS only for macros.
 
Use functions to outsource functionality. Your main() is way too big.

Declare variables as late and local as possible. The sf::Event event; declaration remains unused for many lines.

[magneonx]

@Nexus

Wow thanks for the review!!! Although I am aware with some coding rules there and I intentionally put everything procedurally, I am also keen on the algorithms I used in making the screen span??? I am using the SFML functions correctly? I am also looking for better ways, but it seems I don't get any reviews there so I think what I have made so far is correct?

I know there is something wrong in here
        int x = sf::Mouse::getPosition( window ).x;
        int y = sf::Mouse::getPosition( window ).y;

Because everytime I move my mouse outside of the screen my game screen still span. I know I missed some good points here. I don't want that behavior. I am using SFML 2.0 anyway.

Also is this part of the code correct?? I think I misinterpreted on how to use, view's zoom and size.

               
 if( event.mouseWheel.delta > 0 and scroll_count <= MAX_SCROLL )
                {
                    cout << "Going up!!!" << endl;
                    scroll_count++;
                    //view.zoom( 0.7 );

                    actual_height -= resize_rate;
                    actual_width  -= resize_rate;


                    view.setSize( actual_width , actual_height );
                }

                if( event.mouseWheel.delta < 0 and scroll_count > 0 )
                {
                    cout << "Going down!!!" << endl;
                    scroll_count--;
                    //view.zoom( 1.9 );

                    actual_height += resize_rate;
                    actual_width  += resize_rate;

                    view.setSize( actual_width , actual_height );
                }
 

Let me know if I am doing things right. Thank you very much for helping me understand!