-
Notifications
You must be signed in to change notification settings - Fork 1
Dlipko #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Есть возможность подключится к серверу. Он объединяет в парочки каждых двух подключившихся. Потом если один из них пишет на сревер, он отправляет это обратно отправителю и его противнику по игре.
user may login or signep by sending xml <user><signup><login>dlipko</login><password>12345</password></signup></user> or <user><login><login>dlipko</login><password>12345</password></login></user>
Добавил коментарии. Генерации карт нет. Из карт пока только студет. Свойств у карт пока тоже нет.
Добавил карты сущности со свойствами, просто сущности и карты со свойствами. Воспользовался паттерном Factory Method. Но генерация карт пока не продуманная. Планирую вынести генерацию карт в отдельный класс. Пока что все в юзере. Перенес генерацию xml(qdomelement) в сами классы.
leshiy1295
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
сейчас самая большая проблема - большая привязка к Qt
client/abstractcard.cpp
Outdated
| @@ -0,0 +1,17 @@ | |||
| #include "abstractcard.h" | |||
|
|
|||
| AbstractCard::AbstractCard(int id, QString info): | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
отделите код от Qt, где в этом нет необходимости
client/abstractcard.h
Outdated
| #ifndef ABSTRACTCARD_H | ||
| #define ABSTRACTCARD_H | ||
|
|
||
| #include <QString> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Qt - это реализация, поэтому постарайтесь не включать без необходимости его библиотеки в .h файлах
client/client.cpp
Outdated
| { | ||
| _socket = new QTcpSocket(this); | ||
|
|
||
| _socket->connectToHost("127.0.0.1", 1234); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
это необходимо пробрасывать через конструктор и по-хорошему брать либо из argv, либо из конфигурационных файлов
client/client.cpp
Outdated
|
|
||
| void Client::disconnected() | ||
| { | ||
| qDebug() << "disconnected..."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лучше создать отдельную сущность логгера и пользоваться ей через единый интерфейс везде, в main же создавать реализацию на Qt
client/client.cpp
Outdated
|
|
||
| QByteArray data; | ||
| data = _socket->readAll(); | ||
| while (!data.contains("</server>") && _socket->waitForReadyRead()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
кажется, что это хардкод
server/player.cpp
Outdated
|
|
||
| for (int i= 4; i < HEAND_SIZE; ++i) | ||
| { | ||
| newDeck->addCard(new EntitySpy(i, "Woodcutter Spy", Woodcutter, qrand() % MAX_STRENGTH + 1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не нужно использовать Qt без необходимости
server/player.cpp
Outdated
|
|
||
| void Player::setWin() | ||
| { | ||
| _win = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
хардкод
server/player.cpp
Outdated
|
|
||
| QDomElement Player::toQDomElement(QString playerName) | ||
| { | ||
| QDomDocument document; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
методы сериализации стоит вынести в отдельный интерфейс, от которого наследовать Player
server/player.h
Outdated
| Deck *_deckGenerator(); | ||
| Deck *_heandGenerator(); | ||
|
|
||
| QDomElement _domElement(QString elementName, int value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
вынести Qt из реализации
| @@ -0,0 +1,25 @@ | |||
| #ifndef USER_H | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
Полностью избавился от привязки к qt там где в этом нет необходимости. Добавил конфигурационный файл и логгер. Постарался воспользоваться умными указателями. По возможности исправил все указанные недочеты.
leshiy1295
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Намного лучше
Нужно немного доправить архитектуру
server/configparser.h
Outdated
| std::map<PROPERTY_TYPE, std::string> getPropertiesInfo() const; | ||
|
|
||
| private: | ||
| pugi::xml_document doc; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
зависимость осталась в header-файле - нехорошо
в качестве решения можно предложить использовать идиому PIMPL
server/game.cpp
Outdated
| if (!_waitingForCouple.empty()){ | ||
|
|
||
| // создает игроков | ||
| auto player1 = std::shared_ptr<Player>(new Player(_getUser(_waitingForCouple))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
есть make_shared
| InterLayer::~InterLayer() { | ||
|
|
||
| for (auto socket: _socket) | ||
| delete socket.second; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
удобнее использовать умные указатели
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Уже несколько раз пытался использовать с сокетами shared_ptr. Каждый раз появляется не понятная проблема, с которой мне пока что так и не удалось разобраться.
| std::string _parseGetStr3(pugi::xml_document &message,std::string ancestor1, | ||
| std::string ancestor2, std::string parent) const; | ||
|
|
||
| bool _signIn(pugi::xml_document &message, Socket* client); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PIMPL
server/socket.h
Outdated
|
|
||
| #include <string> | ||
|
|
||
| class Socket: public QTcpSocket |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
нужно сделать Socket абстрактным
и QSocket уже Ваш, который будет наследоваться от Socket и от QTcpSocket
| #include <memory> | ||
|
|
||
|
|
||
| class Game: QDomSerializer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
немного не так - Вы сделали Game QT-классом
нужно сделать абстрактный интерфейс Serializer, добавить поле Parser внутрь Game, передавать ему QtParser, который уже будет сериализовывать
Использовал идиому PIMPL для pugi. Сделал Socket абстрактным классом. Создал класс QSocket, который унаследовал от Socket и от QTcpSocket. Сделать абстрактный интерфейс Serializer, добавил поле Parser внутрь Game, в который передаю QDomSerializer.
|
|
||
| private: | ||
| pugi::xml_document doc; | ||
| std::shared_ptr<pugi::xml_document> doc; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
это не PIMPL..
PIMPL - это когда тут Вы оставляете указатель на класс реализации...
А в нём уже подключаете этот файл.
Из этого же класса Вы проксируете все запросы в класс реализации
| // создает игроков | ||
| auto player1 = std::shared_ptr<Player>(new Player(_getUser(_waitingForCouple))); | ||
| auto player2 = std::shared_ptr<Player>(new Player(_getUser(login))); | ||
| auto player1 = std::make_shared<Player>(_getUser(_waitingForCouple)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
когда тип однозначно определён, то make_shared можно явно не инстанцировать - в этом вся соль)
| #include <QHostAddress> | ||
|
|
||
| class Server: public QTcpServer | ||
| class Server |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
раз это сервер, который не привязан к Server, то у него не должно быть методов с QByteArray - нужно просто с байтами. В Вашем же классе Вы просто преобразуете QByte к обычным (ну или наоборот)
Клиент подключается к серверу и отправляет xml с логином и паролем
(abcd12345)
Клиент отправляет на сервер команду о желании начать партию
(1)
-Клиент получает данные: победа, количество побед в раунде, его ли очередь хода, карты в руке, карты на поле, количество карт в руке соперника
-После того как пасанет один игрок, другой может продолжать кидать карты пока он сам не пасанет.
-За ход можно сыграть одну карту из руки (не считая карт со способностями, которые могут вытягивать на поле другие карты)
-В каждом раунде поле обновляется(не считая карт со специальных карт со способностями)
-Колода одна на всю игру
-Карты в руке и колода при перехода от раунда к раунду не изменяются
Есть три типа карт:
-Сущность (обладает силой)
Скорее всего от сущности будет зависеть её сила
-Сущность со способностью(обладает силой и способностью)
-Способность (Какие-нибудь карты погоды или что-нибудь подобное)
3.1. buff Удваивает свою силу и силу карты на поле с соответствующей сущностью (Для сущности со способностью)
3.2. spy карта кладется на стол противника, но в руку получаете +2 карты (Для сущности со способностью)
3.3 rollcall вытягивает подобную сущность из колоды вместе с собой на поле (Для сущности со способностью)
Воспользовался паттерном Factory Method. Но генерация карт пока не продуманная. Планирую вынести генерацию карт в отдельный класс. Пока что все в юзере.
Перенес генерацию xml(qdomelement) в сами классы.