[yRemootz]

Originally Posted by JusTGo Updated: -V2.5 * Rewritten the code to make it clearer. * Added a tracking cvar. I guess it's ready for a review now.

I think you should comment out your code to explain what each line does. Just to be more explained.
But, great plugin!

[JusTGo]

Originally Posted by yRemootz I think you should comment out your code to explain what each line does. Just to be more explained. But, great plugin!

I have already commented on the parts that need to be explained, the rest can be easily figured by variable and function names, which part doesn't seem clear to you?

[iceeedr]

Do not use magic numbers "114, 96"

PHP Code:

#define getUserTeam(%1)    get_pdata_int(%1, 114)
#define isC4(%1)    (get_pdata_int(%1, 96) & (1<<8)) 


[JusTGo]

Originally Posted by iceeedr Do not use magic numbers "114, 96" PHP Code: #define getUserTeam(%1)    get_pdata_int(%1, 114) #define isC4(%1)    (get_pdata_int(%1, 96) & (1<<8))

IMO it's fine if I'm using them only once, and the define name is pretty descriptive but I can change them if that's really needed.

[iceeedr]

What I told you is exactly what will be said when your plugin is analyzed, but feel free to leave it that way.

[JusTGo]

Originally Posted by iceeedr What I told you is exactly what will be said when your plugin is analyzed, but feel free to leave it that way.

If a reviewer asks for it then I will do it, even though I think it's fine to use it this way as I'm using these numbers only once, also in amx1.9+ there are new natives that use game data files that could be used to avoid using the numbers in the plugin but some people might be still using amx1.8.2 and it's still the official stable release.

[HamletEagle]

Originally Posted by JusTGo If a reviewer asks for it then I will do it, even though I think it's fine to use it this way as I'm using these numbers only once, also in amx1.9+ there are new natives that use game data files that could be used to avoid using the numbers in the plugin but some people might be still using amx1.8.2 and it's still the official stable release.

It's absolutely NOT fine. It doesn't matter that you are using it only once. Quick, without googling anything tell me what this line does:

PHP Code:

get_pdata_cbase(id, 373, 5) 


You can't, even tho I used it only once. What in the world is 373? What is this supposed to do? You don't know unless you just so happen to know that offset 373 is m_pActiveItem. Now, look at the following line:

PHP Code:

get_pdata_cbase(id, m_pActiveItem, XO_PLAYER) 


You know instantly what it does. A number doesn't say anything, but a name does.

Not having magic values in your plugin is a requirement for approval, for a very good reason. Doing otherwise is asking for unmaintainable, unreadable and crappy code.
Use 1.8.2, it's fine, in fact it's even recommended if it can be reasonably done as it is still the latest stable release. Just name things properly, it's not hard.

[JusTGo]

Originally Posted by HamletEagle It's absolutely NOT fine. It doesn't matter that you are using it only once. Quick, without googling anything tell me what this line does: PHP Code: get_pdata_cbase(id, 373, 5)  You can't, even tho I used it only once. What in the world is 373? What is this supposed to do? You don't know unless you just so happen to know that offset 373 is m_pActiveItem.

That's true but in my code, there is a clear context which is the defined name which is IMO pretty descriptive and straightforward.

PHP Code:

#define getUserTeam(%1)    get_pdata_int(%1, 114) 


Anyone who reads getUserTeam will immediately know that 114 is related to a user team member.

I lean towards making descriptive names in my coding style instead of writing more code/comments, but I will update it in case someone finds this confusing.

-V2.6
* Minor changes to improve readability.

[HamletEagle]

Yes, I know it gets the users team by looking at getUserTeam. But maybe I want to understand HOW it does it. Maybe I want to check regamedll and see where offset 114 is used and how it keeps track of teams.
But I can't, because I don't know what offset 114 is.

[JusTGo]

Originally Posted by HamletEagle Yes, I know it gets the users team by looking at getUserTeam. But maybe I want to understand HOW it does it. Maybe I want to check regamedll and see where offset 114 is used and how it keeps track of teams. But I can't, because I don't know what offset 114 is.

True, you are right.

A few more changes I guess can be made:

PHP Code:

#define isC4(%1)    (get_pdata_int(%1, m_bIsC4) & (1<<8)) 


to:

PHP Code:

#define isC4(%1)    get_pdata_bool(%1, m_bIsC4) 


edit: Actually get_pdata_bool is not defined in amx1.8.2... I have to drop the support for it If I do that

Also a new function for getting dmgtime as I'm using it twice:

PHP Code:

bool:aboutToExplode(ent)
{
    static Float:dmgtime
    pev(ent, pev_dmgtime, dmgtime)
    
    // It's going to explode :D
    return (dmgtime <= get_gametime())
}