Indy TCPServer disconnecting Clients

of course it's wrong

you need to implement a server side mechanism for that. basically, you need to assign each connection a username so that when you look for a username to kcick, yuo will find its conneciton.

one idea is like this: http://www.ciuly.com/delphi/indy/indyTcpClientServerDemo.zip
you will see that the client must send a login command and when it does, it's name is coralated with it's connection.
then when you send the kick, you will just send a command like
Kick username
then look for the thread of the username and then do thread.connection.disconnect;

and that's all.

no, the structure of the command kick is already done, what i need is some function to  disconnect a client from the server... since the usernames of all clients are already in a listbox, i just need this to make it work:

for k:= 0 to listbox1.items.count do
begin
if listbox1.items.strings[i]=user then // some procedure or function to disconnect this client
end;

received messsage would be :'kick someone' and user would be just 'someone'

right?? Well i GUESS this should work =\

you don't understand: you need to get the connection associated with the username. until you don't do that, you cannot kick the user. simple as that.

you have a username. you need a connection. you need a relation between the username and the connection. one such relation is given by the TUsers class from the demo above. but you need such a a relation, on the server. until that ... you're stuck.

you don't need to change the kick command. I was only giving an example. you MUST make a relation between a username and a conneciton.

and when I say connection, thatv can be a TIdTcpConnection or a TIdPeerThread (which has a connection property of type TIdTcpConnection).

there is no other way..

one more thing, maybe that is easier for you.

IF, the listbox is on the server (!!!!), then, when you add the user to the listbox you also add the connection./thread. something like

instead of listbox1.items.add(username) you wiil do
listbox1.items.adObject(username, connection or thread).

then, you can do

for k:= 0 to listbox1.items.count do
begin
if listbox1.items.strings[i]=user then // some procedure or function to disconnect this client
  TIdPeerThread(listbox1.items.objects[i]).connection.disconnect;
-or-
  TIdTcpConnection(listbox1.items.objects[i]).disconnect;
end;
depending what you added with addobject.

THAT, is a relation between a username and the connection. but again, you need such a relation on the server (not the client, since the server needs to do the disconnect).

I tryied to do that and nothing happened =\

    else if sametext(c, CKick) then
      begin
      for k:=0 to ListBox1.Items.Count do
        begin
        if ListBox1.Items.Strings[i]=s then
          begin
          serverMassCommand(CMessage+' '+s+' have been kicked!');
          TIdPeerThread(ListBox1.Items.Objects[i]).Connection.Disconnect;
          end;
        end;
      end

and on Kick button:

clientSendCommand(CKick+' '+edit3.Text);

what is wrong with this code?

well, first of all, you did not confirmed that you have used the addobject when the client logged in. that could be the problem.
second, you didn't mention where exactly you put that whole else if sametext. I can only assume you placed it right, but then again, I've seen people doing a logical thing to do completly illogical. usually because they didn't understand the initial explanations :) so feel free to ask as many questions as you think will make things clear to you.

but, I really don't understand why you are not using my implementation I have given you. I've choosen that because it is simple and easy to extend.
I see just now that the demo posted in my first post was also posted in a previous question of yours. I am sorry but I have a short memory when it comes to usernames :) so I didn't see that it was you whom I wrote it for in the first place.

for example, in order to add kick functionality to my demo all you have to do is:
- add a popupmenu with a kick menu item or a button witha kick caption.
- in the onclick event do
  if listbox1.ItemIndex=-1 then // you want to make this test just to be on the safe side ;)
    exit;
  clientSendCommand(CKick+' '+listbox1.items[listbox1.ItemIndex]);
- if you want you can have the user type the username in an editbox but that is ... ugly :) it's nicer if you just click the user and then click on kick, no? :P anyway, that is minor detail. it's up to you how you do it.
- then in the processServerCommand you do
    else if sametext(c, CKick) then
      processServerKickCommand(AThread, s)
- and you write the above processServerKickCommand procedure like:
procedure TForm1.processServerKickCommand(thread: TIdPeerThread;
  cmd: string);
var t:TIdPeerThread;
begin
  // in our case, cmd is the username. we can extend this to also contain a reason.
  t:=u.find(cmd);
  if t<>nil then
  begin
    serverSendCommand(t, CError+' you have been kicked by '+u.UserNames[thread]);
    t.Connection.Disconnect;
  end;
end;

and you're done.

it is very important to keep generic procedures, generic. as I;ve done, I have written anotehr procedure to deal with the kick command and not wrote the logic of the kick command in the logic of the process command. it is a matter of design and will of course help you in the future, plus it keeps the code nice and tidy. so you should try and keep this coding standard :) it's in your best interest ;)

the TUsers class I wrote povides you with a bidirectional relation between a username and a connection. just as I explained, you can find the connection of a user by it's username.
And you don't have to keep it in the listbox, since you already have it in the TUsers.

now, we need to straiten up a few things. you have one application acting as both server and client. this can be confusing especially for people who do not understand client-server applications too well.
so you might get tricked into believing that the lsitbox belongs to the server application, which is wrong. the listbox belongs to the client application and keeping the server connections in the client listbox is a logical error that will byte you back one day. Initially, I thought that yuo have a server and that server has the connected users in a listbox. there are such applications. but it's not your case.

it would be a good exercise for you to split the aplication into 2 separate applications: a client and a server. but it's your choise. paying good attention to details, allows one to write a client-server application having only one application.

in conclusion, I hope you understand the need of the server part of the application to know the relation between usernames and connections in order to perform any operation on a connection based on the username.

Hmm i guessi got it ciuly... before you post i have already made some ugly sh*ts to imitate kick command, i get the message like any other in the client and check if (messagereceived - (CMessage + ' ')) has the string ('kick ' + username), if yes then client.disconnect...

As i said, it's ugly :p

I'll try your way and then i'll post back here ;)

Ok, it works perfectly, but the online list (listbox1) only get refreshed for the Host... the other clients get the listbox1 with the name that have been kicked... I know the listbox has nothing to do with the Server, but thats what's happening...

Should i add:

u.delete(t)

in TForm1.processServerKickCommand??

no. there is one small bug in the hole thing. I fixed it and updated the demo on the site (the demo has a menu item for cicking, so just rightclick a user and kick). basically what needs to be changed is:
- in declaration procedure serverMassCommand(cmd:string; notTo:TIdPeerThread=nil);// add the second parameter
- in implementation of procedure TForm1.serverMassCommand(cmd: string; notTo:TIdPeerThread);// add second parameter
    for i:=1 to c.Count do
      if TIdPeerThread(c[i-1])<>notTo then // add this line
        serverSendCommand(TIdPeerThread(c[i-1]), cmd);
- in procedure TForm1.serverDisconnect(AThread: TIdPeerThread);
    serverMassCommand(CDisconnect+' '+s, AThread);// add second parameter

the idea behind this is that in the massmessage a command was sent to all threads. but when you kick somebody, that thread was not yet taken out from the list, so we were sending him too the message, which raised an indy exception (which was later consumed by the indy framework) and so none of the clients connected after the kicked client got the message.

you can test this by opening 4 times the executable, and you will have 4 clients. kick the 3rd client. you will see that the first 2 receive the message and the listbox is updated, but the last one does not :)

the above should work fine. I also modified the way error messages are handled in the client since it's not a good idea to have 2 message boxes displayed in teh same time (like when you kicked somebody, the kick message and the disconnect message)

Hmm, I tried do modify this on the code but no changes, guess i'm doing something wrong...

Can you pass me the link again plz? i tried the link above but it's not correct =\

got it ;)

thanks ciuly!!

Just for curiosity, how long did you take to discover this bug??

10-15 minutes? something like that. why?

LoL, i guess even if i passed 4 hrs in front of the computer i wouldnt find it!!

Well, it works PERFECTLY!!

Thanks ciuly ;)

you'r welcome :)