KickPlayer problem

Started by Fjose, Dec 11, 2014, 02:57 AM

Previous topic - Next topic

Fjose

Kicking player and nothing happens, incredibly this code was working perfectly on 0.3z version

function KickPlr( id )
{
local player = FindPlayer( id );
if ( player )
{
if ( pinfo[id].Logged == true ) SavePlayerData( id );
pinfo[id] = null;
local blah = collectgarbage();
print( blah );
players.rawdelete(id );
KickPlayer( player );
}
}




.

#1
Let's go down and evaluate things incrementally (line by line)


Let's change the argument name into identifier because it'll make more sense in the next line.

function KickPlr(identifier)


local player = FindPlayer(id);
The problem with dynamically typed languages (like Squirrel) is that you cannot trust the contents of a variable. The function FindPlayer() expects either an integer or a string (which is why I've changed the name of the argument because it can be either id or name which can both be considered identifiers because they identify a player on the server). Anything else will result in an exception. Therefore you need to do some checking on the passed argument before submitting it to that function.

Which is why we make that line of code into two lines actually:
if (typeof identifier != "integer" && typeof identifier != "string") return; // We have no business here
// We know we have a valid identifier so it's safe to pass it to the function
local player = FindPlayer(identifier);



We know FindPlayer() returns either a player instance or null.
if (player)But why not be on the safe side in case the function behaviour will change in in future versions.
if (typeof player == "instance")


You people really need to make some sense when naming your variables by applying some prefixes to them.
if ( pinfo[id].Logged == true ) SavePlayerData( id );Usually people prefix global variables with "g" or just a simple underscore "_". Anything to differentiate the from local variables.

Either one of the following does the job:
// Prefixed with g_ and a capital first letter to know it's a global variable
if (g_Pinfo[id].Logged == true) SavePlayerData(identifier);
// Prefixed with _ and a capital first letter to know it's a global variable
if (_Pinfo[id].Logged == true) SavePlayerData(identifier);

But it's a bit to late for you  to change your mind now and adopt a new style.



This one makes sense.
pinfo[id] = null;


DA FUQ ARE YOU DOING HERE? The module is compiled with reference counting enabled. This function only makes sense for builds that use garbage collection instead of reference counting.
local blah = collectgarbage();Why do you think you're doing this here?
pinfo[id] = null;You're doing it to release that reference and allow the instance to be deleted if there aren't any more references left.

Because the above doesn't make sense then this one doesn't make either.
print( blah );


DA FUQ ARE YOU DOING HERE TOO? Why do you remove that array element and effectively decreasing the array size by 1. You're just making a time bomb for later to explode in your face.
players.rawdelete(id );This line is enough and it's the only thing you need to do.
pinfo[id] = null;


This line makes sense because if you reached here you definitely have a valid player instance and the function should do as instructed.
KickPlayer(player);


This is your time bomb right here. This will only lead you to kicking other players because when you remove an array element you effectively reduce the array size which causes the players to shift places where players above end up in the slot bellow them.
players.rawdelete(id );
EXAMPLE:

Let's say you have an array like the following:

MyArray:
  • Player ID 1
  • Player ID 2
  • Player ID 3
  • Player ID 4
  • Player ID 5
  • Player ID 6

You see how nicely the array indexes match with the player ID? Because that's what you're counting on. For them to match so that you can identify the player in the array using it's ID. Now let's REMOVE (NOT DISABLE) Player 3 (because that's what the line above does).

MyArray:
  • Player ID 1
  • Player ID 2
  • Player ID 4
  • Player ID 5
  • Player ID 6

You see what just happened? You see where the players above 3 are now positioned? They shifted back 1 element and they're indexes no longer match the player ID.

Now let's say you want to kick player with the ID 4. You would expect player with ID 4 to be at the array element number 4. But you're WRONG. The array elements are now shifted by that removal and you'll just kick the player with the ID 5 instead.



Let's just leave the above code run a while longer until 4-5 more players connect and disconnect. What do you think will happen when you want to access the player with ID 5? What do you think is going to be the answer of that array?

MyArray:
  • I'm empty bruh... I can't give you what's at index 5 because there is no index 5 anymore. You removed them. Remember? One by one.



Let's gather all of what we learned and have a clean safe version:

function KickPlr(identifier)
{
if (typeof identifier != "integer" && typeof identifier != "string") return;
local player = FindPlayer(identifier);
if (typeof player == "instance")
{
if (pinfo[id].Logged == true) SavePlayerData(id);
pinfo[id] = null;
KickPlayer(player);
}
}

If you prefer the version where code is much more linear and without nesting:
function KickPlr(identifier)
{
if (typeof identifier != "integer" && typeof identifier != "string") return;
local player = FindPlayer(identifier);
if (typeof player != "instance") return;
if (pinfo[id].Logged == true) SavePlayerData(id);
pinfo[id] = null;
KickPlayer(player);
}

Whichever one you prefer.
.

Fjose

#2
I was eaten alive lol
1) I understood, collectgarbage is useless
2) "g" "_" for global variables, totally useless when I already know what are the global variables
3) KickPlr was only identifying the player.ID, I'll update it, ok hh.
4) then, what do you think it is the best idea if a player leaves or is kicked with the players table.. if I don't clean the data then this will continue summing until collapse?  is supposed that I need clean the data if a new player joins
5) of where was taken out the "instance"?

.

#3
Quote from: Fjose on Dec 11, 2014, 04:03 PM4) then, what do you think it is the best idea if a player leaves or is kicked with the players table.. if I don't clean the data then this will continue summing until collapse?  is supposed that I need clean the data if a new player joins

When the player disconnects:
my_array[index] = null;
That's enough to remove any previous data. It's just a signal that you no longer use that slot/index/element of the array.

When the player connects:
my_array[index] = my_class();

Now you signal that you wan't to use that slot/index/element of the array again. Keeping your array size constant and reducing overhead or resizing the array.

NOTE: Do not mistake "=" (assignment) with "<-" (instancing). This is a Squirrel concept that's really f***t up and sometimes confusing to new scriptwriters.

Quote from: Fjose on Dec 11, 2014, 04:03 PM"g" "_" for global variables, totally useless when I already know what are the global variables

Let's say that you that you have this global variable named "info" and the following code happens:
info <- 3;

function test()
{
local info = 0;
print(info);
}

function test2(info)
{
print(info);
}

test();
test2(1);

What do you think will be the output of that program? Do you think it will be 33? WRONG! the output will be 01, because you had no idea in the function body that you overloaded the global one. You think you know but it's easy to make mistakes. Not only that you have a more descriptive code but you're also protecting you're self from such mistakes.


Quote from: Fjose on Dec 11, 2014, 04:03 PMof where was taken out the "instance"?

Even though you use OOP you still don't have the slightest idea of what an instance is. I might write a short guide about classes and OOP some day but you really need to document a little about the concept.
.

Fjose

Quote from: S.L.C on Dec 11, 2014, 04:20 PMWhen the player disconnects:
my_array[index] = null;
When the player connects:
my_array[index] = my_class();
Now you signal that you wan't to use that slot/index/element of the array again. Keeping your array size constant and reducing overhead or resizing the array.

I was talking about the "players" table not about an array. about your examples everybody must know it lol (if is using an array), I have that in my functions but the question is, what will happen with the player when this is kicked? if you are saying that the players table must be removed from kickplr?

then I'll be only cleaning the data when players leaves but not when are kicked? I guess it you are confused or forgetting something

Quote from: S.L.C on Dec 11, 2014, 04:20 PMWhat do you think will be the output of that program? Do you think it will be 33? WRONG! the output will be 01, because you had no idea in the function body that you overloaded the global one. You think you know but it's easy to make mistakes. Not only that you have a more descriptive code but you're also protecting you're self from such mistakes.

I know it

Quote from: S.L.C on Dec 11, 2014, 04:20 PM
Quote from: Fjose on Dec 11, 2014, 04:03 PMof where was taken out the "instance"?

Even though you use OOP you still don't have the slightest idea of what an instance is. I might write a short guide about classes and OOP some day but you really need to document a little about the concept.

just explain me now '-'
my skills are limited, what I learned, was reading the scripts created by others lol

.

#5
Quote from: Fjose on Dec 11, 2014, 05:16 PMI was talking about the "players" table not about an array. about your examples everybody must know it lol (if is using an array), I have that in my functions but the question is, what will happen with the player when this is kicked? if you are saying that the players table must be removed from kickplr?

then I'll be only cleaning the data when players leaves but not when are kicked? I guess it you are confused or forgetting something

I have no idea what you mean here :-\ Can you please be more specific and clear with that question?

Quote from: Fjose on Dec 11, 2014, 05:16 PMjust explain me now '-'

I don't have the time to do that now but I remember having to do the same thing a year ago for someone else in PHP. Just replace the word object with instance because they (almost) mean the same thing essentially. You could find that simple description here but keep in mind that it's just a simple example that doesn't cover Polymorphism and all the other OOP concepts. It's just a raw simple explanation of classes and objects/instances of classes.


Quote from: Fjose on Dec 11, 2014, 05:16 PMmy skills are limited, what I learned, was reading the scripts created by others lol

No wonder you don't have a naming style or understand the importance of a naming conventions for a programmer and how much does it help.
.

Fjose

#6
Quote from: S.L.C on Dec 11, 2014, 05:46 PM
Quote from: Fjose on Dec 11, 2014, 05:16 PMI was talking about the "players" table not about an array. about your examples everybody must know it lol (if is using an array), I have that in my functions but the question is, what will happen with the player when this is kicked? if you are saying that the players table must be removed from kickplr?

then I'll be only cleaning the data when players leaves but not when are kicked? I guess it you are confused or forgetting something

I have no idea what you mean here :-\ Can you please be more specific and clear with that question?

I talking about the players.rawdelete( id ) and you said:

Quote from: S.L.C link=msg=658This is your time bomb right here. This will only lead you to kicking other players because when you remove an array element you effectively reduce the array size which causes the players to shift places where players above end up in the slot bellow them.

players.rawdelete( id );

if I can't delete data when the player is kicked, what would be the best option? when player leaves or is kicked, I have to clean that global varible. an example: when players joins I use players.rawset( player.ID, player ), then if the player leaves and I use a cmd to check that player of following way.

foreach( id, plr in players )
{
   if ( plr == text )
   {
       // do something
   }
}
// do something if result is true

result always will be true with the player if is or not connected, until that someone join with the same id and then the data will change.

Note: About the instance, your class is different to the mine xD

I fixed the problem with a temporal way.

Locked.