Tcp server dealing with clients disconnecting

[Tytan]

Oooookkayyyy, first off the code is for a server, but the problem is with me trying to remove a client from the list of connected clients.

Problematic code:

(click to show/hide)

sf::Socket::Status status = clients.receive(buffer, sizeof(buffer), received);

         if (status == sf::Socket::Disconnected)
         {
            std::cout << "\nUser #" << i << " has discconected and has been removed from the user list" << std::endl;
            clients.disconnect();
            iter_swap(clients.begin() + i, clients.begin() + maxClients); //Swap indexs of disconnected user with client at end of list
            numClients--;
            continue;
           }

Now wtf i'm trying to do:
When a client disconnects, I want to remove them from the list of clients (as said above), but the way I have set up the server makes this really annoying bug, basically if a client disconnects, it kicks the person that joined last.
The way I am trying to fix this is by moving the disconnected clients vector entry thingy to the back of the vector, so that it would continue server-ing normally.


Now the error I end up with by using the above code:

(click to show/hide)

'sf::TcpSocket::TcpSocket(const sf::TcpSocket &)': attemtping to referance a deleted function

What I've tried:

• Using a deque instead of a vector to remove elements from the list, and move the ones above to it's place (Same error)
• (Probs a dumb idea kek) Using a second vector to represent the other one, filled with ints that represent indexes in the client vector, allowing my to do clients[indexs].blah_blah

Extra notes:

• Clients is a vector of sf::TcpSockets
• The disconnect check thing is in a for loop iterating though each client connected
• numClients is how many people are connected

Please don't treat me like people on stack overflow do xD

[eXpl0it3r]

A socket can't be copied, so don't try to copy it.

Two possible solutions:

• Use a std::vector<std::unique_ptr<sf::TcpSocket>>
• Use a std::list<sf::TcpSocket>

[Tytan]

Thank you so much!
I think you saved a newb a lot of time with this ;P

Also, which one is better out of the two options? ( If one is better of course ;P )

[Tytan]

Also *2, I might go with the unique_ptr option, because with std::list (I think) I would have to remake my entire server loop thingy lol

Also *3, this line:

while (listener.accept(clients[numClients + 1].get()) == sf::Socket::Done)

Would be used to accept the newest conection, but I get an error saying that the initial value of reference to non-const must be an lvalue, what does this mean?

[Tytan]

Please someone help me
This is very annoying...

[eXpl0it3r]

while (listener.accept(clients[numClients + 1].get()) == sf::Socket::Done)

Would be used to accept the newest conection, but I get an error saying that the initial value of reference to non-const must be an lvalue, what does this mean?

Did you google the error message?
It's a C++ error message and I bet you'll find a general explanation of it on StackOverflow or other places.

The best way to get some fast help, is to create a compilable and minimal example that demonstrates the issue. That way anyone can see the full scope and understand what is going on.

The error with the code here doesn't seem to make as much sense to me. It would mean that accept() expects a reference (which it does), but you're passing a r-value object (i.e. temporary variable), which you aren't really. Instead you're passing a raw pointer to the accept function. So that's why I feel the error isn't really describing the problem of the posted code.

[Tytan]

while (listener.accept(clients[numClients + 1].get()) == sf::Socket::Done)

Would be used to accept the newest conection, but I get an error saying that the initial value of reference to non-const must be an lvalue, what does this mean?

Did you google the error message?
It's a C++ error message and I bet you'll find a general explanation of it on StackOverflow or other places.

The best way to get some fast help, is to create a compilable and minimal example that demonstrates the issue. That way anyone can see the full scope and understand what is going on.

The error with the code here doesn't seem to make as much sense to me. It would mean that accept() expects a reference (which it does), but you're passing a r-value object (i.e. temporary variable), which you aren't really. Instead you're passing a raw pointer to the accept function. So that's why I feel the error isn't really describing the problem of the posted code.

Yes I'm an idiot for not googling it.
Sorry about that.

Anyways, I took that same code listener.accept code and put it in a new project.
Same problem...

(click to show/hide)

#include <iostream>
#include <vector>
#include <memory>
#include <SFML\Network.hpp>

int main()
{
       const int maxClients = 100;
   int numClients = 0;
   std::vector<std::unique_ptr<sf::TcpSocket>> clients(maxClients);
   sf::TcpListener listener;

   while (listener.accept(clients[numClients + 1].get()) == sf::Socket::Done)
   {
      numClients++;
      std::cout << "new guy connected" << std::endl;
   }

    return 0;
}

Might show this to stack overflow (And also prepare for insults lol)

[Gleade]

You're making some very basic mistakes here. You should probably look deeper into vectors and smart pointers. 

[Tytan]

Was discussing the problem with a friend and he told me that std::unique_ptr doesn't call the constructor for sf::TcpSocket, and that I need to make them own the objects.

As far as I know, I could do this by using:

(click to show/hide)

std::unique_ptr<sf::TcpSocket> clients(new sf::TcpSocket)

But I don't know how I would do this in my case since I'm making a vector of them and can't pass args to the constructor of std::unique_ptr...

You're making some very basic mistakes here. You should probably look deeper into vectors and smart pointers.

Is this the what you were talking about?

[eXpl0it3r]

#include <iostream>
#include <vector>
#include <memory>
#include <SFML/Network.hpp>

int main()
{
    const int maxClients = 100;
    std::vector<std::unique_ptr<sf::TcpSocket>> clients;
    sf::TcpListener listener;
    std::unique_ptr<sf::TcpSocket> socket = std::make_unique<sf::TcpSocket>();

    while (clients.size() < maxClients && listener.accept(*socket) == sf::Socket::Done)
    {
        clients.push_back(std::move(socket));
        std::cout << "new guy connected" << std::endl;
       
        if (clients.size() < maxClients)
            socket = std::make_unique<sf::TcpSocket>();
    }
}

It's not a good idea to create 100 sockets and then potentially fill them with new connections. Instead you should just add new connections when they actually happen.

[Tytan]

Thank you so much!
I love you right now!