вопрос by KloopRE · Pull Request #5 · hBuzzy/Pacman-task

KloopRE · 2023-12-06T10:43:10Z

Почему по нажатию кнопок управления ничего не происходит? возможно, что-то блокирует обработку событий клавиш

hBuzzy

У вас фокус слетает на другой элемент. Установите просто еще одну строку:

setFocusPolicy(Qt::StrongFocus);
setFocus();

Кроме того, задание подразумевает (как описано в разделе интерфейс), использование QGraphicsView и, соответственно, QGraphicsScene для отображения поля. Другие варианты в задаче не принимаются.

В чём ошибка? D:\Pacman-task\hostile.cpp:143: ошибка: undefined reference to `Hostile::Hostile(std::vector<std::vector<int, std::allocator<int> >, std::allocator<std::vector<int, std::allocator<int> > > >&)' D:\Pacman-task\mainwindow.cpp:90: ошибка: undefined reference to `Hostile::Hostile()'

hBuzzy

У вас в hostile.h определены два конструктора, а где их реализация в hostile.cpp? Что вызываться то должно при создании объекта? На это вам компилятор и ругается.

Подумайте, точно ли position_ должен быть ссылкой?

У меня проблема с событием перемещения мыши. Когда я вожу по краям своей формы ui, то событие срабатывает и выводятся координаты, но когда я вожу по его внутренней составляющей он не срабатывает. Я думаю, что проблема в этих строках: scene = new QGraphicsScene(this); view = new QGraphicsView(scene, this); view->setMouseTracking(true); Можете помочь?

KloopRE · 2023-12-09T23:58:25Z

Последний комит относится к разделу редактирования игровой сетки

hBuzzy

У вас есть главный виджет для которого вы переопределили mouseMoveEvent и на этот виджет вы добавляете QGraphicsView. Как только вы переводите мышь на view, главный виджет теряет фокус и у него больше не вызывается mouseMoveEvent. Отсюда и ничего не выводится. Если вам нужен этот функционал, переопределяйте передвижение мыши для других виджетов.

add redactor

edir comments

hBuzzy

Исправить замечания. Нигде не вижу проверки на коллизии -> задание выполнено не полностью.

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

На самом деле, своей реализацией через сетку, в убили всю логику задания с QGraphicsView, но да ладно, если вам так удобно - пусть движение будет осуществляться так.

hBuzzy · 2023-12-13T13:15:09Z

+
+CustomGraphicsView::CustomGraphicsView(QGraphicsScene *scene, QWidget *parent, int gridSize)
+    : QGraphicsView(scene, parent), gridSize(gridSize)  // Добавьте инициализацию gridSize
+{


Раз начали не переносить скобки в части проекта, то не переносите их везде. Касается всего проекта. + Комментарий.

hBuzzy · 2023-12-13T13:15:17Z

@@ -0,0 +1,38 @@
+


Лишняя пустая строка.

hBuzzy · 2023-12-13T13:15:42Z

+        center.setX(this->center.x() - gridSize / 2);
+        center.setY(this->center.y() - gridSize / 2);


2 - магическое число.

hBuzzy · 2023-12-13T13:17:28Z

+    QPointF center;
+    bool isdrawing = false;


Публичных полей, если они не константы, быть не должно. Сделайте их приватными, доступ другими классами к полям осуществляется через методы.

hBuzzy · 2023-12-13T13:18:19Z

+    void mouseReleaseEvent(QMouseEvent *event) override;
+
+private:
+    int gridSize;


К приватным полям добавляется постфикс "_". gridSize -> gridSize_

hBuzzy · 2023-12-13T14:05:57Z

+        if (gameGrid_[a.y()][a.x()] == 2)
+        {
+            isStatePacmanElement_ = false;
+        }
+        gameGrid_[a.y()][a.x()] = dragElement_;
+        if(dragElement_ == 2)
+        {
+            isStatePacmanElement_ = true;
+        }


пробел после if. 2 - магическое число

hBuzzy · 2023-12-13T14:07:13Z

+private:
+    Ui::Redactor *ui;
+    MainWindow *MW;
+    int **gameGrid_;
+    const int rows_ = 10;
+    const int cols_ = 10;
+    int gridSize_ = 64;
+    void setupGameGrid();
+    QGraphicsScene *scene;
+    CustomGraphicsView *view;
+    bool isdrawing_ = false;
+    QPointF center_;
+    int lineWidth_;
+    QColor lineColor_;
+    void drawItems(QPainter *painter, const QPoint &center);
+    int getElement();
+    QGraphicsPixmapItem* wallItem_;
+    QGraphicsPixmapItem* puckmanItem_;
+    QGraphicsPixmapItem* hostileItem_;
+    QGraphicsPixmapItem* coinItem_;
+    QGraphicsPixmapItem* dragItem_ = nullptr;
+    QPoint getGridPoint();
+    int dragElement_;
+    void dragItem();
+    void handleCustomMouseRelease();
+    bool isStatePacmanElement_ = false;
+    QPushButton *myButton;
+    void handleButtonClick();
+    void openGame();
+    void exitRedaction();


Наведите порядок в сортировке. Не все поля имеют постфикс. Константы названы неверно, должен быть префикс "к". Опять же, gridSize - это именно размер всей сетки или ее ячейки?

hBuzzy · 2023-12-13T14:08:10Z

+StartGame::StartGame(QWidget *parent) :
+    QWidget(parent),
+    ui(new Ui::StartGame)
+{
+    ui->setupUi(this);
+    loadUI();
+}
+
+StartGame::~StartGame()
+{
+    delete ui;
+}
+
+void StartGame::loadUI(){
+    ui->play_button->setFont(QFont(default_font_family_, font_size_));
+    ui->redactor_button->setFont(QFont(default_font_family_, font_size_));
+}
+
+void StartGame::on_play_button_clicked()
+{
+    this->close();
+    MainWindow *game = new MainWindow(nullptr, nullptr);
+    game->show();
+}
+
+void StartGame::on_redactor_button_clicked()
+{
+    this->close();
+    Redactor *game = new Redactor();
+    game->show();
+}


Скобки не переносим

hBuzzy · 2023-12-13T14:08:39Z

+void StartGame::on_play_button_clicked()
+{
+    this->close();
+    MainWindow *game = new MainWindow(nullptr, nullptr);
+    game->show();
+}
+
+void StartGame::on_redactor_button_clicked()


Лучше в camelCase даже именование стандартных событий.

hBuzzy · 2023-12-13T14:09:22Z

+    static const int font_size_ = 15;
+    const QString default_font_family_ = "Default";


Константы названы неверно. kCamelCase.

Простите, можно мне пожалуйста не делать collidingItems, просто у меня уже прописана вся логика и мне придется её переписывать, я невнимательно читал задание : ( . Извините!

hBuzzy

Исправить замечания.

hBuzzy · 2023-12-17T11:30:10Z

+
+void CustomGraphicsView::setDrawing(bool newDrawing)
+{
+    isDrawing_ = newDrawing;


newDrawing должно быть аналогично в нотации bool переменных. У вас есть поле isDrawing_, напрашивается сразу присвоить ему значение из переменной isDrawing. Тут нам, чтобы не изобретать велосипед, помогает разница в нотации переменных и приватных полей.

hBuzzy · 2023-12-17T11:32:45Z

+    if (isDrawing_)
+    {
        setCursor(Qt::CrossCursor);
    } else {
        setCursor(Qt::ArrowCursor);
    }


У else неверно расставлены скобки. Раз везде решили не переносить, то:

if () { //Код } else { //Код }

hBuzzy · 2023-12-17T11:38:37Z

-    vector<vector<Point>> path(rows, vector<Point>(cols, Point(-1, -1)));
+    vector<vector<int>> distance(rows, vector<int>(columns, INT_MAX));
+    vector<vector<Point>> path(rows, vector<Point>(columns, Point(-1, -1)));
    priority_queue<Node, vector<Node>, greater<Node>> pq;


Все еще не исправлено сокращение.

hBuzzy · 2023-12-17T11:41:36Z

+                if (dx == -1)
+                {
+                    directions.push_back("Left");
+                } else if (dx == 1)
+                {
+                    directions.push_back("Right");
+                } else if (dy == -1)
+                {
+                    directions.push_back("Up");
+                } else if (dy == 1)
+                {
+                    directions.push_back("Down");


Опять же, во многих частях кода есть данные строки с направлениями. Если начнем их менять, попадем в ад.

Если все в пределах одного класса, то можно создать константы внутри класса:

const QString kLeft = "Left" и т.д.

Если они разбросаны по разным классам, то просто создайте абстрактный класс Directions в котором будут статические константы направлений.

hBuzzy · 2023-12-17T11:42:14Z

        }

-        for (int i = 0; i < 4; ++i) {
+        int step_weight = 1;


step_weight -> stepWeight.

hBuzzy · 2023-12-17T11:55:46Z

                }
            }
        }
        int RETURN = 2;


Не исправлено замечание про неверную нотацию именования переменной.

hBuzzy · 2023-12-17T11:56:48Z

+        puckmanItem_ = scene->addPixmap(QPixmap(":resource/puckman.png").scaled(cellSize_, cellSize_));
+        puckmanItem_->setX((kColumns_+RETURN) * cellSize_);
+        puckmanItem_->setY(4 * cellSize_);


Пробелы и магические числа, как тут, так и в coinItem, 1, 4, 6, 8 и т.д.

hBuzzy · 2023-12-17T11:57:35Z

+    if(view->getCenterX()< hostileItem_->x()+cellSize_ && view->getCenterX()> hostileItem_->x()-cellSize_)
    {
-        if(view->center.y() < hostileItem_->y()+gridSize_ && view->center.y() > hostileItem_->y()-gridSize_)
+        if(view->getCenterY() < hostileItem_->y()+cellSize_ && view->getCenterY() > hostileItem_->y()-cellSize_)
        {
            qDebug() << "you drag item: (" << item << ")";
-            return 3;
+            return GameElement::Hostile;
        }
    }
-    if(view->center.x() < coinItem_->x()+gridSize_ && view->center.x() > coinItem_->x()-gridSize_)
+    if(view->getCenterX()< coinItem_->x()+cellSize_ && view->getCenterX()> coinItem_->x()-cellSize_)
    {
-        if(view->center.y() < coinItem_->y()+gridSize_ && view->center.y() > coinItem_->y()-gridSize_)
+        if(view->getCenterY() < coinItem_->y()+cellSize_ && view->getCenterY() > coinItem_->y()-cellSize_)
        {
            qDebug() << "you drag item: (" << item << ")";
-            return 4;
+            return GameElement::Coin;
        }
    }


Вообще почти нигде не расставлены пробелы. Ни после if, ни вокруг мат. операторов, ни вокруг операторов сравнения.

hBuzzy · 2023-12-17T11:57:49Z

+        if(dragElement_ == GameElement::Puckman)
        {
            isStatePacmanElement_ = true;
        }
+        if(dragElement_ == GameElement::Coin)


Пробелы перед if

hBuzzy · 2023-12-17T11:57:57Z

-    QPoint a = getGridPoint();
-    if(a.x() > 0 || a.y() > 0)
+    QPoint cell = getGridPoint();
+    if(cell.x() > 0 || cell.y() > 0)


Пробел перед if

hBuzzy

Исправить замечания. Внимательно просмотрите все замечания, некоторые замечания не исправляются уже 3ий раз.

hBuzzy · 2023-12-17T16:24:50Z

+    if (dx > 0)
+    {
+        directions.push_back(Directions(Directions::Right));
+    } else if (dx < 0)


else if нужно перенести

hBuzzy · 2023-12-17T16:28:32Z

+{
+    if (counsCount_ <= 0)
+    {
+        std::string str1 = "couns count < 0";


str1 - бесполезное название, дайте осмысленное название.

hBuzzy · 2023-12-17T16:30:37Z

+        nextRow = 4 * cellSize_;
+
+        puckmanItem_ = scene->addPixmap(QPixmap(":resource/puckman.png").scaled(cellSize_, cellSize_));
+        puckmanItem_->setX((extraColumns) * cellSize_);
+        puckmanItem_->setY(nextRow);
+        qDebug() << "puckmanItem Coordinates: (" << puckmanItem_->x() << ", " << puckmanItem_->y() << ")";
+        extraColumns = 2;
+        nextRow = 6 * cellSize_;
+
+        hostileItem_ = scene->addPixmap(QPixmap(":resource/hostile.png").scaled(cellSize_, cellSize_));
+        hostileItem_->setX((extraColumns) * cellSize_);
+        hostileItem_->setY(nextRow);
+        qDebug() << "hostileItem Coordinates: (" << hostileItem_->x() << ", " << hostileItem_->y() << ")";
+        extraColumns = 2;
+        nextRow = 8 * cellSize_;


Все еще 6, 8, 4, 1 встречаются в коде в виде просто чисел. Что это за числа!? Все это - магические значения, никто, кроме того, кто писал код, сразу не поймет что это такое. Дайте им названия в полях или переменных.

hBuzzy · 2023-12-17T16:30:53Z

+                }
+            }
+        }
+        int extraColumns = kColumns_+2;


Пробел с двух сторон от оператора сложения.

hBuzzy · 2023-12-17T16:33:32Z

+
+    gameTimer_ = new QTimer(this);
+    gameTimer_->start(250);
+    connect(gameTimer_, SIGNAL(timeout()), this, SLOT(updateGameTime()));


Все еще старая запись сигналов и словом, вместо новой с указателями.

hBuzzy · 2023-12-17T16:40:37Z

+void MainWindow::generateRandomElements(int element, int count)
+{
+    Hostile newHostile;
+    Point p(0,0);


Название с сокращением не исправляется уже 3ий раз!

hBuzzy · 2023-12-17T16:41:14Z

+            randomRow = std::rand() % kRows_;
+            randomColumn = std::rand() % kColumns_;
+        }
+        if(element == 3)


hBuzzy · 2023-12-17T16:44:13Z

+            hostilePosition = currentHostile.getPosition();
+            gameGrid_[hostilePosition.y][hostilePosition.x] = currentHostile.getPreviousElement();
+            currentHostile.setPreviousElement(gameGrid_[hostilePosition.y][hostilePosition.x - 1]);
+            hostilePosition.x = hostilePosition.x - 1;
+            gameGrid_[hostilePosition.y][hostilePosition.x] = 3;
+            currentHostile.setPosition(hostilePosition);
+        }
+        if (direction[0] == Directions::Right)
+        {
+            hostilePosition = currentHostile.getPosition();
+            gameGrid_[hostilePosition.y][hostilePosition.x] = currentHostile.getPreviousElement();
+            currentHostile.setPreviousElement(gameGrid_[hostilePosition.y][hostilePosition.x + 1]);
+            hostilePosition.x = hostilePosition.x + 1;
+            gameGrid_[hostilePosition.y][hostilePosition.x] = 3;
+            currentHostile.setPosition(hostilePosition);
+        }
+        if (direction[0] == Directions::Up)
+        {
+
+            hostilePosition = currentHostile.getPosition();
+            gameGrid_[hostilePosition.y][hostilePosition.x] = currentHostile.getPreviousElement();
+            currentHostile.setPreviousElement(gameGrid_[hostilePosition.y-1][hostilePosition.x]);
+            hostilePosition.y = hostilePosition.y - 1;
+            gameGrid_[hostilePosition.y][hostilePosition.x] = 3;
+            qDebug() << "gameGrid_ Coordinates: (" << hostilePosition.x << ", " << hostilePosition.y << ")";
+            currentHostile.setPosition(hostilePosition);
+        }
+        if (direction[0] == Directions::Down)
+        {
+
+            hostilePosition = currentHostile.getPosition();
+            gameGrid_[hostilePosition.y][hostilePosition.x] = currentHostile.getPreviousElement();
+            currentHostile.setPreviousElement(gameGrid_[hostilePosition.y+1][hostilePosition.x]);
+            hostilePosition.y = hostilePosition.y + 1;
+            gameGrid_[hostilePosition.y][hostilePosition.x] = 3;
+            qDebug() << "gameGrid_ Coordinates: (" << hostilePosition.x << ", " << hostilePosition.y << ")";
+            currentHostile.setPosition(hostilePosition);


4 одинаковых куска кода, отличающиеся только параметрами -> Дубляж кода! Вынесите повторяющийся код в метод / методы!

hBuzzy · 2023-12-17T16:45:25Z

+    }
+}
+
+void MainWindow::movePlayerRight()


Опять же, все методы движения - дубляж кода.... опять не исправлено. Весь повторяющийся код нужно разбивать и выносить отдельно...

hBuzzy · 2023-12-17T16:46:34Z

+    int uiWidth = (kColumns_ + 6) * cellSize_;
+    int uiHeight = (kRows_ + 1) * cellSize_;


6 и 1 все еще дубляж кода. Вообще у вас уже есть схожий конструктор в классе mainwinow.... прямо напрашивается создать базовый класс и вынести эту функциональность в нее.

else if нужно перенести Опять 1 и 4 - числа, уже 3е исправление. Все это - магические числа. Как я уже писал, у вас 1, 4, 6, 8 встречаются числами во всем коде - все это магические числа. Исправьте во ВСЕМ проекте. 1 - не во вех местах маг. число, но там, где оно идет по логике с 4, 6, 8 - магическое точно. str1 ? Почему она тут 1? Дайте нормальное название. Не исправлено сокращение Название с сокращением не исправляется уже 3ий раз! Пробел после if, скобку не переносим player_.getY()+1 Пробелы. 6 и 1 все еще дубляж кода. Вообще у вас уже есть схожий конструктор в классе mainwinow.... прямо напрашивается создать базовый класс и вынести эту функциональность в нее. str1 - бесполезное название, дайте осмысленное название.

hBuzzy

Эх, не буду больше вас тиранить, задачу приму.

Однако, в задаче все еще куча проблем. Самые важные из них:

Дубляж кода все так же и остался актуальным, а это очень плохо.
Скобки так же летают по проекту. Либо везде переносим, либо везде - нет. Точка. Не так, что для методов переносим, а для тела метода - нет.

В целом, работы проделано достаточно, если будет время и сможете исправить остаток замечаний, я даже думаю, что накину вам доп. баллы, к примеру, к паутине с 0.5 текущих до 1.

вопрос

07e9843

Почему по нажатию кнопок управления ничего не происходит? возможно, что-то блокирует обработку событий клавиш

hBuzzy reviewed Dec 6, 2023

View reviewed changes

hBuzzy reviewed Dec 8, 2023

View reviewed changes

hBuzzy reviewed Dec 11, 2023

View reviewed changes

KloopRE added 2 commits December 12, 2023 11:52

redactor

6f7e4bd

add redactor

pacman

4747e86

edir comments

hBuzzy requested changes Dec 13, 2023

View reviewed changes

hBuzzy requested changes Dec 17, 2023

View reviewed changes

Исправление ошибок

bdb0d05

hBuzzy requested changes Dec 17, 2023

View reviewed changes

hBuzzy reviewed Dec 19, 2023

View reviewed changes

		center.setX(this->center.x() - gridSize / 2);
		center.setY(this->center.y() - gridSize / 2);

		static const int font_size_ = 15;
		const QString default_font_family_ = "Default";

		int uiWidth = (kColumns_ + 6) * cellSize_;
		int uiHeight = (kRows_ + 1) * cellSize_;

Conversation

KloopRE commented Dec 6, 2023

Uh oh!

hBuzzy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hBuzzy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KloopRE commented Dec 9, 2023

Uh oh!

hBuzzy left a comment

Choose a reason for hiding this comment

Uh oh!

hBuzzy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hBuzzy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hBuzzy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

hBuzzy left a comment •

edited

Loading

hBuzzy left a comment •

edited

Loading