Jump to content
  • Advertisement
Sign in to follow this  
Code_Black

Code review

This topic is 546 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Yes I was hoping I could get some feed back on my code. I know everythings in main, but when I try to add custom headers I get all kinds of errors. Thanks.
 

#include <iostream>
#include <SFML/Graphics.hpp>
#include<cmath>
//#include"block.h"
//sf::RenderWindow win(sf::VideoMode(800,600),"RPG");
using namespace std;
enum class State{PAUSE,PLAYING};

class Block{
    sf::Texture tex;
    sf::Sprite spr;
    sf::Vector2f pos;

public:
    int pos_i;
    Block(float x=0.f,float y=0.f,float hi=0.f,float wd=0.f){
        sf::Vector2f size(hi,wd);
        tex.loadFromFile("1.png");
        pos.x=x; pos.y=y;
        spr.setTexture(tex);
        spr.setTextureRect(sf::IntRect(0,0,size.x,size.y));
        spr.setPosition(sf::Vector2f(x,y));
    }

    void draw(sf::RenderWindow* win){win->draw(spr);}
    void set_size(sf::Vector2f size){spr.setTextureRect(sf::IntRect(0,0,size.x,size.y));}
    sf::Vector2f get_pos(){return pos;}
    sf::Sprite& get_tex(){return spr;}
    void set_pos(sf::Vector2f pos){spr.move(pos);}
};

class World{
    int wd;
    int hi;
    int level[6][6];

public:
    std::vector<Block> block1;
    int count;
    //void set_pos(sf::Vector2f pos,int i,int j){block[i*j].set_pos(pos);}

    World():level{
        {1,0,0,0,0,0},
        {1,0,0,0,0,0},
        {1,0,0,0,0,0},
        {1,0,0,0,0,0},
        {1,0,0,0,0,0},
        {1,1,1,1,1,1}}
    {
        wd=6;
        hi=6;
        //blocks=new Block[wd*hi];

        for(int i=0;i<hi;i++){
            for(int j=0;j<wd;j++){
                if(level[i][j]==1){
                    Block blk(j*24,i*24,24,24);
                    block1.push_back(blk);
                    sf::Vector2f pos=blk.get_pos();
                    cout<<" x "<<j<<" y "<<i<<'\n';
                    cout<<" pos x "<<pos.x<<" pos y "<<pos.y<<'\n';
                }
            }
        }
        //block[1].set_pos(sf::Vector2f(1.f*16,4.f*16));
    }

    void draw(sf::RenderWindow* win){
        for(int i=0;i<block1.size();i++){
            block1[i].draw(win);
        }
    }
};

class weapon{
    Block blc;
    bool m_up;
    bool m_dn;
    bool m_lt;
    bool m_rt;
    sf::Vector2f m_res;

public:
    weapon():blc(0,0,8,8){}
    void move_rt(){m_rt=true;}
    void move_lt(){m_lt=true;}
    void move_up(){m_up=true;}
    void move_dn(){m_dn=true;}

    void stop_rt(){m_rt=false;}
    void stop_lt(){m_lt=false;}
    void stop_up(){m_up=false;}
    void stop_dn(){m_dn=false;}
    void setPos(sf::Vector2f pos){blc.set_pos(pos);}
    sf::Sprite getWep(){return blc.get_tex();}
    void update( sf::Vector2f pos ,sf::Vector2i mse_pos){
        if(m_up)
            blc.set_pos(pos - sf::Vector2f(0.f,10.f));
        if(m_dn)
            blc.set_pos(pos + sf::Vector2f(0.f,10.f));
        if(m_rt)
            blc.set_pos(pos - sf::Vector2f(10.f,0.f));
        if(m_lt)
            blc.set_pos(pos + sf::Vector2f(10.f,0.f));

        float angle=(atan2(mse_pos.y-m_res.y/2,mse_pos.x-m_res.x/2))*180/3.14f;

        blc.get_tex().setRotation(angle);
    }
};

class Player{
    Block blc;
    weapon Weapon;
    sf::Vector2f m_res;

    bool m_up;
    bool m_dn;
    bool m_lt;
    bool m_rt;

    float m_speed;
    sf::Time last_hit;

public:
    Player():blc(0,0,16,16){}

    void spawn(sf::Vector2f res,sf::Vector2f loc){
        blc.set_pos(loc);
        Weapon.setPos(sf::Vector2f(loc.x+5.0f,loc.y+5.0));
        m_res.x=res.x;
        m_res.y=res.y;
    }
    weapon& getWep(){return Weapon;}
    bool hit(sf::Time time_hit);
    sf::FloatRect get_pos(){return blc.get_tex().getGlobalBounds();}
    sf::Vector2f get_cen(){return blc.get_pos();}
    float get_rot(){return blc.get_tex().getRotation();}
    sf::Sprite get_pla(){return blc.get_tex();}

    void move_rt(){m_rt=true;}
    void move_lt(){m_lt=true;}
    void move_up(){m_up=true;}
    void move_dn(){m_dn=true;}

    void stop_rt(){m_rt=false;}
    void stop_lt(){m_lt=false;}
    void stop_up(){m_up=false;}
    void stop_dn(){m_dn=false;}

    void update( sf::Vector2f pos ,sf::Vector2i mse_pos){
        if(m_up)
            blc.set_pos(pos - sf::Vector2f(0.f,10.f));
        if(m_dn)
            blc.set_pos(pos + sf::Vector2f(0.f,10.f));
        if(m_rt)
            blc.set_pos(pos - sf::Vector2f(10.f,0.f));
        if(m_lt)
            blc.set_pos(pos + sf::Vector2f(10.f,0.f));

        float angle=(atan2(mse_pos.y-m_res.y/2,mse_pos.x-m_res.x/2))*180/3.14f;

        blc.get_tex().setRotation(angle);
    }


};


int main()
{
    //State s=State::PAUSE;
    sf::Vector2f res;
    res.x=sf::VideoMode::getDesktopMode().width;
    res.y=sf::VideoMode::getDesktopMode().height;
    sf::RenderWindow win(sf::VideoMode(res.x,res.y),"RPG");
    sf::RenderWindow win1(sf::VideoMode(res.x,res.y),"RPG");
    sf::View mainView;
    mainView.reset(sf::FloatRect(0,0,res.x,res.y));
    mainView.setViewport(sf::FloatRect(0,0,1.0f,1.0f));
    sf::Clock clock;
    sf::Time gameTotal;
    sf::Vector2f mouseWPos;
    sf::Vector2i mouseSPos;

    World w;
    Player pla;
    weapon wep;
    pla.spawn(res,sf::Vector2f(2.f,2.f));
    while(win.isOpen()){
        sf::Event eve;
        while(win.pollEvent(eve)){
            if(true){
                if(sf::Keyboard::isKeyPressed(sf::Keyboard::W)){
                    pla.move_up();
                    pla.getWep().move_up();
                }
                else{
                    pla.stop_up();
                    pla.getWep().stop_up();
                }
                if(sf::Keyboard::isKeyPressed(sf::Keyboard::S)){
                    pla.move_dn();
                    pla.getWep().move_dn();
                }
                else{
                    pla.stop_dn();
                    pla.getWep().stop_dn();
                }
                if(sf::Keyboard::isKeyPressed(sf::Keyboard::A)){
                    pla.move_lt();
                    pla.getWep().move_lt();
                }
                else{
                    pla.stop_lt();
                    pla.getWep().stop_lt();
                }
                if(sf::Keyboard::isKeyPressed(sf::Keyboard:)){
                    pla.move_rt();
                    pla.getWep().move_rt();
                }
                else{
                    pla.stop_rt();
                    pla.getWep().stop_rt();
                }
            }
            sf::Vector2f plaPos(pla.get_cen());
            pla.update(plaPos,sf::Mouse::getPosition());
            pla.getWep().update(plaPos,sf::Mouse::getPosition());
        }

        clock.restart();

        win.clear(sf::Color::Black);
        win.draw(pla.getWep().getWep());
        win.draw(pla.get_pla());
        w.draw(&win);
        win.setView(mainView);

        win.display();
    }
    //cout << "Hello world!" << endl;
    return 0;
}
Edited by frob
Add code tags, adjust formatting.

Share this post


Link to post
Share on other sites
Advertisement
Can't really read it in unformatted form, but:

- You need longer and more descriptive variable names.
- You shouldn't abbreviate so much stuff.
- You have inconsistent capitalization conventions (class weapon).
- All of your up/down/left/right stuff could probably be enum and array-based instead.
- move_* and stop_* should probably be changed to set(direction, bool).
- All of your numeric constants should be turned into 'const' definitions somewhere. Edited by Nypyren

Share this post


Link to post
Share on other sites

but when I try to add custom headers I get all kinds of errors.

Please include the exact error message.

The error is probably something inside your header file.  That's plain code as well, and any parse errors in there will show up wherever the file is included.

Share this post


Link to post
Share on other sites
Nypyren mentioned it already, but let me emphasize it: The abbreviations make the code unreadable. It took me a while to figure that `res' stands for "resolution".

Using 3.14f as the value of pi hurts my soul. It's unfortunate that SFML wants you to specify angles, but it's not your fault.

Do you understand how textures and sprites are supposed to be used in SFML? Each Block object should not have its own sf::Texture; it should use an sf::Texture that lives somewhere else.

    sf::Sprite& get_tex(){return spr;}

That's just weird. Why would "get_tex" return a sprite and not a texture? This is further evidence that perhaps you didn't understand how textures and sprites are supposed to be used. Textures contain an image from which sprites are cut. They are fairly heavy objects and you shouldn't create many of them. Sprites on the other hand are just a handful of numbers that describe what part of the texture should be drawn. These are light objects and you can feel free to create many of them, copy them around, etc.

get_pla returns a sprite; get_pos returns a vector if called on a block (this is fine), but it returns a rectangle if called on a weapon (???); getWep returns a sprite if called on a weapon, but it returns a weapon if called on a player? You are just messing with us. :)

Instead of
    m_res.x=res.x;
    m_res.y=res.y;
just use
    m_res = res;

What is the clock for?


  //cout << "Hello world!" << endl;


I understand you are sentimental about the origins of your program, but you should just let go of this one.

In general, comments should be used to explain some design decision, or to clarify some difficult part of the code (this is in a way an acceptance of defeat), or to document the interface to a part of your code. The only comments you have are old lines of code that don't help the reader at all, and should go.

Share this post


Link to post
Share on other sites
Thank you. Like I said this is my first real try at something showing what I've learned so far. I know it's messy but I'm trying to get things to work. So any advice would be greatful.

Share this post


Link to post
Share on other sites

@frob when I #include "block"


That does not address what frob said. He said to "include the exact error message". Error messages might seem like mystical nonsense to you, but they are important.

Share this post


Link to post
Share on other sites
Ok here are the errors please help

||=== Build: Debug in cat (compiler: GNU GCC Compiler) ===|
obj\Debug\main.o||In function `ZN5WorldC1Ev':|
C:\Users\chris\Documents\cat\main.cpp|69|undefined reference to `Block::Block(float, float, float, float)'|
C:\Users\chris\Documents\cat\main.cpp|71|undefined reference to `Block::get_pos()'|
obj\Debug\main.o||In function `ZN5World4drawEPN2sf12RenderWindowE':|
C:\Users\chris\Documents\cat\main.cpp|87|undefined reference to `Block::draw(sf::RenderWindow*)'|
obj\Debug\main.o||In function `ZN6weaponC1Ev':|
C:\Users\chris\Documents\cat\main.cpp|102|undefined reference to `Block::Block(float, float, float, float)'|
obj\Debug\main.o||In function `ZN6weapon6setPosEN2sf7Vector2IfEE':|
C:\Users\chris\Documents\cat\main.cpp|112|undefined reference to `Block::set_pos(sf::Vector2<float>)'|
obj\Debug\main.o||In function `ZN6weapon6getWepEv':|
C:\Users\chris\Documents\cat\main.cpp|113|undefined reference to `Block::get_tex()'|
obj\Debug\main.o||In function `ZN6weapon6updateEN2sf7Vector2IfEENS1_IiEE':|
C:\Users\chris\Documents\cat\main.cpp|117|undefined reference to `Block::set_pos(sf::Vector2<float>)'|
C:\Users\chris\Documents\cat\main.cpp|119|undefined reference to `Block::set_pos(sf::Vector2<float>)'|
C:\Users\chris\Documents\cat\main.cpp|121|undefined reference to `Block::set_pos(sf::Vector2<float>)'|
C:\Users\chris\Documents\cat\main.cpp|123|undefined reference to `Block::set_pos(sf::Vector2<float>)'|
C:\Users\chris\Documents\cat\main.cpp|129|undefined reference to `Block::get_tex()'|
obj\Debug\main.o||In function `ZN6PlayerC1Ev':|
C:\Users\chris\Documents\cat\main.cpp|151|undefined reference to `Block::Block(float, float, float, float)'|
obj\Debug\main.o||In function `ZN6Player5spawnEN2sf7Vector2IfEES2_':|
C:\Users\chris\Documents\cat\main.cpp|156|undefined reference to `Block::set_pos(sf::Vector2<float>)'|
obj\Debug\main.o||In function `ZN6Player7get_cenEv':|
C:\Users\chris\Documents\cat\main.cpp|167|undefined reference to `Block::get_pos()'|
obj\Debug\main.o||In function `ZN6Player7get_plaEv':|
C:\Users\chris\Documents\cat\main.cpp|169|undefined reference to `Block::get_tex()'|
obj\Debug\main.o||In function `ZN6Player6updateEN2sf7Vector2IfEENS1_IiEE':|
C:\Users\chris\Documents\cat\main.cpp|185|undefined reference to `Block::set_pos(sf::Vector2<float>)'|
C:\Users\chris\Documents\cat\main.cpp|187|undefined reference to `Block::set_pos(sf::Vector2<float>)'|
C:\Users\chris\Documents\cat\main.cpp|189|undefined reference to `Block::set_pos(sf::Vector2<float>)'|
C:\Users\chris\Documents\cat\main.cpp|192|undefined reference to `Block::set_pos(sf::Vector2<float>)'|
C:\Users\chris\Documents\cat\main.cpp|198|undefined reference to `Block::get_tex()'|
||=== Build failed: 20 error(s), 0 warning(s) (0 minute(s), 1 second(s)) ===|

Share this post


Link to post
Share on other sites
Those look like linker errors. You have to make sure to list all the .cpp files in the compilation command.

If you still have trouble, please post the exact compilation command you are using. Edited by Álvaro

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!