SA-MP Forums

Go Back   SA-MP Forums > SA-MP Scripting and Plugins > Gamemode Scripts

Reply
 
Thread Tools Display Modes
Old 05/08/2019, 04:09 PM   #21
GeorgeMcReary
Big Clucker
 
Join Date: May 2015
Posts: 185
Reputation: 9
Default Re: Very Basic Script

Quote:
Originally Posted by SiaReyes View Post
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.
__________________
GeorgeMcReary is offline   Reply With Quote
Old 05/08/2019, 04:27 PM   #22
GTLS
High-roller
 
GTLS's Avatar
 
Join Date: Aug 2014
Location: India
Posts: 1,048
Reputation: 115
Default Re: Very Basic Script

Quote:
Originally Posted by SiaReyes View Post
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 View Post
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.
__________________

I don't help for rep. I help cuz I was helped in the past.


Retired from SAMP Scene. Might open SAMP Forums once in a while.

Some treats for ya. Click em
Basic House System
Simple Speedo
Base Login/Register Script

GTLS is offline   Reply With Quote
Old 05/08/2019, 04:35 PM   #23
SiaReyes
Huge Clucker
 
SiaReyes's Avatar
 
Join Date: Mar 2019
Location: Coordinates X,Y,Z
Posts: 317
Reputation: 22
Default Re: Very Basic Script

Quote:
Originally Posted by GTLS View Post
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 View Post
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)
__________________
Join Darkside Roleplay : 144.217.19.104:7777

Quote:
Originally Posted by Toroi View Post
blackmail people that join your server using their ip address as hostage

or get a job and play sa-mp for fun
SiaReyes is offline   Reply With Quote
Old 05/08/2019, 05:16 PM   #24
Calisthenics
Gangsta
 
Join Date: May 2018
Posts: 680
Reputation: 111
Default Re: Very Basic Script

Quote:
Originally Posted by GeorgeMcReary View Post
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 View Post
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 View Post
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.
Calisthenics is offline   Reply With Quote
Old 06/08/2019, 06:59 AM   #25
GTLS
High-roller
 
GTLS's Avatar
 
Join Date: Aug 2014
Location: India
Posts: 1,048
Reputation: 115
Default Re: Very Basic Script

Quote:
Originally Posted by Calisthenics View Post
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..!
__________________

I don't help for rep. I help cuz I was helped in the past.


Retired from SAMP Scene. Might open SAMP Forums once in a while.

Some treats for ya. Click em
Basic House System
Simple Speedo
Base Login/Register Script

GTLS is offline   Reply With Quote
Old 06/08/2019, 08:04 AM   #26
Calisthenics
Gangsta
 
Join Date: May 2018
Posts: 680
Reputation: 111
Default Re: Very Basic Script

Quote:
Originally Posted by GTLS View Post
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 View Post
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.
Calisthenics is offline   Reply With Quote
Old 06/08/2019, 08:58 AM   #27
GTLS
High-roller
 
GTLS's Avatar
 
Join Date: Aug 2014
Location: India
Posts: 1,048
Reputation: 115
Default Re: Very Basic Script

Quote:
Originally Posted by Calisthenics View Post
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 View Post

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?
__________________

I don't help for rep. I help cuz I was helped in the past.


Retired from SAMP Scene. Might open SAMP Forums once in a while.

Some treats for ya. Click em
Basic House System
Simple Speedo
Base Login/Register Script

GTLS is offline   Reply With Quote
Old 06/08/2019, 11:59 AM   #28
Calisthenics
Gangsta
 
Join Date: May 2018
Posts: 680
Reputation: 111
Default Re: Very Basic Script

Quote:
Originally Posted by GTLS View Post
Index where?
`Name` column.
Calisthenics is offline   Reply With Quote
Old 07/08/2019, 04:47 PM   #29
GTLS
High-roller
 
GTLS's Avatar
 
Join Date: Aug 2014
Location: India
Posts: 1,048
Reputation: 115
Default Re: Very Basic Script

Quote:
Originally Posted by Calisthenics View Post
`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.
__________________

I don't help for rep. I help cuz I was helped in the past.


Retired from SAMP Scene. Might open SAMP Forums once in a while.

Some treats for ya. Click em
Basic House System
Simple Speedo
Base Login/Register Script

GTLS is offline   Reply With Quote
Old 08/08/2019, 11:21 PM   #30
Mugala
High-roller
 
Mugala's Avatar
 
Join Date: Nov 2012
Location: Georgia, Tbilisi
Posts: 1,065
Reputation: 50
Default Re: Very Basic Script

fits to a name xd, gj.
__________________
Coding in 8 languages including Pawn, C#, C++ and SQL.
Available for hiring.

If you have a coding question or need an assistance in a code, just PM me.
If you want me to create something for you, here is a Discord Mugala#5651

Currently helping Community members in a coding.
Mugala is offline   Reply With Quote
Reply

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
[GameMode] Gang War - Basic script Matnix Gamemode Scripts 19 10/08/2017 08:55 AM
Basic 'Skeleton' Rp script JamesStryker Scripting Help 3 16/08/2013 08:23 PM
Script error - Please help, really basic. Kyle. Scripting Help 11 19/11/2011 06:11 AM
Help with very basic script cazz21 Help Archive 4 20/05/2011 12:33 PM
[Request] Basic Script Matthew_Murdoch Help Archive 3 27/08/2009 03:17 PM


All times are GMT. The time now is 01:10 AM.


Powered by vBulletin® Version 3.8.6
Copyright ©2000 - 2019, Jelsoft Enterprises Ltd.