AlliedModders

AlliedModders (https://forums.alliedmods.net/index.php)
-   Scripting Help (https://forums.alliedmods.net/forumdisplay.php?f=11)
-   -   How to improve this code? (https://forums.alliedmods.net/showthread.php?t=243277)

pob18 07-02-2014 07:35

How to improve this code?
 
I got this code that reads steam ids from a config file.
After that when you plant/defuse the bomb, the plugin will check the steamids stored in the cfg and print in chat a message.

Like:
your steamidxxx -> [tag/etc] Player planted the bomb
non steamidxxx from -cfg -> Player planted the bomb...

But what if I want to use this for other functions? I always need to copy/paste the same function?
Isn't this a mistake?

How to improve this?

PHP Code:

#include <amxmodx>
#include <amxmisc>
#include <cstrike>
#include <csx>

#include <colorchat>

new Trie:g_tAuthIdOfPeople
#define FILE    "/Steams.cfg"
#define cm(%0)  ( sizeof(%0) - 1 )  

public plugin_init()
{
    
g_tAuthIdOfPeople TrieCreate()
    
ReadFile()
}

public 
ReadFile()
{
    new 
szFilePath128 ]
    
get_configsdirszFilePathcmszFilePath ) )
    
addszFilePathcmszFilePath ), FILE )

    new 
fopenszFilePath"rt" )
    
    if( !
)
    {
        new 
szMessage256 ]
        
formatexszMessagecmszMessage ), "Unable to open %s"szFilePath )
        
set_fail_stateszMessage )
    }
    
    new 
szData128 ]
    new 
szAuthID35 ]
    new 
szDummy]
    
    while( !
feof) )
    {
    
fgetsfszDatacmszData ) )
        
    if( !
szData] || szData] == ';' || szData] == '/' && szData] == '/' )
    {
        continue
    }

    
trimszData )
    
parseszDataszAuthIDcmszAuthID ), szDummycmszDummy ) )
        
    
TrieSetCellg_tAuthIdOfPeopleszAuthID)
        
    }
    
    
fclose)
}

public 
bomb_planted(index)
{
    new 
szAuthID35 ]; get_user_authidindexszAuthIDcmszAuthID ) )
    new 
iDummy;
    
    new 
iPlayers32 ], iNumiPlayers;
    
get_playersiPlayersiNum"ceh""TERRORIST" )
    
    for( 
0iNumi++ )
    {
    
Players iPlayers];
        if( 
TrieGetCellg_tAuthIdOfPeopleszAuthIDiDummy ) )
        {
        
ColorChat(PlayersGREEN"[tag/etc] %s ^1planted the bomb"get_nick(index) )
        }   
        
        else
        
ColorChat(PlayersGREEN"%s ^1planted the bomb"get_nick(index) )
    }
}

public 
bomd_defused(index)
{
                    
// Paste the code below again? This is not the best way right?
                    // How to improve?
}

get_nick( const index )
{
    new 
szName32 ]; get_user_nameindexszNamecmszName ) );
    return 
szName;



Arkshine 07-02-2014 08:59

Re: How to improve this code?
 
If you want to call the same code again, you just need to create a private function with the shared code, and calling it in both forward.

And by the way:

Trim() should be called before the check.
TrieGetCell() -> TrieKeyExists ; since you don't need associated value.
Call TrieDestroy() on plugin_end() forward.
set_fail_state() can format string, but not sure in what AMXX version it's available.

HamletEagle 07-02-2014 10:03

Re: How to improve this code?
 
Quote:

Originally Posted by Arkshine (Post 2160793)
If you want to call the same code again, you just need to create a private function with the shared code, and calling it in both forward.

And by the way:

Trim() should be called before the check.
TrieGetCell() -> TrieKeyExists ; since you don't need associated value.
Call TrieDestroy() on plugin_end() forward.
set_fail_state() can format string, but not sure in what AMXX version it's available.

Quote:

Originally Posted by ConnorMcLeod (Post 1844750)
Tries don't need to be destroyed at plugin_end, it is automatic.

So,who is wrong ? I just want to know the correct way.

Arkshine 07-02-2014 11:43

Re: How to improve this code?
 
With 1.8.2, It's designed like: at map change the handle system is well freed, but not the actual allocated memory from plugin datas, which will be reused (or extented) if needed. In others words, if you allocate x bytes from plugin A, and this plugin is disabled on next map, you will have x bytes allocated for nothing.

With 1.8.3, the trie natives are no more based on KTrie library but on Hashmap instead, the whole system is different and this time, memory should be freed properly at each map change.

Still, it's a good practice to free always when you can something you have previously allocated.
I would recommend to use always in this context TrieDestroy in plugin_end().

Backstabnoob 07-02-2014 12:10

Re: How to improve this code?
 
I don't know what you're doing within the loop in bomb_planted, but it's wrong.


All times are GMT -4. The time now is 21:17.

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