Sign in to follow this  
Followers 0
Code_Black

Code review

12 posts in this topic

Posted (edited)

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.
0

Share this post


Link to post
Share on other sites

Posted (edited)

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
-1

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.

1

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.
2

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.
0

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.
0

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)) ===|
0

Share this post


Link to post
Share on other sites

Posted (edited)

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
0

Share this post


Link to post
Share on other sites

obj\Debug\main.o||In function `ZN5WorldC1Ev':| C:\Users\chris\Documents\cat\main.cpp|69|undefined reference to `Block::Block(float, float, float, float)'|

Thos look to me like you're not linking in a .OBJ file built from your Block code. I'm not sure what environment you;re in (GCC on Windows, so it's not Visual Studio) but I suspect somewhere you need to add "block.cpp" to a project or something.

0

Share this post


Link to post
Share on other sites
All I did was install cb and started new project and tried to add my own class and errors but when I all the code in main problem sloved. I know it wrong to code in one file so help. Thanks

obj\Debug\main.o||In function `ZN5WorldC1Ev':| C:\Users\chris\Documents\cat\main.cpp|69|undefined reference to `Block::Block(float, float, float, float)'|

Thos look to me like you're not linking in a .OBJ file built from your Block code. I'm not sure what environment you;re in (GCC on Windows, so it's not Visual Studio) but I suspect somewhere you need to add "block.cpp" to a project or something.

obj\Debug\main.o||In function `ZN5WorldC1Ev':| C:\Users\chris\Documents\cat\main.cpp|69|undefined reference to `Block::Block(float, float, float, float)'|

Thos look to me like you're not linking in a .OBJ file built from your Block code. I'm not sure what environment you;re in (GCC on Windows, so it's not Visual Studio) but I suspect somewhere you need to add "block.cpp" to a project or something.
Yes I'm using codeblocks on windows 10 32bit,2gb,1.83g atom
0

Share this post


Link to post
Share on other sites
Learn one thing at a time. If you want to learn how to have multi-file projects, create a new project. Make two .cpp files and one header file.

main.cpp:
#include "print_it.h"

int main() {
  print_it();
}
print_it.h:
#pragma once

void print_it();
print_it.cpp:
#include "print_it.h"

void print_it() {
  std::cout << "Hello, world!\n";
}

Figure out how to make that work in your environment. You'll have to add both .cpp files to your project, or something like that. Use Google to see how this is done in your particular environment.

Try compiling from the command line. Something like this should work:
> g++ -std=c++11 main.cpp print_it.cpp -o main.exe -Wall -Wextra -O3
0

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now
Sign in to follow this  
Followers 0