Address code review

README.md

@@ -71,7 +71,7 @@ Some messages consist of only type character others contain more data.
 - `j` - request join (player will be sent to game room or queue)
 - `q` - request quit (player will be removed from game room or queue, can still rejoin with `j`)
 - `m` `<x1>` ` ` `<y1>` ` ` `<x2>` ` ` `<y2>` `\n` - request unit to move (`<x1><y1>` are coordinates of the unit, `<x2><y2>` designate destination)
-- `a` `<x1>` ` ` `<y1>` ` ` `<x2>` ` ` `<y2>` `\n` - request unit to move (`<x1><y1>` are coordinates of the unit, `<x2><y2>` coordinates of the target unit) (possible to attack own units)
+- `a` `<x1>` ` ` `<y1>` ` ` `<x2>` ` ` `<y2>` `\n` - request unit to attack another unit (`<x1><y1>` are coordinates of the unit, `<x2><y2>` coordinates of the target unit) (possible to attack own units)
 - `d` `<x1>` ` ` `<y1>` `\n` - request unit to mine the resource (`<x1><y1>` are coordinates of the unit) (unit can only mine resource that it is standing on)
 
 ### From server
@@ -116,7 +116,7 @@ Structure as follows:
 
 Numbers are represented as strings of characters (97 ---> "97" not 'a').
 
-### Communication order (client`s perspective)
+### Communication order (client's perspective)
 
 **1:** server sends `c`  
 **2:** client sends `n`  

src/msg/addressUnit.cpp

@@ -7,7 +7,7 @@ message::addressUnitByCoordinates::addressUnitByCoordinates(
     ) : addressUnit(), x(xpos), y(ypos) {}
 
 rts::unit* message::addressUnitByCoordinates::getUnit(const rts::game* g) const{
-    const rts::field* f = g->_board.getConstField(x, y);
+    const rts::field* f = g->_board.getField(x, y);
     if (!f) return nullptr;
     return f->_unit;
 }

src/net/client.cpp

@@ -6,12 +6,22 @@
 
 const unsigned long client::bufsiz = 256;
 
-client::client(server* server_) : owner(server_), clientAddr(), clientAddrSize(sizeof(clientAddr)) {
+client::client(server* server_) : owner(server_) {
+
+    sockaddr_storage clientAddr;
+    socklen_t clientAddrSize(sizeof(clientAddr));
 
     // accept new connection
     _fd = accept(server_->fd(), (sockaddr *)&clientAddr, &clientAddrSize);
-    if (_fd == -1)
+    if (_fd == -1) {
         perror("accept failed");
+        throw connectException("accept failed");
+    }
+
+    char host_[NI_MAXHOST], port_[NI_MAXSERV];
+    getnameinfo((sockaddr *)&clientAddr, clientAddrSize, host_, NI_MAXHOST, port_, NI_MAXSERV, 0);
+    _hostname = std::string(host_);
+    _port = stoi(std::string(port_));
 
     // tell who has connected
     printf("new connection from: ");
@@ -25,28 +35,16 @@ client::client(server* server_) : owner(server_), clientAddr(), clientAddrSize(s
 
 int client::fd() const {return _fd;}
 
-std::string client::host() const {
-    std::string host;
-    name(&host, nullptr);
-    return host;
+std::string client::hostname() const {
+    return _hostname;
 }
 
 unsigned int client::port() const {
-    unsigned int port;
-    name(nullptr, &port);
-    return port;
-}
-
-void client::name(std::string* host, unsigned int* port) const {
-    char host_[NI_MAXHOST], port_[NI_MAXSERV];
-    getnameinfo((sockaddr *)&clientAddr, clientAddrSize, host_, NI_MAXHOST, port_, NI_MAXSERV, 0);
-    if (host) *host = std::string(host_);
-    if (port) *port = stoi(std::string(port_));
+    return _port;
 }
 
 void client::printName() const {
-    std::string host_ = host();
-    printf("%s:%d (fd: %d)", host_.c_str(), port(), _fd);
+    printf("%s:%d (fd: %d)", _hostname.c_str(), _port, _fd);
 }
 
 epoll_event client::inEvent() {
@@ -114,4 +112,7 @@ client::~client() {
     printf("closed client ");
     printName();
     printf("\n");
-}
+}
+
+client::connectException::connectException(const char* _Message) : runtime_error(_Message) {}
+client::connectException::connectException(const std::string& _Message) : runtime_error(_Message) {}

src/net/client.hpp

@@ -5,15 +5,16 @@
 #include <vector>
 #include <functional>
 #include <string>
+#include <stdexcept>
 
 class server;
 
 class client {
 private:
     int _fd;
+    unsigned int _port;
     server* owner; // server to which this client is connected to
-    sockaddr_storage clientAddr;
-    socklen_t clientAddrSize;
+    std::string _hostname;
 
     static const unsigned long bufsiz;
     std::vector<char> buffer;
@@ -42,13 +43,18 @@ class client {
 
 public:
 
+    class connectException : public std::runtime_error {
+    public:
+        connectException(const char* _Message);
+        connectException(const std::string& _Message);
+    };
+
     std::function<void(const std::vector<char>&)> onReceive = [](const std::vector<char>&){};
     std::function<void()> onDisconnect = [](){};
 
     int fd() const;
-    std::string host() const;
+    std::string hostname() const;
     unsigned int port() const;
-    void name(std::string* host, unsigned int* port) const;
     void printName() const;
 
     // @brief copies data to buffer and sets servers' epoll to wait for EPOLLOUT event.

src/net/server.cpp

@@ -55,10 +55,14 @@ int server::fd() const {return _fd;}
 int server::epollFd() const {return _epollFd;}
 
 client* server::newClient() {
-    client* client_ = new client(this);
-    clients.insert(client_);
-    onNewClient(client_);
-    return client_;
+    try {
+        client* client_ = new client(this);
+        clients.insert(client_);
+        onNewClient(client_);
+        return client_;
+    } catch (client::connectException& cce){
+        return nullptr;
+    }
 }
 
 void server::deleteClient(client* client_){
@@ -69,34 +73,31 @@ void server::deleteClient(client* client_){
 
 void server::loop(const int& millis){
     epoll_event ee;
-    int remaining = millis;
+    auto end = std::chrono::steady_clock::now() + std::chrono::milliseconds(millis);
     while (true)
     {
-        auto begin = std::chrono::steady_clock::now();
-        int ready = epoll_wait(_epollFd, &ee, 1, remaining);
+        int ready = epoll_wait(_epollFd, &ee, 1, 
+            std::chrono::duration_cast<std::chrono::milliseconds>(
+            end - std::chrono::steady_clock::now()).count());
+
         if (ready == 1) {
             if (ee.data.ptr == nullptr){
                 newClient();
             }
             else {
                 client* client_ = (client*)(ee.data.ptr);
-                if (ee.events & EPOLLIN) {
-                    client_->receive();
-                }
                 if (ee.events & EPOLLOUT) {
                     client_->sendFromBuffer();
                 }
+                if (ee.events & EPOLLIN) {
+                    client_->receive();
+                }
             }
         }
-        auto end = std::chrono::steady_clock::now();
-        remaining -= std::chrono::duration_cast<std::chrono::milliseconds>(end - begin).count();
 
-        if (remaining <= 0) {
-            auto begin = std::chrono::steady_clock::now();
+        if (end <= std::chrono::steady_clock::now()) {
             loopLogic();
-            remaining = millis;
-            auto end = std::chrono::steady_clock::now();
-            remaining -= std::chrono::duration_cast<std::chrono::milliseconds>(end - begin).count();
+            end += std::chrono::milliseconds(millis);
         }
     }
 

src/rts/board.cpp

@@ -17,7 +17,7 @@ rts::field* rts::board::getField(unsigned int xpos, unsigned int ypos) {
         return &fields[xpos][ypos];
     else return nullptr;
 }
-const rts::field* rts::board::getConstField(unsigned int xpos, unsigned int ypos) const {
+const rts::field* rts::board::getField(unsigned int xpos, unsigned int ypos) const {
     if (xpos < getXdim() && ypos < getYdim())
         return &fields[xpos][ypos];
     else return nullptr;
@@ -84,7 +84,9 @@ rts::field* rts::board::closestEmptyField(const field* source) {
 }
 
 rts::field* rts::board::spawnResource(unsigned int hp) {
-    return randomResourceField(false)->spawnResource(hp);
+    field* f = randomResourceField(false);
+    if (f) f->spawnResource(hp);
+    return f;
 }
 
 void rts::board::spawnResources(unsigned int amount, unsigned int hp) {

src/rts/board.hpp

@@ -15,7 +15,7 @@ namespace rts {
         board(unsigned int x = 256, unsigned int y = 256);
 
         field* getField(unsigned int xpos, unsigned int ypos);
-        const field* getConstField(unsigned int xpos, unsigned int ypos) const;
+        const field* getField(unsigned int xpos, unsigned int ypos) const;
 
         std::vector<field*> resourceFields(bool resource);
         std::vector<const field*> constResourceFields(bool resource) const;

src/rts/game.cpp

@@ -121,7 +121,12 @@ void rts::game::handleNewClient(client* client_) {
 
 void rts::game::loopLogic(){
     // spawn resource and inform players
-    if (rand() % 10 == 0) sendToPlayers(activePlayers, newResourceMessage(_board.spawnResource(resourceHp)));
+    if (rand() % 10 == 0) {
+        field* f = _board.spawnResource(resourceHp);
+        if (f) {
+            sendToPlayers(activePlayers, newResourceMessage(f));
+        }
+    }
 
     // allow units to move again
     for (player* p : activePlayers) {
@@ -258,6 +263,7 @@ unsigned int rts::game::getNextUnitId() {
 }
 
 bool rts::game::nameValid(const std::string& name) const {
+    if (name.empty()) return false;
     for (char c : name){
         if (allowedNameCharacters.find(c) == std::string::npos) return false;
     }