7 réponses à ce sujet

[HaplessHarpy]

I've recently got back into the swing of working on my old project since downloading the latest beta I uploaded to NWVault a few years back.
 
I was messing around with the OnClientEnter script for the module, which is designed to strip a players inventory and give them some basic stuff based on their class/weapon focus.
 
I figured I'd post it here in hopes of some suggestions or optimizations, as it's currently just shy of 300 lines of code.
I'm also interested as to whether this would work if I were to make the module a PW of some sort, as that is a possibility in the future.

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
}

void main()
{
    object oPC = GetEnteringObject();
    if(!GetIsPC(oPC) || GetIsDM(oPC)) return;

// Check Equip Items and get rid of them
    int i;
    for(i<17; i++;)
    {
    object oEquip = GetItemInSlot(i,oPC);
    if(GetIsObjectValid(oEquip))
        {
        SetPlotFlag(oEquip,FALSE);
        DestroyObject(oEquip);
        }
    }
// Check general Inventory and clear it out.
    object oItem = GetFirstItemInInventory(oPC);
    while(GetIsObjectValid(oItem))
        {
        SetPlotFlag(oItem,FALSE);
        DestroyObject(oItem);
        oItem = GetNextItemInInventory(oPC);
        }
// Give them Gold
    TakeGoldFromCreature(GetGold(oPC), oPC);
    GiveGoldToCreature(oPC, 250);

    // Get First class and assign default item tags
    int iClass = GetClassByPosition(1,oPC);
    string sAmmo, sWeapon = "staff", sArmour = "plainrobe";
    int iAmmoAmount, iWepStacksize = 1;
    // Assign Item tags based on primary class
    switch (iClass)
    {
        // Barbarian
        case 0:
            sArmour = "hidearmour";
            sWeapon = "handaxe";
            break;
        // Bard
        case 1:
            sArmour = "plainrobe";
            sWeapon = "staff";
            break;
        // Cleric
        case 2:
            sArmour = "chainmail";
            sWeapon = "lightmace";
            break;
        // Druid
        case 3:
            sArmour = "hidearmour";
            sWeapon = "staff";
            break;
        // Fighter
        case 4:
            sArmour = "halfplate";
            sWeapon = "longsword";
            break;
        // Monk
        case 5:
            sArmour = "paddedcloth";
            sWeapon = "staff";
            break;
        // Paladin
        case 6:
            sArmour = "fullplate";
            sWeapon = "greatsword";
            break;
        // Ranger
        case 7:
            sArmour = "heavyleather";
            sWeapon = "longbow";
            sAmmo = "arrow";
            iAmmoAmount = 99;
            break;
        // Rogue
        case 8:
            sArmour = "leatherarmour";
            sWeapon = "shortsword";
            break;
        // Sorcerer
        case 9:
            sArmour = "plainrobe";
            sWeapon = "dagger";
            break;
        // Wizard
        case 10:
           sArmour = "plainrobe";
           sWeapon = "staff";
            break;
    }
    // Check for Weapon focus
    string sFeatWeapon = GetWeapon();
    if(sFeatWeapon != "")
    {
        sWeapon = sFeatWeapon;
    }

    // Check for ammo requirement
    if(sWeapon == "shortbow" || sWeapon == "longbow")
    {
        sAmmo = "arrow";
    }
    else if(sWeapon == "lightcrossbow" || sWeapon == "heavycrossbow")
    {
        sAmmo = "bolt";
    }
    else if(sWeapon == "sling")
    {
        sAmmo = "bullet";
    }

    // Check weapon stacksize
    if(sWeapon == "dart" || sWeapon == "throwingaxe")
    {
        iWepStacksize = 50;
    }
    else if(sWeapon == "shuriken")
    {
        iWepStacksize = 99;
    }

    CreateItemOnObject(sArmour, oPC);
    CreateItemOnObject(sWeapon, oPC, iWepStacksize);
    if(sAmmo != "")
    {
        CreateItemOnObject(sAmmo, oPC, 99);
    }
    string sHerb = "medicinalherb";
    CreateItemOnObject(sHerb, oPC);
    CreateItemOnObject(sHerb, oPC);
    CreateItemOnObject(sHerb, oPC);
}

I'm still re-accustoming myself with NWScript, although I've learned C# since the last time I used it, and it has some close similarities.

[WhiZard]

The quick answer is that a script is not optimal here; at least not for a large number of cases, then a 2da would be a whole lot more useful.  You could either add columns to an existing one (like baseitems.2da or feat.2da) or you could create your own with similar indexing.  Alternatively you can check the weapon the PC has equipped and increase the likelihood of that weapon if the PC has feats to support it.  I bring this up because your (BioWare standard) system of giving out loot prioritize based on the alphabetization of the weapon's name.  If I had two weapon focuses then I would be getting the weapon whose name is earliest alphabetically.

[BelowTheBelt]

As far as script is concerned, I'd declare the sAmmo and iWepStackSize (at a default value) before the class-based case statement.  Then, in the case statements, you add a line for each of the affected items to change the value.  That way, you don't have to do any 'if' checks afterward.  You can move right into setting the ammo and the stack size.

[Proleric]

2DA is a good way to go.

You could also use a pseudo-array or a string with delimited values (e.g. "longsword\bastard sword\...") as a lookup table, but that would be unwieldy in this case, with no great advantage over 2DA.

As a lateral thought : in EI3 I approach this problem by providing an armoury, from which the player can select only one item per placeable before it becomes unusable.

[HaplessHarpy]

As far as script is concerned, I'd declare the sAmmo and iWepStackSize (at a default value) before the class-based case statement.  Then, in the case statements, you add a line for each of the affected items to change the value.  That way, you don't have to do any 'if' checks afterward.  You can move right into setting the ammo and the stack size.

The trouble with that is that the only weapon that needs sAmmo, iAmmoAmount or iWepStacksize in the class case statements is the longbow, most of the ammo-requiring weapons, or thrown weapons, come from the feats, which are checked in a different function that directly returns the tag of the weapon, meaning I can't set sAmmo, iAmmoAmount or iWepStacksize in there, so I'd still have to do the if statement.
 
 

The quick answer is that a script is not optimal here; at least not for a large number of cases, then a 2da would be a whole lot more useful.  You could either add columns to an existing one (like baseitems.2da or feat.2da) or you could create your own with similar indexing.  Alternatively you can check the weapon the PC has equipped and increase the likelihood of that weapon if the PC has feats to support it.  I bring this up because your (BioWare standard) system of giving out loot prioritize based on the alphabetization of the weapon's name.  If I had two weapon focuses then I would be getting the weapon whose name is earliest alphabetically.

 

2DA is a good way to go.

You could also use a pseudo-array or a string with delimited values (e.g. "longsword\bastard sword\...") as a lookup table, but that would be unwieldy in this case, with no great advantage over 2DA.

As a lateral thought : in EI3 I approach this problem by providing an armoury, from which the player can select only one item per placeable before it becomes unusable.

Hmm, I've never worked with 2DA before, and would have no idea where to start..
I guess it's research time.

 

Thanks for the advice guys!

[Shadooow]

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 .

[MagicalMaster]

/edit DAMNIT SHADOOOW!   /endedit

 

The whole Prefers function seems unneeded.

int Prefers(int nFeatWeaponType, object oAdventurer) // Taken from the chest by spawn point in OC Prelude
{

    if (GetHasFeat(nFeatWeaponType, oAdventurer) == TRUE)

    return TRUE;

    else

    return FALSE;

}

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 (Prefers(FEAT_WEAPON_FOCUS_BASTARD_SWORD, oPC))

    {

        return "bastardsword";

    }
}

could be made into

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";
    }
}

I think all you have to do is type GetHasFeat instead of Prefers and you can drop that function entirely.

[HaplessHarpy]

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!!!

 

Yeah I noticed the prefers function was totally unneeded shortly after posting it, but as the comments say, the original code was taken from the spawn chest in the OC Prelude. Fixed now

 

I changed the GetIsPC line to

if(!GetIsPC(oPC) || GetIsDM(oPC)) return;

 

I also changed the gold code to

TakeGoldFromCreature(GetGold(oPC), oPC);

GiveGoldToCreature(oPC, 250);

 

Changed the max limit on the for loop that strips equipment to 17.

 

I wasn't actually aware that the default value for variables was 0 or equivalent, as I said, I'm a C# programmer, so I'm used to undefined variables being equal to null.

 

Thanks for the help Shadooow.

 

Also updated the OP with the new code.