[RevCreww]

HeLLo EveryOne

This is My First Time I Make Post, Hope I Follow Rules

Description
This Is Classic Plugin For (Awp Menu) + (Knife Menu)

For Open Menu
Awp Menu = say /awp
Knife Menu = say /knife

[+ARUKARI-]

I looked at awp.sma and this is code that fits in 170 lines.

[HamletEagle]

What in the world is happening inside AwpCase? That code is nightmare material. Same with CurentWeapon. One could rewrite your 900 lines of code in around 100 and have this plugin more customizable, allowing an arbitrary number of models.

I'm sorry, but this clearly shows you need to learn more before releasing plugins. Take your time, learn and post some quality work. If you would like help or details about how to improve your code please ask me or feel free to post in scripting help. Note that even if you improve the code, this will not be approved because we have other plugins wich are not hardcoded and much more customizable.

[RevCreww]

Originally Posted by HamletEagle What in the world is happening inside AwpCase? That code is nightmare material. Same with CurentWeapon. One could rewrite your 900 lines of code in around 100 and have this plugin more customizable, allowing an arbitrary number of models. I'm sorry, but this clearly shows you need to learn more before releasing plugins. Take your time, learn and post some quality work. If you would like help or details about how to improve your code please ask me or feel free to post in scripting help. Note that even if you improve the code, this will not be approved because we have other plugins wich are not hardcoded and much more customizable.

ok, i realy need help for learn

[HamletEagle]

Good code should always be modular and easy to extend/change. In your case you took the same code you would have used for one AWP model and copy-pasted it 22 times. But what if you wanted to have 50 AWP models? Or 100? Would you have copy-pasted the same code 100 times? The answer is no, you can't simply keep copy-pasting the same code over and over again. You need to think of a modular approach, something that can dynamically support any number of models, without you having to copy and paste the same code.

Let's look at your AwpX arrays and observer, again, that your approach required you to create 22 of them. That's not good code, so let's figure out a way around that.
What if, instead of creating 22 individual arrays you would create just one, 2-dimensional array like so:

PHP Code:

new bool:AWP[33][22] 


Notice how I merged all of them in one single array. Now, if you decide you want 50 models all you have to do is replace 22 by 50, much easier than adding 28 more lines to the source code and creating an unreadable and unmaintainable mess.

But how would you use it this way? AWP[id] refers to the awp models that the player with the index "id" has. Now, if you wanted to check if this player has the first awp you would use AWP[id][0] which would be equivalent to your AWP1 array.
This is better, but still not great. Notice that at any moment the player can have only one model so only one of these booleans can be true. Therefore you don't need one for each model. Turns out you can just do:

PHP Code:

new AWP[33] 


Notice I removed the bool tag because now we aren't going to use bools anymore, but integers. If you set AWP[id] to 0 it means the player has the first awp, if AWP[id] is 1 it means he has the second AWP, ..., and if it is 21 it means it has the last awp model.

Now, you can rewrite AwpCase to be just:

PHP Code:

public AwpCase(id, menu, item)
{
    if(item == MENU_EXIT)
    {
        return 1;
    }

    AWP[id] = item;

    menu_destroy (menu);
    return 1;
} 


Notice how much useless code we were able to remove just by designing our approach carefully. Also, in this case, you don't need to use the whole menu_getinfo, it is enough to use the item parameter. Plus, there is no need to specify "1", "2", etc in menu_additem.

Similarly, you can reduce all this:

PHP Code:

new const Awp1Model[66] = "models/v_awp.mdl";
new const Awp2Model[66] = "models/RevCreww/awp1.mdl";
new const Awp3Model[66] = "models/RevCreww/awp2.mdl";
new const Awp4Model[66] = "models/RevCreww/awp3.mdl";
new const Awp5Model[66] = "models/RevCreww/awp4.mdl";
new const Awp6Model[66] = "models/RevCreww/awp5.mdl";
new const Awp7Model[66] = "models/RevCreww/awp6.mdl";
new const Awp8Model[66] = "models/RevCreww/awp7.mdl";
new const Awp9Model[66] = "models/RevCreww/awp8.mdl";
new const Awp10Model[66] = "models/RevCreww/awp9.mdl";
new const Awp11Model[66] = "models/RevCreww/awp10.mdl";
new const Awp12Model[66] = "models/RevCreww/awp11.mdl";
new const Awp13Model[66] = "models/RevCreww/awp12.mdl";
new const Awp14Model[66] = "models/RevCreww/awp13.mdl";
new const Awp15Model[66] = "models/RevCreww/awp14.mdl";
new const Awp16Model[66] = "models/RevCreww/awp15.mdl";
new const Awp17Model[66] = "models/RevCreww/awp16.mdl";
new const Awp18Model[66] = "models/RevCreww/awp17.mdl";
new const Awp19Model[66] = "models/RevCreww/awp18.mdl";
new const Awp20Model[66] = "models/RevCreww/awp19.mdl";
new const Awp21Model[66] = "models/RevCreww/awp20.mdl";
new const Awp22Model[66] = "models/RevCreww/awp21.mdl"; 


Into this:

PHP Code:

new const AWPModels[][] =
{
    "models/v_awp.mdl",
    "models/RevCreww/awp1.mdl",
    ....,
    "models/RevCreww/awp21.mdl"
} 


AWPModels[0] would be the equivalent of your Awp1Model, AWPModels[1] would be the equivalent of Awp2Model and so on.

With this change you can simply plugin_precache to just:

PHP Code:

public plugin_precache()
{
    for(new i; i < sizeof AWPModels; i++)
    {
        precache_model(AWP[i])
    }
} 


Since all your models are in the same array you can just iterate over it and precache every model in there, one by one with a for loop.

And we can keep going and also fix your CurentWeapon function.

PHP Code:

public CurentWeapon(id)
{
    if(get_user_weapon(id) == CSW_AWP)
    {
        set_pev(id, pev_viewmodel2, AWPModels[AWP[id]])
    }
} 


Recall that AWP[id] gives you the index of the current AWP and it turns out you can index AWPModels(where you have the awp models stored) by this value to retrieve the correct model.
For example, if in the menu a player pressed "1" then AwpCase would execute and item will be 0. Then you would also store 0 inside AWP[id](AWP[id] = 0). AWPModels[AWP[id]] will be equivalent to AWPModels[0] and this will give you "models/v_awp.mdl".

One more thing, instead of using CurWeapon please look into Ham_ItemDeploy, you can find plenty of examples around here.

Hope this is helpful for you. There are more things that can be improved and I tried to keep the modifications I made relatively simple so you can compare to what you have right now and figure out the thought process behind these changes. Future improvements could be to use a dynamic array to allow an unlimited number of models, reading the models from a file and adding them to the array so people don't need to edit the code & recompile to add more models, etc.

[RevCreww]

Originally Posted by HamletEagle Good code should always be modular and easy to extend/change. In your case you took the same code you would have used for one AWP model and copy-pasted it 22 times. But what if you wanted to have 50 AWP models? Or 100? Would you have copy-pasted the same code 100 times? The answer is no, you can't simply keep copy-pasting the same code over and over again. You need to think of a modular approach, something that can dynamically support any number of models, without you having to copy and paste the same code. Let's look at your AwpX arrays and observer, again, that your approach required you to create 22 of them. That's not good code, so let's figure out a way around that. What if, instead of creating 22 individual arrays you would create just one, 2-dimensional array like so: PHP Code: new bool:AWP[33][22]  Notice how I merged all of them in one single array. Now, if you decide you want 50 models all you have to do is replace 22 by 50, much easier than adding 28 more lines to the source code and creating an unreadable and unmaintainable mess. But how would you use it this way? AWP[id] refers to the awp models that the player with the index "id" has. Now, if you wanted to check if this player has the first awp you would use AWP[id][0] which would be equivalent to your AWP1 array. This is better, but still not great. Notice that at any moment the player can have only one model so only one of these booleans can be true. Therefore you don't need one for each model. Turns out you can just do: PHP Code: new AWP[33]  Notice I removed the bool tag because now we aren't going to use bools anymore, but integers. If you set AWP[id] to 0 it means the player has the first awp, if AWP[id] is 1 it means he has the second AWP, ..., and if it is 21 it means it has the last awp model. Now, you can rewrite AwpCase to be just: PHP Code: public AwpCase(id, menu, item) {     if(item == MENU_EXIT)     {         return 1;     }     AWP[id] = item;     menu_destroy (menu);     return 1; }  Notice how much useless code we were able to remove just by designing our approach carefully. Also, in this case, you don't need to use the whole menu_getinfo, it is enough to use the item parameter. Plus, there is no need to specify "1", "2", etc in menu_additem. Similarly, you can reduce all this: PHP Code: new const Awp1Model[66] = "models/v_awp.mdl"; new const Awp2Model[66] = "models/RevCreww/awp1.mdl"; new const Awp3Model[66] = "models/RevCreww/awp2.mdl"; new const Awp4Model[66] = "models/RevCreww/awp3.mdl"; new const Awp5Model[66] = "models/RevCreww/awp4.mdl"; new const Awp6Model[66] = "models/RevCreww/awp5.mdl"; new const Awp7Model[66] = "models/RevCreww/awp6.mdl"; new const Awp8Model[66] = "models/RevCreww/awp7.mdl"; new const Awp9Model[66] = "models/RevCreww/awp8.mdl"; new const Awp10Model[66] = "models/RevCreww/awp9.mdl"; new const Awp11Model[66] = "models/RevCreww/awp10.mdl"; new const Awp12Model[66] = "models/RevCreww/awp11.mdl"; new const Awp13Model[66] = "models/RevCreww/awp12.mdl"; new const Awp14Model[66] = "models/RevCreww/awp13.mdl"; new const Awp15Model[66] = "models/RevCreww/awp14.mdl"; new const Awp16Model[66] = "models/RevCreww/awp15.mdl"; new const Awp17Model[66] = "models/RevCreww/awp16.mdl"; new const Awp18Model[66] = "models/RevCreww/awp17.mdl"; new const Awp19Model[66] = "models/RevCreww/awp18.mdl"; new const Awp20Model[66] = "models/RevCreww/awp19.mdl"; new const Awp21Model[66] = "models/RevCreww/awp20.mdl"; new const Awp22Model[66] = "models/RevCreww/awp21.mdl";  Into this: PHP Code: new const AWPModels[][] = {     "models/v_awp.mdl",     "models/RevCreww/awp1.mdl",     ....,     "models/RevCreww/awp21.mdl" }  AWPModels[0] would be the equivalent of your Awp1Model, AWPModels[1] would be the equivalent of Awp2Model and so on. With this change you can simply plugin_precache to just: PHP Code: public plugin_precache() {     for(new i; i < sizeof AWPModels; i++)     {         precache_model(AWP[i])     } }  Since all your models are in the same array you can just iterate over it and precache every model in there, one by one with a for loop. And we can keep going and also fix your CurentWeapon function. PHP Code: public CurentWeapon(id) {     if(get_user_weapon(id) == CSW_AWP)     {         set_pev(id, pev_viewmodel2, AWPModels[AWP[id]])     } }  Recall that AWP[id] gives you the index of the current AWP and it turns out you can index AWPModels(where you have the awp models stored) by this value to retrieve the correct model. For example, if in the menu a player pressed "1" then AwpCase would execute and item will be 0. Then you would also store 0 inside AWP[id](AWP[id] = 0). AWPModels[AWP[id]] will be equivalent to AWPModels[0] and this will give you "models/v_awp.mdl". One more thing, instead of using CurWeapon please look into Ham_ItemDeploy, you can find plenty of examples around here. Hope this is helpful for you. There are more things that can be improved and I tried to keep the modifications I made relatively simple so you can compare to what you have right now and figure out the thought process behind these changes. Future improvements could be to use a dynamic array to allow an unlimited number of models, reading the models from a file and adding them to the array so people don't need to edit the code & recompile to add more models, etc.

Wow Tnx bro You Help soo much i understand now
Tnx Man

[RevCreww]

Originally Posted by HamletEagle Good code should always be modular and easy to extend/change. In your case you took the same code you would have used for one AWP model and copy-pasted it 22 times. But what if you wanted to have 50 AWP models? Or 100? Would you have copy-pasted the same code 100 times? The answer is no, you can't simply keep copy-pasting the same code over and over again. You need to think of a modular approach, something that can dynamically support any number of models, without you having to copy and paste the same code. Let's look at your AwpX arrays and observer, again, that your approach required you to create 22 of them. That's not good code, so let's figure out a way around that. What if, instead of creating 22 individual arrays you would create just one, 2-dimensional array like so: PHP Code: new bool:AWP[33][22]  Notice how I merged all of them in one single array. Now, if you decide you want 50 models all you have to do is replace 22 by 50, much easier than adding 28 more lines to the source code and creating an unreadable and unmaintainable mess. But how would you use it this way? AWP[id] refers to the awp models that the player with the index "id" has. Now, if you wanted to check if this player has the first awp you would use AWP[id][0] which would be equivalent to your AWP1 array. This is better, but still not great. Notice that at any moment the player can have only one model so only one of these booleans can be true. Therefore you don't need one for each model. Turns out you can just do: PHP Code: new AWP[33]  Notice I removed the bool tag because now we aren't going to use bools anymore, but integers. If you set AWP[id] to 0 it means the player has the first awp, if AWP[id] is 1 it means he has the second AWP, ..., and if it is 21 it means it has the last awp model. Now, you can rewrite AwpCase to be just: PHP Code: public AwpCase(id, menu, item) {     if(item == MENU_EXIT)     {         return 1;     }     AWP[id] = item;     menu_destroy (menu);     return 1; }  Notice how much useless code we were able to remove just by designing our approach carefully. Also, in this case, you don't need to use the whole menu_getinfo, it is enough to use the item parameter. Plus, there is no need to specify "1", "2", etc in menu_additem. Similarly, you can reduce all this: PHP Code: new const Awp1Model[66] = "models/v_awp.mdl"; new const Awp2Model[66] = "models/RevCreww/awp1.mdl"; new const Awp3Model[66] = "models/RevCreww/awp2.mdl"; new const Awp4Model[66] = "models/RevCreww/awp3.mdl"; new const Awp5Model[66] = "models/RevCreww/awp4.mdl"; new const Awp6Model[66] = "models/RevCreww/awp5.mdl"; new const Awp7Model[66] = "models/RevCreww/awp6.mdl"; new const Awp8Model[66] = "models/RevCreww/awp7.mdl"; new const Awp9Model[66] = "models/RevCreww/awp8.mdl"; new const Awp10Model[66] = "models/RevCreww/awp9.mdl"; new const Awp11Model[66] = "models/RevCreww/awp10.mdl"; new const Awp12Model[66] = "models/RevCreww/awp11.mdl"; new const Awp13Model[66] = "models/RevCreww/awp12.mdl"; new const Awp14Model[66] = "models/RevCreww/awp13.mdl"; new const Awp15Model[66] = "models/RevCreww/awp14.mdl"; new const Awp16Model[66] = "models/RevCreww/awp15.mdl"; new const Awp17Model[66] = "models/RevCreww/awp16.mdl"; new const Awp18Model[66] = "models/RevCreww/awp17.mdl"; new const Awp19Model[66] = "models/RevCreww/awp18.mdl"; new const Awp20Model[66] = "models/RevCreww/awp19.mdl"; new const Awp21Model[66] = "models/RevCreww/awp20.mdl"; new const Awp22Model[66] = "models/RevCreww/awp21.mdl";  Into this: PHP Code: new const AWPModels[][] = {     "models/v_awp.mdl",     "models/RevCreww/awp1.mdl",     ....,     "models/RevCreww/awp21.mdl" }  AWPModels[0] would be the equivalent of your Awp1Model, AWPModels[1] would be the equivalent of Awp2Model and so on. With this change you can simply plugin_precache to just: PHP Code: public plugin_precache() {     for(new i; i < sizeof AWPModels; i++)     {         precache_model(AWP[i])     } }  Since all your models are in the same array you can just iterate over it and precache every model in there, one by one with a for loop. And we can keep going and also fix your CurentWeapon function. PHP Code: public CurentWeapon(id) {     if(get_user_weapon(id) == CSW_AWP)     {         set_pev(id, pev_viewmodel2, AWPModels[AWP[id]])     } }  Recall that AWP[id] gives you the index of the current AWP and it turns out you can index AWPModels(where you have the awp models stored) by this value to retrieve the correct model. For example, if in the menu a player pressed "1" then AwpCase would execute and item will be 0. Then you would also store 0 inside AWP[id](AWP[id] = 0). AWPModels[AWP[id]] will be equivalent to AWPModels[0] and this will give you "models/v_awp.mdl". One more thing, instead of using CurWeapon please look into Ham_ItemDeploy, you can find plenty of examples around here. Hope this is helpful for you. There are more things that can be improved and I tried to keep the modifications I made relatively simple so you can compare to what you have right now and figure out the thought process behind these changes. Future improvements could be to use a dynamic array to allow an unlimited number of models, reading the models from a file and adding them to the array so people don't need to edit the code & recompile to add more models, etc.

Bro, Have Problem here

PHP Code:

public plugin_precache()
{
    for(new i; i < sizeof AWPModels; i++)
    {
        precache_model(AWP[i])
    }
} 


Models Not Precached

[HamletEagle]

What do you mean by models not precached? Is the server not starting up or crashing instantly? Could you attach the modified plugin with the changes you made to your next post? It would be much easier for me to help you if I see the entire code.

[RevCreww]

Originally Posted by HamletEagle What do you mean by models not precached? Is the server not starting up or crashing instantly? Could you attach the modified plugin with the changes you made to your next post? It would be much easier for me to help you if I see the entire code.

i mean game creashed whene i chouse awp, reason model not precached

[+ARUKARI-]

replace

precache_model(AWPModels[i])