ShubinGeorge / cplusplus-tasks

Represetory for C++ homework
2 stars 0 forks source link

homework11-snake #13

Open gitluup opened 5 years ago

gitluup commented 5 years ago

1 и 2) Вопросы 1 и 2 связаны, поэтому я постараюсь ответить сразу на оба.

Во-первых, вы снова смешали логику и графику. Посмотрим на метод Wall::Render. В нём сразу два бага. Первый: https://github.com/ShubinGeorge/cplusplus-tasks/blob/191c643a55734222b39147472d917c81c2f678bb/SNAKE_LESSON/SNAKE_LESSON/wall.cpp#L25 shape_ связан с отрисовкой. И он хочет получить не логические координаты, а координаты в пикселях, где ему отрисовываться. Поэтому в строке выше в метод setPosition нужно передавать не {(float)topleft.x, (float)topleft.y}, а то же самое, но домноженное на BLOCK_SIZE. Второй баг в следующей строке: https://github.com/ShubinGeorge/cplusplus-tasks/blob/191c643a55734222b39147472d917c81c2f678bb/SNAKE_LESSON/SNAKE_LESSON/wall.cpp#L26 Представьте себе стену толщиной одна логическая клетка. Что это значит? Это, например, значит, что у topleft и bottomright одинаковые иксовые координаты. Какая у неё толщина? Естественно, одна логическая клетка. А сейчас у вас толщина будет равна нулю. Поэтому надо добавить единичку к толщине: (bottomright.x + 1 - topleft.x) BLOCK_SIZE (bottomright.y + 1 - topleft.y) BLOCK_SIZE)

И последнее: здесь вроде https://github.com/ShubinGeorge/cplusplus-tasks/blob/191c643a55734222b39147472d917c81c2f678bb/SNAKE_LESSON/SNAKE_LESSON/world.cpp#L13 и здесь https://github.com/ShubinGeorge/cplusplus-tasks/blob/191c643a55734222b39147472d917c81c2f678bb/SNAKE_LESSON/SNAKE_LESSON/world.cpp#L17 вместо единичек должны быть нолики. И если уж прям совсем строго логически, то никакая клеточка в нагем игровом мире не может иметь координату, например, WORLD_SIZES.x. Только WORLD_SIZES.x - 1.


3) MessageBox должен находиться в приватной секции внутри класса Game. Если это член класса Game, то конструктор MessageBox'a обязан вызываться до начала работы первой строки конструктора Game'a. Т.е. он должен вызываться абсолютно также, как и конструктор для Game::world_ или Game::mainwindow (см. то, что находится перед открывающей фигурной скобкой конструктора класса Game). А метод MessageBox::Create должен вызываться из конструктора класса MessageBox.


4) position, который передаётся в конструктор класса MessageBox, представляет собой положение верхнего левого угла box'a. Для того, чтобы это осознать, нужно разобраться, что такое вообще MessageBox и из чего он состоит. MessageBox представляет собой текст, который появляется на прямоугольном фоне, т.е. он состоит из прямоугольного фона и текста на нём. Тогда получается, что position - это верхний левый угол фона, на котором появляется текст, т.е. верхний левый угол background'a (см. sf::RectangleShape background;//фон для сообщений в файле messagebox.h). Ну и, соответственно, sizes - это размеры этого самого фона, т.е. размеры background'a.


gitluup commented 5 years ago

https://github.com/ShubinGeorge/cplusplus-tasks/blob/d52bcede091d0f3c48521371b0f63bd2743f8c7d/SNAKE_LESSON/SNAKE_LESSON/game.cpp#L50

Вы ещё не забыли, что желательно, чтобы функции и методы начинались с глагола?)


https://github.com/ShubinGeorge/cplusplus-tasks/blob/d52bcede091d0f3c48521371b0f63bd2743f8c7d/SNAKE_LESSON/SNAKE_LESSON/main.cpp#L2

Понятно, что наличие здесь этого инклюда в нашем случае ни на что не влияет. Но лучше стараться не инклюдить лишние файлы, т.к. потом возможно возникновение нетривиальных ошибок с инклюдами (ошибки типа unknown type specifier. int assumed ...). В файле main.cpp не требуется использовать ничего из файла SFML/Graphics.hpp. Поэтому этот инклюд можно и желательно убрать.


Ещё такой момент. Старайтесь не делать слишком длинные строки. А именно, старайтесь, чтобы длина любой строки не превышала 79 символов. Какой в этом смысл? Дело в том, что разработчики часто смотрят код на разделённом экране (например, на левой половине монитора .h-файл, а на правой половине - .cpp-файл). В питоне, например, есть PEP8, который именно это и говорит: https://www.python.org/dev/peps/pep-0008/#maximum-line-length В С++, к сожалению, нет единого стандарта на подобие PEP8. Но многие всё же придерживаются мнения, что строки не надо делать длиннее 79 символов.


https://github.com/ShubinGeorge/cplusplus-tasks/blob/d52bcede091d0f3c48521371b0f63bd2743f8c7d/SNAKE_LESSON/SNAKE_LESSON/snake.cpp#L98 static_cast чуточку предпочтительнее


https://github.com/ShubinGeorge/cplusplus-tasks/blob/d52bcede091d0f3c48521371b0f63bd2743f8c7d/SNAKE_LESSON/SNAKE_LESSON/game.cpp#L52 https://github.com/ShubinGeorge/cplusplus-tasks/blob/d52bcede091d0f3c48521371b0f63bd2743f8c7d/SNAKE_LESSON/SNAKE_LESSON/game.cpp#L53 https://github.com/ShubinGeorge/cplusplus-tasks/blob/d52bcede091d0f3c48521371b0f63bd2743f8c7d/SNAKE_LESSON/SNAKE_LESSON/snake.cpp#L98 Константность, константность и ещё раз константность! const не просто так создали. Константность защищает от случайных изменений того, что не должно изменяться. Меня выручало и не раз.


Да, вы верно подметили про проверку столкновений из класса Game. Можно поступить следующим образом: в классе World заводите enum: enum class Event { CollisionWithWall, CollisionWithSnake, CollisionWithApple }; и в приватной секции std::vector events_;

Тогда после каждого апдейта класс Game просматривает события, произошедшие в классе World (класс Game имеет доступ к привтному полю events_, т.к. он является другом класса World), и анализирует их.


gitluup commented 5 years ago

Вектор событий в классе World имеет смысл делать с точки зрения расширяемости. Когда вы будете добавлять новый функционал в проект, то в один прекрасный момент может обнаружиться, что за один апдейт может произойти и 2, и 3, и 10 событий. И вот тогда начинают побаливать головы) Лучше сразу стараться писать расширяемый код.


static_cast ставится в подобных местах https://github.com/ShubinGeorge/cplusplus-tasks/blob/7208f7ae517f53987d15edeba873fbf5e814f7ef/SNAKE_LESSON/SNAKE_LESSON/snake.cpp#L139

вместо (float). Т.е. должно быть как-то так: static_cast < float > (new_part.position.x), static_cast < float > (new_part.position.y) (уберите лишние пробелы, я их вставил, т.к. без них отображалось без значков < и > )


P.S. Домашку я уже засчитал. Будет круто, если вы именно к змейке прикрутите CMake.

gitluup commented 5 years ago

https://github.com/ShubinGeorge/cplusplus-tasks/blob/a98ef77d7edf5fc851bbe8acdd9afa745cad426b/SNakeWithCmake/include/apple.h#L11 Этот метод должен принимать свой аргумент по константной ссылке. У меня сейчас из-за этого не компилировалось. Потому что просто нельзя принять ссылку на неконстантный временный объект.


Ещё папочки data со шрифтами не хватает