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

Weapon Lights * Update 0.7.1 Beta (26-Feb-2009)


Post New Thread Reply   
 
Thread Tools Display Modes
Arkshine
AMX Mod X Plugin Approver
Join Date: Oct 2005
Old 02-06-2009 , 06:29   Re: Weapon Lights * Updated v0.6 (03-Feb-2009)
Reply With Quote #21

Like what ?
Arkshine is offline
Hawk552
AMX Mod X Moderator
Join Date: Aug 2005
Old 02-06-2009 , 10:15   Re: Weapon Lights * Updated v0.6 (03-Feb-2009)
Reply With Quote #22

Detagging this is unnecessary:
Code:
enum _:Coord_e{ Float:x, Float:y, Float:z };

Instead, just tag the variable that uses it.

This is extra code that will be loaded into memory which is unnecessary:
Code:
return (get_pdata_int (i_WeaponIndex, m_iSilencerFireMode, EXTRAOFFSET_WEAPONS ) & M4A1_SILENCED);

Instead, make a variable which defaults to the get_pdata_int() call, then in the switch statement, run the & operator on it with M4A1_SILENCED or USP_SILENCED. You can then simply return that value after the switch or add a default statement which returns 0.

Code:
#define MAX_WEAPONS 30 #define NOT_GUNS_BITSUM (1 << CSW_HEGRENADE | 1 << CSW_SMOKEGRENADE | 1 << CSW_FLASHBANG | 1 << CSW_KNIFE | 1 << CSW_C4)
v

Bad form. Define everything at the top.

Code:
gi_Red

If you consider this Hungarian notation, it is bad form. The proper form would be:

Code:
g_iRed

This would be similar for all other variables.

Code:
    RegisterHamsPrimaryAttack ();

This is unnecessary redirection. Just run this function in plugin_init() since it's only called once. A C++ compiler would optimize this out by inlining it but the Pawn compiler is stupid and won't. You may also consider removing the call to CacheCvarsValue() since it will be called a few seconds after plugin_init() anyway on round start.

Code:
    switch (get_pdata_int (i_WeaponIndex, m_iWeaponType, EXTRAOFFSET_WEAPONS))     {     case CSW_M4A1 : return (get_pdata_int (i_WeaponIndex, m_iSilencerFireMode, EXTRAOFFSET_WEAPONS ) & M4A1_SILENCED);     case CSW_USP : return (get_pdata_int (i_WeaponIndex, m_iSilencerFireMode, EXTRAOFFSET_WEAPONS ) & USP_SILENCED);     case CSW_TMP : return 1;     }     return 0;

Bad indentation. Indent the case statements by one more tab.

All-in-all, this plugin is pretty well done. I noticed the indentation in the previous problem and automatically assumed it probably had other problems, but reading over it again I would have put "This plugin is well done."
__________________
Hawk552 is offline
Send a message via AIM to Hawk552
Arkshine
AMX Mod X Plugin Approver
Join Date: Oct 2005
Old 02-06-2009 , 12:56   Re: Weapon Lights * Updated v0.6 (03-Feb-2009)
Reply With Quote #23

Quote:
Detagging this is unnecessary:
You're right. Don't know why I've always do that.

Quote:
Instead, make a variable which defaults to the get_pdata_int() call, then in the switch statement, run the & operator on it with M4A1_SILENCED or USP_SILENCED. You can then simply return that value after the switch or add a default statement which returns 0.
Could you explain the benefit ? I mean using if() elseif() structure it would be normal, but using a switch I don't understand well. [ Edit ] : I did not read this part "This is extra code that will be loaded into memory which is unnecessary:" <== For few bytes it doesn't matter, plus I would prefer to return directly the value instead of creating a var each time. Also, caching the value like you ask you will call it for all weapons when you need to make it only for m4/usp. So, I think your suggestion is wrong. If I did not understand, could you give more informations , please ?

Quote:
Bad form. Define everything at the top.
Since the plugin is 'short', I was thinking that it doesn't matter and more readable to see the #define corresponding the function. But I understand that it's not bad to keep the form.

Quote:
If you consider this Hungarian notation, it is bad form. The proper form would be:
It's my own notation. Not Hungarian. Or at least a very very simplified Hungarian version ; just applying for some vars. Is it something wrong ? It's always the same. If the offset name are not the same is because I prefer to keep the original name for that.

Quote:
This is unnecessary redirection. Just run this function in plugin_init() since it's only called once. A C++ compiler would optimize this out by inlining it but the Pawn compiler is stupid and won't.
Same question, what is the benefit to avoid calls ? You say unnecessary, but it's called one time. Here I would prefer readability against optimization. Though I still don't understand what is the optimization here.

Quote:
Bad indentation. Indent the case statements by one more tab.
In my original code all in well-indented. Shame on HiSS. ^^


Thanks for your explanations, Hawk552.

Last edited by Arkshine; 02-06-2009 at 13:25.
Arkshine is offline
Hawk552
AMX Mod X Moderator
Join Date: Aug 2005
Old 02-06-2009 , 13:18   Re: Weapon Lights * Updated v0.6 (03-Feb-2009)
Reply With Quote #24

Quote:
Originally Posted by arkshine View Post
Could you explain the benefit ? I mean using if() elseif() structure it would be normal, but using a switch I don't understand well.
The benefit of what I'm talking about is that less memory would be used because there are fewer possible instructions. Your question isn't very clear and I don't know if you're asking me to explain what a switch is or simply how to do it in this case, but here's what I meant:

PHP Code:
    new Offset get_pdata_int (i_WeaponIndexm_iSilencerFireModeEXTRAOFFSET_WEAPONS )
    switch (
get_pdata_int (i_WeaponIndexm_iWeaponTypeEXTRAOFFSET_WEAPONS))
    {
    case 
CSW_M4A1 : return Offset M4A1_SILENCED);
    case 
CSW_USP : return Offset USP_SILENCED);
    case 
CSW_TMP : return 1;
    } 
On review, though, this actually adds an unnecessary instruction if it's a TMP. Since I've read the code properly now, I'd say the best way to do it would be something like this:

PHP Code:
GetWeaponSilen (const i_WeaponIndex)
{
    return 
get_pdata_int (i_WeaponIndexm_iWeaponTypeEXTRAOFFSET_WEAPONS) == CSW_TMP || get_pdata_int (i_WeaponIndexm_iSilencerFireModeEXTRAOFFSET_WEAPONS ) & M4A1_SILENCED|USP_SILENCED

This may not work if the silencer mode is randomly set if it's not an M4A1 or USP. It should, though.

Quote:
Originally Posted by arkshine View Post
Since the plugin is 'short', I was thinking that it doesn't matter and more readable to see the #define corresponding the function. But I understand that it's not bad to keep the form.
Rules always apply no matter how big or small a plugin is.

Quote:
Originally Posted by arkshine View Post
It's my own notation. Not Hungarian. Or at least a very very simplified Hungarian version ; just applying for some vars. Is it something wrong ? It's always the same. If the offset name are not the same is because I prefer to keep the original name for that.
Hungarian notation was made very specifically to be as useful as possible. Messing around with it like you have is just kinda weird because people who both are or aren't familiar with HN will both be confused. From all the scripts I've seen, there are only 3 styles that are legible in terms of HN:
  • Full HN - I used to do this but got out of it because it's really ugly and useless for Pawn.
  • Partial HN - This is what I would recommend. This involves using only some tags, such as g_ or p_. You should never tag floats, ints or bools if you're using partial HN. It is sometimes acceptable to tag strings.
  • No HN - This sometimes make your code difficult to read because globals are not very obvious.
Quote:
Originally Posted by arkshine View Post
Same question, what is the benefit to avoid calls ? You say unnecessary, but it's called one time. Here I would prefer readability against optimization. Though I still don't understand what is the optimization here.
The way you have it now, it has the overhead of an extra, unnecessary function call. This uses up more CPU cycles and memory. If you want it to be readable, just comment it. As I said, a good C++ compiler will inline it but Pawn has no such capability.
__________________
Hawk552 is offline
Send a message via AIM to Hawk552
Arkshine
AMX Mod X Plugin Approver
Join Date: Oct 2005
Old 02-06-2009 , 13:43   Re: Weapon Lights * Updated v0.6 (03-Feb-2009)
Reply With Quote #25

Thanks for your clarification.

Though, about the switch, I would prefer to use it and without caching the value because your way it will be called for all weapons when it needs to be called only for m4/usp. Extra-code for more cpu against memory ; i would prefer the second solution.

About the HN, I'm using a partial HN too, though with my own style. I will try to not overuse this notation but it will hard to break this habit. ( more readable for me to see the variable type fastly ).

Anyway, thanks again.

( btw, keep up the good work, you're more sexy this way. ^^ )

Last edited by Arkshine; 02-06-2009 at 13:48.
Arkshine is offline
mcardo
Junior Member
Join Date: Jan 2009
Old 02-07-2009 , 19:06   Re: Weapon Lights * Updated v0.6 (03-Feb-2009)
Reply With Quote #26

Installed it right now and it is a great realistic effect but...

1. The light color resets to "red" after round ends or map changes.
2. When you press the "fire" key and the gun runs out of ammo while still holding the "fire" key, the light remains "on" just like a flashlight.

It'd be better if the color I set in my cfg file remained constant and the fire light would flash strictly when the weapon is being fired and not when you hold the "fire" key only.

Thanks!
mcardo is offline
Squallkins
AlliedModders Donor
Join Date: Mar 2005
Old 02-07-2009 , 21:20   Re: Weapon Lights * Updated v0.6 (03-Feb-2009)
Reply With Quote #27

Will there be any option to define admin flag in future version?



Squall.
Squallkins is offline
HiSS
Member
Join Date: Nov 2008
Location: Celtic Gallaecia
Old 02-09-2009 , 11:07   Re: Weapon Lights * Updated v0.6 (03-Feb-2009)
Reply With Quote #28

Ok thank you for improvements, sorry I couldn't answer before I was very bussy. I'll try to update it next version, but I found a bug/issue:

If you're using a pistol, and your clip is not empty, if you shoot one bullet and still doing +attack (pressing mouse1, or whatever you have binded to), the light still drawing ultil you release mouse1 (-attack).
It doesn't happen if the clip is empty or if you use a sniper or another gun that automatically shoot next bullet.

I tried to fix it using read_data(3) and it seems to work fine but sometimes the first shot after reloading/switching the gun doesn't make the light.

Any suggestion?

Thanks and sorry my bad english.
__________________
[IMG]http://img227.**************/img227/3148/celeritasetsubtilitasuw0.jpg[/IMG]
HiSS is offline
Hawk552
AMX Mod X Moderator
Join Date: Aug 2005
Old 02-09-2009 , 11:11   Re: Weapon Lights * Updated v0.6 (03-Feb-2009)
Reply With Quote #29

Post this in scripting help. I only give small fixes and suggestions.
__________________
Hawk552 is offline
Send a message via AIM to Hawk552
HiSS
Member
Join Date: Nov 2008
Location: Celtic Gallaecia
Old 02-09-2009 , 11:11   Re: Weapon Lights * Updated v0.6 (03-Feb-2009)
Reply With Quote #30

Quote:
Originally Posted by mcardo View Post
Installed it right now and it is a great realistic effect but...

1. The light color resets to "red" after round ends or map changes.
2. When you press the "fire" key and the gun runs out of ammo while still holding the "fire" key, the light remains "on" just like a flashlight.
Hi,

1 - You should use quotes after and before the values:

"0 0 255" > blue
"255 0 0" > red
"0 255 0" > green
etc.

2 - That should already be solved in v0.6, read previous post it's a bug only happens if you're using a pistol and your clip is not empy.

Greetings.
__________________
[IMG]http://img227.**************/img227/3148/celeritasetsubtilitasuw0.jpg[/IMG]
HiSS 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 08:08.


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