Advice on condensing a script : V

» Sat Nov 17, 2012 3:00 am

I am working on an arcane/empowered archer type mod.
However the script that i have made, works, but is quite repetative. I would appreciate any advice on how i can condense the script, but keeping the same functionality

I have added more notes to the script than i would normally, to explain what each area does. I wouldn't normally have that much notation in a script.

Spoiler

Scriptname ArcaneArcherBowScript_NECMAS extends Actor{Enables the Empowered Archer to fire spells when the bow is fired, also  equips a quiver with a never ending supply of arrows}Ammo Property AMMO_EmpoweredArrow6_NECMAS AutoAmmo Property AMMO_EmpoweredArrow8_NECMAS AutoAmmo Property AMMO_EmpoweredArrow10_NECMAS AutoAmmo Property AMMO_EmpoweredArrow12_NECMAS AutoAmmo Property AMMO_EmpoweredArrow14_NECMAS AutoAmmo Property AMMO_EmpoweredArrow16_NECMAS AutoAmmo Property AMMO_EmpoweredArrow18_NECMAS AutoAmmo Property AMMO_EmpoweredArrow22_NECMAS AutoFaction Property Fact_EmpoweredBow_NECMAS AutoKeyword Property EmpoweredFireShroud AutoKeyword Property EmpoweredFrostShroud AutoKeyword Property EmpoweredLightningShroud AutoMagicEffect Property MFx_EmpoweredArcher_NECMAS AutoSpell Property SP_EmpoweredArcherFirebolt_NECMAS AutoSpell Property SP_EmpoweredArcherFrostBolt_NECMAS AutoSpell Property SP_EmpoweredArcherShock_NECMAS AutoSpell Property Paralyze AutoWeapon Property Weap_EmpoweredBow_NECMAS AutoImport GameEvent OnPlayerBowShot(Weapon akWeapon, Ammo akAmmo, float afPower, bool abSunGazing)Float DL = GetPlayer().GetActorValue("Destruction")Float CL = GetPlayer().GetActorValue("Conjuration")Float ML = GetPlayer().GetActorValue("Marksman")Float AL = GetPlayer().GetActorValue("Alteration") Float rand1Threshold = (25+(ML/4))  ;debug.notification ("num threshold = " +rand1Threshold)Int rand1 = Utility.RandomInt();debug.notification("Random Number = "+ rand1) Float AvgSkLev = ((DL+CL+ML+AL)/4)debug.notification("AvgSkLev =" +AvgSkLev);   This section determines what type of spell is cast depending on the type of shroud the player has cast on themselves;   and the chance it has of casting;   If the player is sneaking and not in combat the spell will cast 100% of the time;   If the player is in combat the chance a spell will cast is 25+the players marksman level divided by 4.;   At marksman level 100 the player will have a 50% chance to cast a spell on bow shot;   When a spell is cast there is also a chance that the target will become paralyzed. The chance that this will occur is marksman level;   divided by 10. At level 20 its 2% at 100 its 10% ;   The Faction "Fact_EmpoweredBow_NECMAS" prevents anything other than an empowered bow from causing the spells to cast.;   The script for this is attached to the bow, the player is placed into the faction on equipping the bow, and;   removed on unequipIf GetPlayer().IsInFaction(Fact_EmpoweredBow_NECMAS)	 ;Check - Player is in the Empowered Bow FactionIf GetPlayer().HasMagicEffect(MFx_EmpoweredArcher_NECMAS)    ;Check - Player has the Empowered Archer magic effect  if GetPlayer().HasEffectKeyword(EmpoweredFireShroud)   ;Check - Player has a fire shroud   if GetPlayer().IsSneaking() && !GetPlayer().IsInCombat() ;Check - Player is sneaking and not in combat    SP_EmpoweredArcherFirebolt_NECMAS.cast(GetPlayer())  ;On bow shot, a fireball is cast   elseif rand1 <= rand1Threshold    SP_EmpoweredArcherFirebolt_NECMAS.cast(GetPlayer())  ;Player has failed the sneaking/combat check, spell is cast on random number check	 if rand1 <= (ML/10)	  paralyze.cast(GetPlayer())	 endif   endif    elseif GetPlayer().HasEffectKeyword(EmpoweredLightningShroud)   if GetPlayer().IsSneaking() && !GetPlayer().IsInCombat()    SP_EmpoweredArcherShock_NECMAS.cast(GetPlayer())   elseif rand1 <= rand1Threshold    SP_EmpoweredArcherShock_NECMAS.cast(GetPlayer())	 if rand1 <= (ML/10)	  paralyze.cast(GetPlayer())	 endif   endif    elseif GetPlayer().HasEffectKeyword(EmpoweredFrostShroud)   if GetPlayer().IsSneaking() && !GetPlayer().IsInCombat()    SP_EmpoweredArcherFrostbolt_NECMAS.cast(GetPlayer())   elseif rand1 <= rand1Threshold    SP_EmpoweredArcherFrostbolt_NECMAS.cast(GetPlayer())	 if rand1 <= (ML/10)	  paralyze.cast(GetPlayer())	 endif   endif  endifendif  ;   This section determines which arrows should be given based on average skill level of the 4 desginated skills;   Each section denoted by  ----- is basically the same code with different arrows given based on the skill level;   Two arrows seem to be the minimum needed to create an illusion of a quiver of never ending arrows;   NOTE: EquipItem() is needed as whilst the arrows are added to the player, they are not usable unless equipped;   COULD THIS SECTION BE REPLACED BY A FORM LIST, HOW DO YOU OBTAIN AN ITEM IN THE FORM LIST BASED ON THE AgSkLEv ;--------------------------------------------------------------------------------------------------   If AvgSkLev <=35   GetPlayer().Additem(AMMO_EmpoweredArrow6_NECMAS, 1)    GetPlayer().EquipItem(AMMO_EmpoweredArrow6_NECMAS)	 debug.notification("Arrow 6 equipped");---------------------------------------------------------------------------------------------------	  elseif AvgSkLev >=36 && AvgSkLev <=40		  ;Check - Average Skill level is between 36 and 40   debug.notification(" Skill level between 36 and 40")      If GetPlayer().GetItemCount(AMMO_EmpoweredArrow8_NECMAS) < 2  ;Check - Amount of arrows on the player for this level	 GetPlayer().Additem(AMMO_EmpoweredArrow8_NECMAS, 2)    ;If less than 2 adds 2 arrows to the player	  GetPlayer().EquipItem(AMMO_EmpoweredArrow8_NECMAS)   ;Equips those arrows, this line is needed see NOTE     Else	 GetPlayer().AddItem(AMMO_EmpoweredArrow8_NECMAS, 1)    ;If player already has 2 or more arrows, adds 1 arrow	  debug.notification(" Arrow 8 added")	   GetPlayer().EquipItem(AMMO_EmpoweredArrow8_NECMAS)  ;Equips arrow	    debug.notification("Arrow 8 equipped")       endif;---------------------------------------------------------------------------------------------     elseif AvgSkLev >=41 && AvgSkLev <=45   debug.notification(" Skill level between 41 and 45")      If GetPlayer().GetItemCount(AMMO_EmpoweredArrow10_NECMAS) < 2	 GetPlayer().Additem(AMMO_EmpoweredArrow10_NECMAS, 2)	  GetPlayer().EquipItem(AMMO_EmpoweredArrow10_NECMAS)     Else	 GetPlayer().AddItem(AMMO_EmpoweredArrow10_NECMAS, 1)	  debug.notification(" Arrow 10 added")	   GetPlayer().EquipItem(AMMO_EmpoweredArrow10_NECMAS)	    debug.notification("Arrow 10 equipped")       endif  ;--------------------------------------------------------------------------------------------------  elseif AvgSkLev >=46 && AvgSkLev <=50   debug.notification(" Skill level between 46 and 50")      If GetPlayer().GetItemCount(AMMO_EmpoweredArrow12_NECMAS) < 2	 GetPlayer().Additem(AMMO_EmpoweredArrow12_NECMAS, 2)	  GetPlayer().EquipItem(AMMO_EmpoweredArrow12_NECMAS)     Else	 GetPlayer().AddItem(AMMO_EmpoweredArrow12_NECMAS, 1)	  debug.notification(" Arrow 12 added")	   GetPlayer().EquipItem(AMMO_EmpoweredArrow12_NECMAS)	    debug.notification("Arrow 12 equipped")       endif;-------------------------------------------------------------------------------------------  elseif AvgSkLev >=51 && AvgSkLev <=55   debug.notification(" Skill level between 51 and 55")      If GetPlayer().GetItemCount(AMMO_EmpoweredArrow14_NECMAS) < 2	 GetPlayer().Additem(AMMO_EmpoweredArrow14_NECMAS, 2)	  GetPlayer().EquipItem(AMMO_EmpoweredArrow14_NECMAS)     Else	 GetPlayer().AddItem(AMMO_EmpoweredArrow14_NECMAS, 1)	  debug.notification(" Arrow 14 added")	   GetPlayer().EquipItem(AMMO_EmpoweredArrow14_NECMAS)	    debug.notification("Arrow 14 equipped")       endif;-------------------------------------------------------------------------------------------  elseif AvgSkLev >=56 && AvgSkLev <=60   debug.notification(" Skill level between 56 and 60")      If GetPlayer().GetItemCount(AMMO_EmpoweredArrow16_NECMAS) < 2	 GetPlayer().Additem(AMMO_EmpoweredArrow16_NECMAS, 2)	  GetPlayer().EquipItem(AMMO_EmpoweredArrow16_NECMAS)     Else	 GetPlayer().AddItem(AMMO_EmpoweredArrow16_NECMAS, 1)	  debug.notification(" Arrow 16 added")	   GetPlayer().EquipItem(AMMO_EmpoweredArrow16_NECMAS)	    debug.notification("Arrow 16 equipped")       endif ;-------------------------------------------------------------------------------------------     elseif AvgSkLev >=61 && AvgSkLev <=65   debug.notification(" Skill level between 61 and 65")      If GetPlayer().GetItemCount(AMMO_EmpoweredArrow18_NECMAS) < 2	 GetPlayer().Additem(AMMO_EmpoweredArrow18_NECMAS, 2)	  GetPlayer().EquipItem(AMMO_EmpoweredArrow18_NECMAS)     Else	 GetPlayer().AddItem(AMMO_EmpoweredArrow18_NECMAS, 1)	  debug.notification(" Arrow 18 added")	   GetPlayer().EquipItem(AMMO_EmpoweredArrow18_NECMAS)	    debug.notification("Arrow 18 equipped")       endif;-------------------------------------------------------------------------------------------  elseif AvgSkLev >=66   debug.notification(" Skill level 66 and higher")      If GetPlayer().GetItemCount(AMMO_EmpoweredArrow22_NECMAS) < 2	 GetPlayer().Additem(AMMO_EmpoweredArrow22_NECMAS, 2)	  GetPlayer().EquipItem(AMMO_EmpoweredArrow22_NECMAS)     Else	 GetPlayer().AddItem(AMMO_EmpoweredArrow22_NECMAS, 1)	  debug.notification(" Arrow 22 added")	   GetPlayer().EquipItem(AMMO_EmpoweredArrow22_NECMAS)	    debug.notification("Arrow 22 equipped")       endif  endif   endifEndEvent

» Sat Nov 17, 2012 5:39 pm

I can give you one tip right off the bat: Add an Actor Property called PlayerRef. Autofill it. Now replace every instance of GetPlayer() with PlayerRef. Your script will execute MUCH faster.

» Sat Nov 17, 2012 6:23 am

You could do some integer maths to determine an index into a formlist basically this, assuming you also use the comment above:

int temp = AvgSkLev - 31 ; this produces the offset you haveif (temp < 0)  temp = 0endif; this calculates the blocks of 5 skill levelsint index = temp / 5 ; this may need a Floor(temp / 5), I dont know whether Papyrus does integer divisions as I expect it to; this restricts it to the items in the list since you only defined 8 arrow types (I believe) this would limit the index to 7if (index > ArrowFormList.GetSize() - 1)  index = ArrowFormList.GetSize() - 1endif; Now we can use index with the followingAmmo AMMO_Item = ArrowFormList.GetAt(index)	If PlayerRef.GetItemCount(AMMO_Item) < 2		 PlayerRef.Additem(AMMO_Item, 2)	Else		 ; Why have this here?		 PlayerRef.AddItem(AMMO_Item, 1)	EndIf	PlayerRef.EquipItem(AMMO_Item)

You would need to add the arrows to the list in the right order of course for this to work

I can't test this code right now, but this should be about right...

» Sat Nov 17, 2012 10:16 am

heilghast, I've been looking at your script, and besides the comments Verteiron & TMPhoenix, it's really not so bad. Sure, there is a lot there, but being in just two IF/THEN blocks and making use of the ELSEIF, your really not executing that much code each iteration. I did notice that it looks like you cut & pasted that code (probably to post?) so what you have there isn't going to compile as it's not in an (event?) function. :wink:

Optimizing the code in the IF/ELSEIF conditions are the most important and I would try to put the conditions in order of most likely first so you don't waste a lot of time checking conditions that aren't likely. I realize since you are checking a number range, so that might not be practical. (I would NOT move around the blocks like having 50-60, then 30-40, 60>, etc, I'd do what you are already doing and keep them in numerical order - but in ascending or descending order, which ever is more likely. But can you optimize the first condition checks?

And finally, nothing wrong with lots of comments! I would seriously hope that Papyrus compiles those out of the code so there should be absolutely no performance issues with comments (not the case with debug statements though) and may really help you down the road when you have to change something. (If I'm wrong about the compiler, I hope someone who's done the testing points it out to me.)

» Sat Nov 17, 2012 5:09 pm

Thank you all for your comments, it is very much appreciated.

» Sat Nov 17, 2012 7:00 am

The compiler removes standard semicolon comments but keeps the special curly-brace comments associated with properties (so they are visible to the CK). But even the ones it keeps shouldn't slow down a running script.

Using a player property instead of the GetPlayer function will give a huge speed increase. My only other suggestion is to make only one check per ElseIf against the average skill level. There's no need to check to see if the value is over 35 if you're in an ElseIf where you had just checked against 35. If your really want both values of the range listed do it in a comment. More importantly since AvgSkLev is a FLOAT type your original version is going to completely skip any values that happen to fall between 35 and 36, 40 and 41, etc. That's probably not what you wanted.

If AvgSkLev <= 35 ; up to and including 35...ElseIf AvgSkLev <= 40  ; over 35 and up to 40...ElseIf AvgSkLev <= 45  ; over 40 and up to 45...

It's also more traditional to trigger a new feature when you reach a target number and not just after. If that's the case maybe you should be using:

If AvgSkLev < 35 ; under 35...ElseIf AvgSkLev < 40  ; at least 35 but not 40...ElseIf AvgSkLev < 45  ; at least 40 but not 45...

When deciding how fast or efficient a script will be you're really just doing a count of the actions it will perform not the total number of things it could do. Your script already has the right structure.

» Sat Nov 17, 2012 4:08 pm

You can easily write functions to shorten this. I've taken the liberty of writing a couple. Warning! They may not be error free.

;...function czGiveArrows(Ammo czArrowType)    if getplayer().getitemcount(czArrowType) < 2        getplayer().additem(czArrowType, 2)        getplayer().equipitem(czArrowType)    else        getplayer().additem(czArrowType, 1)        getplayer().equipitem(czArrowType)    endifendfunctionfunction czTryArrowEvent(Keyword czKeyword, Spell czSpell)    float ML = getplayer().getactorvalue("Marksman")    int rand1 = utility.randomint()    float rand1Threshold = (25+(ML/4))    if getplayer().haseffectkeyword(czKeyword)        if getplayer().issneaking() && !getplayer().isincombat()            czSpell.cast(getplayer())        elseif rand1 <= rand1Threshold            czSpell.cast(getplayer())            if rand1 <= (ML/10)                Paralyze.cast(getplayer())            endif        endif    endifendfunction

» Sat Nov 17, 2012 3:55 am

Once again, i would like to thank everyone who took the time to post. It has been very much appreciated, and as an update, this is now the script i am using which does exactly the same as the original script that i posted
Without your comments and suggestions, i don't think i would have had the script i have now.

Spoiler

Scriptname EmpoweredArcherBowShot_NECMAS extends Actor {Enables the Empowered Archer to fire spells when the bow is fired, also  equips a quiver with a never ending supply of arrows}Actor Property PlayerRef AutoAmmo Property EmpArrowType Auto	  ;This property is defined by the index of the formlistFaction Property Fact_EmpoweredBow_NECMAS AutoFormlist Property FLST_EmpoweredArrow_NECMAS AutoKeyword Property EmpoweredFireShroud AutoKeyword Property EmpoweredFrostShroud AutoKeyword Property EmpoweredLightningShroud AutoMagicEffect Property MFx_EmpoweredArcher_NECMAS AutoSpell Property SP_EmpoweredArcherFirebolt_NECMAS AutoSpell Property SP_EmpoweredArcherFrostBolt_NECMAS AutoSpell Property SP_EmpoweredArcherShock_NECMAS AutoSpell Property Paralyze AutoWeapon Property Weap_EmpoweredBow_NECMAS AutoImport MathFunction CastSpellOnShroudType(Actor akCaster)    if PlayerRef.HasEffectKeyword(EmpoweredFireShroud)	    SP_EmpoweredArcherFirebolt_NECMAS.cast(PlayerRef)       elseif PlayerRef.HasEffectKeyword(EmpoweredLightningShroud)	    SP_EmpoweredArcherShock_NECMAS.cast(PlayerRef)       elseif  PlayerRef.HasEffectKeyword(EmpoweredFrostShroud)	    SP_EmpoweredArcherFrostbolt_NECMAS.cast(PlayerRef)       endif EndFunction Event OnPlayerBowShot(Weapon akWeapon, Ammo akAmmo, float afPower, bool abSunGazing)Float DL = PlayerRef.GetActorValue("Destruction")Float CL = PlayerRef.GetActorValue("Conjuration")Float ML = PlayerRef.GetActorValue("Marksman")Float AL = PlayerRef.GetActorValue("Alteration")Float rand1Threshold = (25+(ML/4))	;debug.notification ("num threshold = " +rand1Threshold)Int rand1 = Utility.RandomInt()       ;debug.notification("Random Number = "+ rand1) Float AvgSkLev = ((DL+CL+ML+AL)/4)    ;debug.notification("AvgSkLev =" +AvgSkLev);=============THIS SECTION DETERMINES WHEN TO CAST A SPELL. NOW USES THE FUNCTION CastSpellOnShroudType=======If PlayerRef.IsInFaction(Fact_EmpoweredBow_NECMAS) && PlayerRef.HasMagicEffect(MFx_EmpoweredArcher_NECMAS)  if PlayerRef.IsSneaking() && !PlayerRef.IsInCombat()   CastSpellOnShroudType(PlayerRef)  elseif rand1 <= rand1Threshold   CastSpellOnShroudType(PlayerRef)    if rand1 <= (ML/10)	 paralyze.cast(playerRef)    endif  endif  ;==============THIS SECTION OBTAINS THE AMMO TO AWARD FROM THE FORMLIST===============================float temp = (AvgSkLev - 31)   ;debug.notification("temp = "+(AvgSkLev - 31))  if (temp < 0)     temp = 0  endifint Index = floor(temp / 5)  if (index > FLST_EmpoweredArrow_NECMAS.GetSize())     Index = (FLST_EmpoweredArrow_NECMAS.GetSize() - 1)  endifEmpArrowType = FLST_EmpoweredArrow_NECMAS.GetAt(Index) as Ammo  ;debug.notification("EmpArrowType = " +FLST_EmpoweredArrow_NECMAS.GetAt(Index))  If PlayerRef.GetItemCount(EmpArrowType) < 2	 PlayerRef.Additem(EmpArrowType, 2)  Else	 PlayerRef.AddItem(EmpArrowType, 1)  Endif    PlayerRef.EquipItem(EmpArrowType)EndifEndEvent