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:
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.