Raised This Month: $12 Target: $400
 3% 

CSS BRush


Post New Thread Reply   
 
Thread Tools Display Modes
RedSword
SourceMod Plugin Approver
Join Date: Mar 2006
Location: Quebec, Canada
Old 01-05-2013 , 01:15   Re: CSS BRush
Reply With Quote #81

Hi there,

Some things should be changed for approval :
  • You shouldn't duplicate your code. You're killing timers in two different ways (code consistency; L562 versus stock in L1516)
  • Only one of your timer that is using a client Id is safe. The rest is unsafe way (i.e. DisplayMenu to a client not in game ).

Beside that, you should remove FCVAR_REPLICATED from version ConVar, as well as FCVAR_NOTIFY from NON-version convar. A post from Peace-Maker explaining why.

Code:
CloseHandle(hRandom);
CloseHandle on ConVar's handle is useless and fails silently (asherkin @ irc).

Code:
MenuTime = (3 + FreezeTime) / 2; //L1610
MenuTime = (4 + FreezeTime) / 2; //L253
Is this really wanted ?

Code:
// KyleS HATES Handles
Good thing ;).

Otherwise, seems alright.
__________________
My plugins :
Red Maze
Afk Bomb
RAWR (per player/rounds Awp Restrict.)
Kill Assist
Be Medic

You can also Donate if you appreciate my work

Last edited by RedSword; 01-05-2013 at 19:35.
RedSword is offline
TnTSCS
AlliedModders Donor
Join Date: Oct 2010
Location: Undisclosed...
Old 01-05-2013 , 17:54   Re: CSS BRush
Reply With Quote #82

Thanks for the feedback... I'll go through the code and respond

...:: TnT Edit ::...

Okay, went through your suggestions.

1. Fixed the timer issue, now using the stock
2. Removed the FCVAR flags you identified.
3. Removed the CloseHandle(hRandom);
4. The MenuTime = ... may not be needed... I can maybe just kill the menu being displayed to the one player OnRoundStart if they haven't picked their teammates by then... I'll look into changing it - however, they have been made to be consistent now, instead of being different.
5. I close and set the ClientTimer[MAXPLAYERS+1] to INVALID_HANDLE OnClientDisconnect, so I thought the way I was handling timers was safe and didn't have to use UserIDs and could stick with ClientIDs.

I've made some changes since the version on this thread and will post it soon.
__________________
View my Plugins | Donate

Last edited by TnTSCS; 01-05-2013 at 18:13.
TnTSCS is offline
TnTSCS
AlliedModders Donor
Join Date: Oct 2010
Location: Undisclosed...
Old 01-05-2013 , 18:26   Re: CSS BRush
Reply With Quote #83

Here's the update plugin...

Changelog:
Code:
* 0.0.2.3	*	Fixed the freezing of players
* 
* 0.0.2.4	+	Added fast round restart option with configurable time
* 
* 0.0.2.5	*	Code adjustment from RedSword's comments in forums.
* 		-	Removed set_random_model to make this CS:GO capable
I'm not going to publish this on the OP until I've tested it more... especially on CS:GO

...:: TnT Edit ::...
I removed the attachment until it's ready for prime time - don't want people running bunk code
__________________
View my Plugins | Donate

Last edited by TnTSCS; 01-05-2013 at 22:01.
TnTSCS is offline
RedSword
SourceMod Plugin Approver
Join Date: Mar 2006
Location: Quebec, Canada
Old 01-05-2013 , 19:35   Re: CSS BRush
Reply With Quote #84

Quote:
Originally Posted by TnTSCS View Post
Here's the update plugin...

Changelog:
Code:
* 0.0.2.3	*	Fixed the freezing of players
* 
* 0.0.2.4	+	Added fast round restart option with configurable time
* 
* 0.0.2.5	*	Code adjustment from RedSword's comments in forums.
* 		-	Removed set_random_model to make this CS:GO capable
I'm not going to publish this on the OP until I've tested it more... especially on CS:GO
When I was talking about FCVAR_NOTIFY, I meant that for NON-version convar. You just removed the possibility to track your plugin... (sorry; I misplaced a comma that could make you misunderstand my point).

Also, it is still used a LOT of time in your new post's attachment...

EDIT :

Also, in your new version, on line L654, client's value has no use. And your code is still prone to errors with timers, as it's possible to print to chat to a client not connected.
__________________
My plugins :
Red Maze
Afk Bomb
RAWR (per player/rounds Awp Restrict.)
Kill Assist
Be Medic

You can also Donate if you appreciate my work

Last edited by RedSword; 01-05-2013 at 19:43.
RedSword is offline
TnTSCS
AlliedModders Donor
Join Date: Oct 2010
Location: Undisclosed...
Old 01-05-2013 , 22:00   Re: CSS BRush
Reply With Quote #85

I see now... not sure what my fingers were typing when I put FCVAR_NOTIFY in all of those CVars... LoL - fixed... and I see what you mean about a few of the timers having the possibility to print to chat to a client not connected - issues with Timer_AddToWaitingList... I fixed it by using userid instead of client, then validating if that userid is still connected before continuing.

I actually forgot about all of those enhancements I added... but like I said, I won't publish it on the OP until I've tested it.

But what I will do is go through the OP code and clean it up and post a good version for approval, then I'll work on the update. Thanks for taking the time to review and comment - I really appreciate it
__________________
View my Plugins | Donate
TnTSCS is offline
TnTSCS
AlliedModders Donor
Join Date: Oct 2010
Location: Undisclosed...
Old 01-06-2013 , 12:44   Re: CSS BRush
Reply With Quote #86

Updated original code on first post. I think I've addressed all of your concerns. Any timer using ClientTimer[MAXPLAYERS+1] should be safe, since OnClientDisconnect, I ClearTimer the ClientTimer[client], so if a timer is set to run with a ClientID, if they disconnect, it will kill that timer.

I'll work on my enhancements and post at a later date, ensuring I pay attention to the things you pointed out.
__________________
View my Plugins | Donate

Last edited by TnTSCS; 01-06-2013 at 12:47.
TnTSCS is offline
RedSword
SourceMod Plugin Approver
Join Date: Mar 2006
Location: Quebec, Canada
Old 01-07-2013 , 04:56   Re: CSS BRush
Reply With Quote #87

Your use of the timer is still prone to errors.

i.e. 505 & 520+...

With almost every function that uses a player, you should be sure that the player is in game/connected (depending on the case), and if not, check if they are in game. When using timers, you're not "in the known". The client may have quit.
__________________
My plugins :
Red Maze
Afk Bomb
RAWR (per player/rounds Awp Restrict.)
Kill Assist
Be Medic

You can also Donate if you appreciate my work
RedSword is offline
TnTSCS
AlliedModders Donor
Join Date: Oct 2010
Location: Undisclosed...
Old 01-07-2013 , 12:16   Re: CSS BRush
Reply With Quote #88

okay... but just so I know for sure:

L505 has
PHP Code:
ClientTimer[client] = CreateTimer(0.1Timer_HandleTeamSwitchclient); 
Then I have
PHP Code:
public OnClientDisconnect(client)
{
    if (
IsClientInGame(client))
    {
        
ClearTimer(ClientTimer[client]);
    }
}

ClearTimer(&Handle:timer)
{
    if (
timer != INVALID_HANDLE)
    {
        
KillTimer(timer);
        
timer INVALID_HANDLE;
    }     

So I thought if the client disconnects before the timer fires, the timer would be cancelled... is that incorrect thinking? Is it the possibility of a frame or two if the client disconnects and the timer still fires?

Just trying to fully understand is all - nothing else... because almost all of my plugin operate in this fashion regarding ClientTimer[client] = CreateTimer passing client along with the timer.
__________________
View my Plugins | Donate
TnTSCS is offline
TnTSCS
AlliedModders Donor
Join Date: Oct 2010
Location: Undisclosed...
Old 01-07-2013 , 12:31   Re: CSS BRush
Reply With Quote #89

In discussing this on IRC, it's been hammered into me that:

a) I should be using UserIDs (or better client serials)
b) I should validate clients inside timer calls due to the remote possibility of a client disconnecting the same time a timer fires?

I'll go through this plugin and the rest of my plugins and fix the timer segments of all of them.

Again, thanks for the feedback, I appreciate it and I learned something new
__________________
View my Plugins | Donate
TnTSCS is offline
RedSword
SourceMod Plugin Approver
Join Date: Mar 2006
Location: Quebec, Canada
Old 01-07-2013 , 16:00   Re: CSS BRush
Reply With Quote #90

Quote:
Originally Posted by TnTSCS View Post
okay... but just so I know for sure:

L505 has
PHP Code:
ClientTimer[client] = CreateTimer(0.1Timer_HandleTeamSwitchclient); 
Then I have
PHP Code:
public OnClientDisconnect(client)
{
    if (
IsClientInGame(client))
    {
        
ClearTimer(ClientTimer[client]);
    }
}

ClearTimer(&Handle:timer)
{
    if (
timer != INVALID_HANDLE)
    {
        
KillTimer(timer);
        
timer INVALID_HANDLE;
    }     

So I thought if the client disconnects before the timer fires, the timer would be cancelled... is that incorrect thinking? Is it the possibility of a frame or two if the client disconnects and the timer still fires?

Just trying to fully understand is all - nothing else... because almost all of my plugin operate in this fashion regarding ClientTimer[client] = CreateTimer passing client along with the timer.
My bad, sorry, I might have reviewed too quickly. You're right; if you kill a timer on disconnect you shouldn'T have any problem.

There is like ~2 good ways to handle the situation. Killing the timer is one, and using userId is the other. UserIds are generally prefered, as they can be fired and forgotten; i.e. no need for a global variable). I think your plugin is fine as it is; I'll have to review it (once) again.

I just thought that from the 2nd revision you would correct it using UserIds rather than killing timer; hence my mistake.

Red
__________________
My plugins :
Red Maze
Afk Bomb
RAWR (per player/rounds Awp Restrict.)
Kill Assist
Be Medic

You can also Donate if you appreciate my work

Last edited by RedSword; 01-07-2013 at 16:25.
RedSword is offline
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

Forum Jump


All times are GMT -4. The time now is 11:44.


Powered by vBulletin®
Copyright ©2000 - 2024, vBulletin Solutions, Inc.
Theme made by Freecode