Игра СФМЛ в одном шприце в подземелье


Я делаю игру на C++ и сфмл впервые и я хотел бы, чтобы кто-то комментарий мой текущий код, прежде чем я заранее и добавить больше комнат/уровней, любой ввод с благодарностью оценили. Я был также интересно, если я должен сделать .cpp-файл для каждого из моего класса заголовочные файлы, или это ОК, чтобы просто положить мои методы с заголовочных файлов.

main.cpp

#include <iostream>
#include <SFML/Graphics.hpp>
#include "Character.h"
#include "Sprite.h"
#include "Computer.h"
#include "Window.h"





//Use std
using namespace std;
//Boolean to determine if screen will scroll
int windowWidth = 5000;//width of window
int windowHeight = 5000;//height of window
sf::View dunge(sf::FloatRect(x, y, 5000, 5000));
sf::RenderWindow window(sf::VideoMode(windowWidth, windowHeight ), "Awesome Game" );

//player that is part of Character class and computer that is part of Sprite class
Character player("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/Player.png");
Sprite computer("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/CompSprite.png", 1200, 100);
Sprite battery("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery4.png", 0, 0);
Sprite wooddoor("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/wood_door.png", 1200, 1350);

//boolean for whether to show weapon or not
bool showweapon;
//main loop






int main() {
    bool showBox = false;

    //Setting up the dungeon back-round
    sf::Texture dungeon;
    dungeon.loadFromFile("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/DungeonBack.png");
    sf::Sprite backround;
    backround.setTexture(dungeon);


    while (window.isOpen()){

        // check all the window's events that were triggered since the last iteration of the loop
        sf::Event event;
        while (window.pollEvent(event)){
            // "close requested" event: we close the window
            if (event.type == sf::Event::Closed)
                window.close();
        }
        //Movement
        if (moveChar == true){

            if (sf::Keyboard::isKeyPressed(sf::Keyboard::Left)){
                player.left();
                }
            if (sf::Keyboard::isKeyPressed(sf::Keyboard::Right)){
                player.right();
                }
            if (sf::Keyboard::isKeyPressed(sf::Keyboard::Up)){
                player.forward();
                }
            if (sf:: Keyboard::isKeyPressed(sf::Keyboard::Down)){
                player.backward();
                }
            if (sf::Keyboard::isKeyPressed(sf::Keyboard::LShift))
            {
                player.Highspeed();
            }
            else{
                player.Lowspeed();
            }
        }

        if (batstat == 4){
            battery.changeimage("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery4.png");
        }
        if (batstat == 3){
            battery.changeimage("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery3.png");
        }
        if (batstat == 2){
            battery.changeimage("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery2.png");
        }
        if (batstat == 1){
            battery.changeimage("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery1.png");
        }
        if (batstat == 0){
            battery.changeimage("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery0.png");
        }
        if (player.getSprite().getGlobalBounds().intersects(computer.getSprite().getGlobalBounds())){
            show = false;
            player.hascomp = true;
        }
        if (player.getSprite().getGlobalBounds().intersects(wooddoor.getSprite().getGlobalBounds()) and show == false){
            window.setView(dunge);
            scroll = true;
            showBox = true;
        }
        if (sf::Keyboard::isKeyPressed(sf::Keyboard::A)){
            moveChar = true;

        }
        //draw and window stuff
        window.clear(sf::Color(0, 0, 0));
        window.draw(backround);
        if (show == true){
            window.draw(computer.getSprite());
        }
        if (show == false){
            window.draw(battery.getSprite());
            window.draw(wooddoor.getSprite());
        }
        window.draw(player.getSprite());
        window.display();
        window.setFramerateLimit(70);

        }
    }

Спрайт.ч

using namespace std;
bool show = true;
class Sprite{
public:
    int Spritex;
    int Spritey;
    string name;
    sf::Texture texture;

    Sprite(string image, int x, int y){
        Spritex = x;
        Spritey = y;
        texture.loadFromFile(image);
    }
    sf::Sprite getSprite() {
        sf::Sprite sprite;
        sprite.setTexture(texture);
        sprite.setPosition(Spritex, Spritey);
        return sprite;

    }

    void changeimage(string image);


};

void Sprite:: changeimage(string image){
    texture.loadFromFile(image);

}

Характер.ч

using namespace std;
#endif /* Character_h */
int wepx = 1200;
int wepy = 660;
int x_pos;
int y_pos;
int x = 0;
int y = 5000;

int left_limit = 110;
int right_limit = 2300;
int up_limit = 100;
int down_limit = 1400;

bool moveChar = true;
bool GoingRight = false;
bool GoingLeft = false;
bool GoingUp = false;
bool GoingDown = false;

bool scroll = false;

class Character{

public:
    string sprite;
    int health;
    int defense;
    int speed;
    int highspeed;
    int experience;
    bool move = true;
    int x_pos = 1200;
    int y_pos = 600;

    bool hascomp = false;


    sf::Texture texture;

    //Constructor - Ran everytime a new instance of the class is created
    Character(string image){
        health = 100;
        defense = 100;
        speed = 6;
        experience = 0;

        texture.loadFromFile(image);


    }
    sf::Sprite getSprite() {
        sf::Sprite sprite;
        sprite.setTexture(texture);
        sprite.setTextureRect(sf::IntRect(0, 0, 100, 100));
        sprite.setPosition(x_pos, y_pos);
        return sprite;
    }

    //Destructor - Ran when the object is destroyed
    ~Character(){

    }
    //Methods
    void forward();
    void backward();
    void left();
    void right();
    void Highspeed();
    void Lowspeed();
};

void Character::forward(){
    if (y_pos > up_limit){
        y_pos = y_pos - speed;
        wepy = wepy - speed;
        GoingUp = true;
        GoingDown = false;
        GoingLeft = false;
        GoingRight = false;
        texture.loadFromFile("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/PlayerUp.png");
        if (scroll == true){
            y = y - speed;
        }
    }
}
void Character::backward(){
    if (y_pos < down_limit){
        y_pos = y_pos + speed;
        wepy = wepy + speed;
        GoingDown = true;
        GoingUp = false;
        GoingLeft = false;
        GoingRight = false;
        texture.loadFromFile("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/Player.png");
        if (scroll == true){
            y = y + speed;
        }
    }
}
void Character::left(){
    if (x_pos > left_limit){
        x_pos = x_pos - speed;
        wepx = wepx - speed;
        GoingLeft = true;
        GoingRight = false;
        GoingUp = false;
        GoingDown = false;
        texture.loadFromFile("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/PlayerLeft.png");
        if (scroll == true){
            x = x - speed;
        }
    }

}
void Character::right(){
    if (x_pos < right_limit){
        x_pos = x_pos + speed;
        wepx = wepx + speed;
        GoingRight = true;
        GoingLeft = false;
        GoingUp = false;
        GoingDown = false;
        texture.loadFromFile("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/PlayerRight.png");
        if (scroll == true){
            x = x + speed;
        }
    }

}

void Character::Highspeed(){
    speed = 10;
}
void Character::Lowspeed(){
    speed = 6;
}

Я чувствую, что это бардак и может быть значительно улучшена.



324
5
задан 23 февраля 2018 в 05:02 Источник Поделиться
Комментарии
2 ответа

Используйте Осмысленные Имена

А не:

dungeon.loadFromFile("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/DungeonBack.png");

Я бы предпочел что-то вроде:

char const *background_file = "/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/DungeonBack.png";

dungeon.loadFromFile(background_file);

Точно так же, а не:

if (player.getSprite().getGlobalBounds().intersects(computer.getSprite().getGlobalBounds())){
show = false;
player.hascomp = true;
}
if (player.getSprite().getGlobalBounds().intersects(wooddoor.getSprite().getGlobalBounds()) and show == false){
window.setView(dunge);
scroll = true;
showBox = true;
}

Я бы предпочел, чтобы код что-то вроде:

auto const &player_bounds = player.getSprite().getGlobalBounds();
auto const &computer_bounds = computer.getSprite().getGlobalBounds();
auto const &door_bounds = wooddoor.getSprite().getGlobalBounds();

if (player_bounds.intersects(computer_bounds)) {
show = false;
player.hascomp = true;
}
if (player_bounds.intersects(door_bounds) && !show) {
window.setView(dunge);
scroll = true;
showBox = true;
}

Также обратите внимание, что для булевой переменной, это лучше, чем просто сказать if (foo) а не if (foo == true)или if (! foo) или if (not foo) а не if (foo == false).

Предпочитают данные в код

Например, вместо:

    if (batstat == 4){
battery.changeimage("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery4.png");
}
if (batstat == 3){
battery.changeimage("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery3.png");
}
if (batstat == 2){
battery.changeimage("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery2.png");
}
if (batstat == 1){
battery.changeimage("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery1.png");
}
if (batstat == 0){
battery.changeimage("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery0.png");
}

Я бы предпочел использовать что-то вроде:

char const *bat_images[] = {
"/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery0.png",
"/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery1.png",
"/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery2.png",
"/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery3.png",
"/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/battery4.png"
};

if (batstat < elements(bat_images))
battery.changeimage(bat_images[batstat]);

Избегайте жестко запрограммированных путей, магические числа и т. п.

Имена файлов в предыдущем разделе, представляют собой очевидный пример кода, который "работает на моей машине", но создает значительные трудности, как только вы хотите использовать его в другом месте.

Не повторяйся

Во-первых у нас есть код, чтобы увидеть, что была нажата код, и от этого решить, какую функцию вызвать:

if (sf::Keyboard::isKeyPressed(sf::Keyboard::Left)){
player.left();
}
if (sf::Keyboard::isKeyPressed(sf::Keyboard::Right)){
player.right();
}
if (sf::Keyboard::isKeyPressed(sf::Keyboard::Up)){
player.forward();
}
if (sf:: Keyboard::isKeyPressed(sf::Keyboard::Down)){
player.backward();
}

Тогда у нас есть четыре функции, которые только слегка отличаются друг от друга:

void Character::forward(){
if (y_pos > up_limit){
y_pos = y_pos - speed;
wepy = wepy - speed;
GoingUp = true;
GoingDown = false;
GoingLeft = false;
GoingRight = false;
texture.loadFromFile("/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/PlayerUp.png");
if (scroll == true){
y = y - speed;
}
}
}

// similar code for `right`, `left` and `back` elided

Я предпочел бы иметь одну функцию, которую мы называем с параметр, чтобы указать направление, и (снова) база больше на информацию, чем код:

static const auto directions[] = {sf::Keyboard::Left, sf::Keyboard::Right, sf::Keyboard::Down, sf::Keyboard::Up};

// In main:
for (auto d : directions)
if (sf::Keyboard::isKeyPressed(d))
player.move(d);

...потом мы заменить то, что сейчас дискретных переменных на отдельные массивы, что-то вроде этого:

// These replace existing variables:
bool going[4] = {false}; // GoingLeft, GoingRight, GoingDown and GoingRight respectively
int limits[4]; // Left, Right, Lower, Upper limits
int positions[2]; // X and Y position respectively
int wep[2] // wepx and wepy respectively
int xy[2]; // x and y respectively

...и тогда наш код для переезда выглядит примерно так:

void Character::move(T direction) {

static char const *texture_names[] = {
"/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/PlayerLeft.png",
"/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/PlayerRight.png"
"/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/Player.png",
"/Users/danielrailic/Desktop/Xcode /NewGame/ExternalLibs/Sprites/PlayerUp.png",
};

// For the moment, I'm going to assume `Left`, `Right`, `Down` and `Up`
// are contiguous integers in that order, so this gives 0..3:
auto index = direction - sf::Keyboard::Left;

// Is this a positive movement or a negative movement?
int sign = direction == Left || direction == Down ? -1 : 1;

// Is the movement in X or Y?
int dir = direction == left || direction == Right ? 0 : 1;

// The amount we're going to move by:
auto delta = speed * sign;

positions[dir] += delta;
wep[dir] += delta;
std::fill(going.begin(), going.end(), false);
going[direction] = true;
texture.locaFromFile(texture_names[direction]);
if (scroll)
xy[dir] += delta;
}

Там, несомненно, больше можно улучшения (там всегда есть), но я оставлю его в этом.

6
ответ дан 23 февраля 2018 в 07:02 Источник Поделиться

1. Не использовать using namespace std;

Пока что будет работать в вашем конкретном случае, это считается плохой практикой. Особенно, когда вы выходите ваш код в отдельные заголовочные файлы.

Подробнее см. здесь, пожалуйста:

Почему “использование пространства имен std;” считается плохой практикой?

2. Использование отдельных единиц перевода


Я был также интересно, если я должен сделать .cpp-файл для каждого из моего класса заголовочные файлы, или это ОК, чтобы просто положить мои методы с заголовочных файлов.

Да, вы должны сделать это, если у вас нет реализаций шаблона класса.

При этом очень важно, чтобы заголовок охранники, как

#ifndef CHARACTER_H
#define CHARACTER_H

// Class declaration of Character goes here ...

#endif // CHARACTER_H

или

#pragma once

// Class declaration of Character goes here ...

чтобы не допустить повторной декларации ошибок компилятора.

3. Не использовать глобальные переменные

От Sprite.h:

bool show = true;

Кроме того, я не понимаю назначение этой переменной, вы не должны использовать глобальные переменные, чтобы контролировать поведение вашего кода. Если вы уверены, что вам понадобится статическая переменная для этого сделать переменную-член класса.

Также вы получите проблемы (несколько ошибки определения), если включить заголовок в несколько единиц трансляции.

4
ответ дан 23 февраля 2018 в 05:02 Источник Поделиться