This script is fine... you can definitely live with it and definitely dont need to use 2DA for this.
But if you want to learn how to write scripts more efficiently, there are my suggestions:
1) Get rid of custom function Prefers and replace with GetHasFeat directly like this
string GetWeapon() // Taken and slightly edited from the chest by spawn point in OC Prelude
{
object oPC = GetEnteringObject();
// Choose the weapon type to create
if (GetHasFeat(FEAT_WEAPON_FOCUS_BASTARD_SWORD, oPC))
{
return "bastardsword";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_BATTLE_AXE,oPC))
{
return "battleaxe";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_CLUB,oPC))
{
return "club";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_DAGGER,oPC))
{
return "dagger";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_DART,oPC))
{
return "dart";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_DIRE_MACE,oPC))
{
return "diremace";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_DOUBLE_AXE,oPC))
{
return "doubleaxe";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_GREAT_AXE,oPC))
{
return "greataxe";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_GREAT_SWORD,oPC))
{
return "greatsword";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_HALBERD,oPC))
{
return "halberd";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_HAND_AXE,oPC))
{
return "handaxe";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_HEAVY_CROSSBOW,oPC))
{
return "heavycrossbow";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_HEAVY_FLAIL,oPC))
{
return "heavyflail";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_KAMA,oPC))
{
return "kama";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_KATANA,oPC))
{
return "katana";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_KUKRI,oPC))
{
return "kukri";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_LIGHT_CROSSBOW,oPC))
{
return "lightcrossbow";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_LIGHT_FLAIL,oPC))
{
return "lightflail";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_LIGHT_HAMMER,oPC))
{
return "lighthammer";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_LIGHT_MACE,oPC))
{
return "lightmace";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_LONG_SWORD,oPC))
{
return "longsword";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_LONGBOW,oPC))
{
return "longbow";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_MORNING_STAR,oPC))
{
return "morningstar";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_RAPIER,oPC))
{
return "rapier";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_SCIMITAR,oPC))
{
return "scimitar";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_SCYTHE,oPC))
{
return "scythe";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_SHORT_SWORD,oPC))
{
return "shortsword";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_SHORTBOW,oPC))
{
return "shortbow";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_SHURIKEN,oPC))
{
return "shuriken";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_SICKLE,oPC))
{
return "sickle";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_SLING,oPC))
{
return "sling";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_SPEAR,oPC))
{
return "spear";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_STAFF,oPC))
{
return "staff";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_THROWING_AXE,oPC))
{
return "throwingaxe";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_TWO_BLADED_SWORD,oPC))
{
return "twobladedsword";
}
else if (GetHasFeat(FEAT_WEAPON_FOCUS_WAR_HAMMER,oPC))
{
return "warhammer";
}
return ""; // Return nothing if character has no weapon focus
}
This was the "worst" part of you code that is really wroten badly. Anything else you have there, whether its written perfectly efficient or not has zero impact on real efficiency. Anyway...
2) the line: "if(!(GetGold(oPC) >= 250))" is better to write GetGold(oPC) < 250. Or take all gold and give 250 directly. Same result but a bit more cheat-proof solution 
'> .
3)
int i;
for(i=0; i<14; i++)
you can drop the first part of the for leaving only for(;i<14;i++) but whenever you do this its usually better idea to use while, for is more efficient in a case you already has a variable and you need to set new value to it, where you can do that in one statement, while with while you would need extra statement. But if you have to declare variable before (and unfortunately you cannot declare variable inside for statement in NWScript) there is no benefit of that.
But thats really minor optimalization, whats more important is the 14, i would replace it with NUM_INVENTORY_SLOTS think its 17? You really want to destroy all items, even those in creature slots otherwise you find out players are loggin in with uber skin with immunities etc. Lazarus can tell 
'> .
4) if(!GetIsPC(oPC)) return;
this is ok, but you should probably also want to return this for a DM? Also even if its singleplayer you will want to ensuere this code wont run twice. I suggest to use SetLocal/GetLocal, but depends - in multiplayer I use a zero XP check and increasing xp to 1 (with bullet proof security to ensure pc wont drop over 1xp later)
5)
string sAmmo = "";
int iAmmoAmount = 0;
setting variables to its default values is pointless. Rather write this:
string sAmmo;
int iAmmoAmount;
also allows to chain this when you are declaring more of them like this:
int a,b,c,d,e,f,g,h = 1; (explanation: a - g = 0, only h = 1)
EDIT: gotcha MM!!!
Modifié par Shadooow, 24 avril 2014 - 07:19 .
