[GeorgeMcReary]

Quote:

Originally Posted by SiaReyes Kane was right, if you had read the whole script, you could have spotted some issues. Mysql connection wasn't close at OnGameModeExit. Player Name is stored in database but no idea why it was not used in script instead using GetPlayerName everytime! `Users` and `USERS` should be `users` in onplayerconnect and OnPlayerDisconnect callback! There is no need to login after the player register. It can be done by pawn Code: forward OnPlayerRegister(playerid); public OnPlayerRegister(playerid) {          pInfo[playerid][MasterID] = cache_insert_id();          pInfo[playerid][PX] = 1527.85;          pInfo[playerid][PY] = -1675.35;          pInfo[playerid][PZ] = 13.3828;          pInfo[playerid][ROOT] = 270;          pInfo[playerid][Skin] = 1;          pInfo[playerid][Level] = 1;     SetPlayerScore(playerid, pInfo[playerid][Level]);     SetSpawnInfo(playerid, 0, pInfo[playerid][Skin], pInfo[playerid][PX], pInfo[playerid][PY], pInfo[playerid]         [PZ],pInfo[playerid][Rot], 0, 0, 0, 0, 0, 0);     SendClientMessage(playerid, 0x0033FFFF /*Blue*/, "Thank you for registering! You will be spawned now!");     TogglePlayerSpectating(playerid, false);     TogglePlayerControllable(playerid, true);         SpawnPlayer(playerid);     return 1; } And the last thing is , No Login detection was checked when player disconnect and no player variable for login check is defined. When player disconnect, you should check if player logged in (after login/register) , so the data will be saved in mysql. What if a player logins with an another player name and exist the server (Calls OnPlayerDisconnect)? or what if a player mistype his password and kicked out? It will save the player's current position and will be stored into the database under the player name. So, that's why the script needs a login variable. Anyways Good work!

MySQL is not case sensitive lmao.. USERS and users and uSeRs are considered same. About login, Its nothing wrong. A lot of servers and even lots of websites asks for login after register. Doesnt directly logs them in. It depends on everyone's style hahah..

About login variable, I agree with you.... There should be a Login Check variable so that there is no issue.

about closing connection, I think when server is not active, it will kill off the connection automatically... but doesnt hurt adding it there.

[GTLS]

Quote:

Originally Posted by SiaReyes Kane was right, if you had read the whole script, you could have spotted some issues. Mysql connection wasn't close at OnGameModeExit. Player Name is stored in database but no idea why it was not used in script instead using GetPlayerName everytime! `Users` and `USERS` should be `users` in onplayerconnect and OnPlayerDisconnect callback! There is no need to login after the player register. It can be done by pawn Code: forward OnPlayerRegister(playerid); public OnPlayerRegister(playerid) {          pInfo[playerid][MasterID] = cache_insert_id();          pInfo[playerid][PX] = 1527.85;          pInfo[playerid][PY] = -1675.35;          pInfo[playerid][PZ] = 13.3828;          pInfo[playerid][ROOT] = 270;          pInfo[playerid][Skin] = 1;          pInfo[playerid][Level] = 1;     SetPlayerScore(playerid, pInfo[playerid][Level]);     SetSpawnInfo(playerid, 0, pInfo[playerid][Skin], pInfo[playerid][PX], pInfo[playerid][PY], pInfo[playerid]         [PZ],pInfo[playerid][Rot], 0, 0, 0, 0, 0, 0);     SendClientMessage(playerid, 0x0033FFFF /*Blue*/, "Thank you for registering! You will be spawned now!");     TogglePlayerSpectating(playerid, false);     TogglePlayerControllable(playerid, true);         SpawnPlayer(playerid);     return 1; } And the last thing is , No Login detection was checked when player disconnect and no player variable for login check is defined. When player disconnect, you should check if player logged in (after login/register) , so the data will be saved in mysql. What if a player logins with an another player name and exist the server (Calls OnPlayerDisconnect)? or what if a player mistype his password and kicked out? It will save the player's current position and will be stored into the database under the player name. So, that's why the script needs a login variable. Anyways Good work!

Quote:

Originally Posted by GeorgeMcReary MySQL is not case sensitive lmao.. USERS and users and uSeRs are considered same. About login, Its nothing wrong. A lot of servers and even lots of websites asks for login after register. Doesnt directly logs them in. It depends on everyone's style hahah.. About login variable, I agree with you.... There should be a Login Check variable so that there is no issue. about closing connection, I think when server is not active, it will kill off the connection automatically... but doesnt hurt adding it there.

I over looked some details but I have fixed them now. Thank you guys for the appreciation and criticism. Criticism makes someone always better.

1> Fixed connection not closed and added saving data OnGameModeExit too.
2> Added Login check- Thanks to SiaReyes. I knew I was forgetting something.. thanks for reminding me..
3> Well yeah, about login thing... Its my way of showing Login dialog after registering. Everyone's way is different as GeorgeMcReary said..
4> Player name is saved in database for login purposes. I dont save them from database on a variable because it looks stupid to me. Again, everyone's way is different. Someone saves them into variable each time player logs in, someone uses GetPlayerName.. none methods are wrong.

No harm done. I always accept criticism, and take it positively.

[SiaReyes]

Quote:

Originally Posted by GTLS 4> Player name is saved in database for login purposes. I dont save them from database on a variable because it looks stupid to me. Again, everyone's way is different. Someone saves them into variable each time player logs in, someone uses GetPlayerName.. none methods are wrong.

Yeah, you got a point, everyone has their own way of scripting. The another method, you could use a simple function that gets the player name with playerid!

Quote:

Originally Posted by GeorgeMcReary MySQL is not case sensitive lmao.. USERS and users and uSeRs are considered same. About login, Its nothing wrong. A lot of servers and even lots of websites asks for login after register. Doesnt directly logs them in. It depends on everyone's style hahah..

Yeah, but some CNR/DM servers just have register. Anyhow, it doesn't matter if we use register with login! (Editable)

[Calisthenics]

Quote:

Originally Posted by GeorgeMcReary MySQL is not case sensitive lmao.. USERS and users and uSeRs are considered same.

Linux is, it will return error for table not found.

https://dev.mysql.com/doc/refman/8.0...nsitivity.html

Quote:

Originally Posted by GTLS I over looked some details but I have fixed them now. Thank you guys for the appreciation and criticism. Criticism makes someone always better.

Quote:

Originally Posted by GTLS No harm done. I always accept criticism, and take it positively.

About users.sql
1) Set `Name` column as UNIQUE KEY.
2) Bcrypt returns 60 characters but you used VARCHAR(255). Length is fixed so better use CHAR(60)
3) Column `Level` is misleading.
4) When a value of column is always between a small range, use different type of integer (MEDIUMINT, SMALLINT, TINYINT)

About Base.pwn
1) Standalone foreach is very outdated, YSI 5 and y_iterate.
2) Pass handle parameter in mysql_errno function because if a filterscript uses mysql and no duplicated connections, it will use handle 1 as default.
3) In OnPlayerConnect, you select all just to see if there are rows. Either COUNT() aggregate function or select password and id there and not in dialog response.
4)

pawn Code:

mysql_tquery(handle, query, "OnPlayerLogin", "ds", playerid, inputtext);

If log level is set as ALL, it will log password as plain text in mysql.log file.
5)

pawn Code:

"SELECT password, Master_ID from `USERS` WHERE Name LIKE '%s'"

Remove LIKE keyword.
6) Be consistent about table and column names. Read my reply to GeorgeMcReary as a reference.
7) `MasterID` column is PRIMARY KEY, LIMIT keyword is obsolete.
8 ) Check against race condition.

[GTLS]

Quote:

Originally Posted by Calisthenics Linux is, it will return error for table not found. https://dev.mysql.com/doc/refman/8.0...nsitivity.html About users.sql 1) Set `Name` column as UNIQUE KEY. 2) Bcrypt returns 60 characters but you used VARCHAR(255). Length is fixed so better use CHAR(60) 3) Column `Level` is misleading. 4) When a value of column is always between a small range, use different type of integer (MEDIUMINT, SMALLINT, TINYINT) About Base.pwn 1) Standalone foreach is very outdated, YSI 5 and y_iterate. 2) Pass handle parameter in mysql_errno function because if a filterscript uses mysql and no duplicated connections, it will use handle 1 as default. 3) In OnPlayerConnect, you select all just to see if there are rows. Either COUNT() aggregate function or select password and id there and not in dialog response. 4) pawn Code: mysql_tquery(handle, query, "OnPlayerLogin", "ds", playerid, inputtext); If log level is set as ALL, it will log password as plain text in mysql.log file. 5) pawn Code: "SELECT password, Master_ID from `USERS` WHERE Name LIKE '%s'" Remove LIKE keyword. 6) Be consistent about table and column names. Read my reply to GeorgeMcReary as a reference. 7) `MasterID` column is PRIMARY KEY, LIMIT keyword is obsolete. 8 ) Check against race condition.

A) Why did I use 255 Chars? To make it future proof. In documentation of PHP's password_hash(), they said that its better to use 255 characters because password_default changes its value to the strongest hashing available in PHP. Most users use password_default while using PHP. So, in case they create a UCP and use password_default and something better than bcrypt comes up, 255 will make it future proof. User will not have to make any changes to his database.

Quote:

Note that this constant is designed to change over time as new and stronger algorithms are added to PHP. For that reason, the length of the result from using this identifier can change over time. Therefore, it is recommended to store the result in a database column that can expand beyond 60 characters (255 characters would be a good choice).

B) Column Level is fine by me. If people want to use other names like score or something, they can.

C) I used standalone foreach because I didnt want to include whole YSI just so I can use one function from it. When people start using this, they can update on their preference or can continue using standalone..

D) What do you mean by race condition?

Other changes have been done in gamemode and database. Files uploaded. Thanks..!

[Calisthenics]

Quote:

Originally Posted by GTLS D) What do you mean by race condition?

I will quote maddinat0r:

pawn Code:

/*  race condition check:
    player A connects -> SELECT query is fired -> this query takes very long
    while the query is still processing, player A with playerid 2 disconnects
    player B joins now with playerid 2 -> our laggy SELECT query is finally finished, but for the wrong player
    what do we do against it?
    we create a connection count for each playerid and increase it everytime the playerid connects or disconnects
    we also pass the current value of the connection count to our OnPlayerDataLoaded callback
    then we check if current connection count is the same as connection count we passed to the callback
    if yes, everything is okay, if not, we just kick the player
*/

Almost none of the released gamemodes check against it but it is quite important.

Quote:

Originally Posted by GTLS Other changes have been done in gamemode and database. Files uploaded. Thanks..!

You forgot to add the index. Without an index, it does full table scan but with it it scans only 1 row. Thanks.

[GTLS]

Quote:

Originally Posted by Calisthenics I will quote maddinat0r: pawn Code: /*  race condition check:     player A connects -> SELECT query is fired -> this query takes very long     while the query is still processing, player A with playerid 2 disconnects     player B joins now with playerid 2 -> our laggy SELECT query is finally finished, but for the wrong player     what do we do against it?     we create a connection count for each playerid and increase it everytime the playerid connects or disconnects     we also pass the current value of the connection count to our OnPlayerDataLoaded callback     then we check if current connection count is the same as connection count we passed to the callback     if yes, everything is okay, if not, we just kick the player */ Almost none of the released gamemodes check against it but it is quite important.

Okay, I will research about it..

Quote:

Originally Posted by Calisthenics You forgot to add the index. Without an index, it does full table scan but with it it scans only 1 row. Thanks.

Index where?

[Calisthenics]

Quote:

Originally Posted by GTLS Index where?

`Name` column.

[GTLS]

Quote:

Originally Posted by Calisthenics `Name` column.

Oh I am sorry for that but I did add the unique to the sql file inside the release zip.. I guess I forgot to change it in the source file which you looked.. I did now.

[Mugala]

fits to a name xd, gj.