Bonus limitation

[DeltaMasterMind]

An ability I am trying to rescript since I thought I had it right but is not as complete as I would have like so here is the deal this ability needs to have a bonus cap to allowable damage and must only allow for Light weapons, Bolas, Shurikens, and if selected the Exotic Weapon from the exotic weapon proficiency feat as the Style that can be chosen is for Exotic Weapon style.

Post-Attributes / 10000
~only run the rest for the first copy
doneif (tagis[Helper.FirstCopy] = 0)

~ If we're not shown, just get out now
doneif (tagis[Helper.ShowSpec] = 0)

~ Generate our damage limit
field[abValue2].value = field[xAllLev].value
field[abValue3].value = hero.child[aWIS].field[aModBonus].value

~ Generate our Bonus Limit
field[abValue].value = minimum(field[abValue3].value, field[abValue2].value)

~ Generate our summary
field[abSumm].text = "+" & field[abValue].value & " to hit, +" & field[abValue].value & " to damage." & hero.childfound[fcSAElves].field[abValue].value & " minutes to regain use of deadly focus."

if (field[abilActive].value <> 0) then
foreach pick in hero from BaseWep where "wClass.Light | IsWeapon.wBolas | IsWeapon.wShuriken"
hero.child[Damage].field[Bonus].value += field[abValue].value
hero.child[Attack].field[Bonus].value += field[abValue].value
nexteach
endif

Sadly this is not only benefiting the selected weapon types, but it also did it to a two handed spear that I tested and the bonus cap I placed is allowing a +4 bonus at 6 levels with a +1 wis stat and it only should allow that +1 and never let that bonus supersede the level value limit.

[ShadowChemosh]

Lets break the script down:

~only run the rest for the first copy
doneif (tagis[Helper.FirstCopy] = 0)

~ If we're not shown, just get out now
doneif (tagis[Helper.ShowSpec] = 0)

This is not bad but you never check for "disabled" meaning an archetype can not stop this ability from running. In addition logically the above is just backwards from other class scripts.

Here is a better way but this does not help with the main issue yet.

      ~ If we're not shown, just get out now
      doneif (tagis[Helper.ShowSpec] <> 1)
      ~ if we've been disabled, get out now
      doneif (tagis[Helper.SpcDisable] <> 0)

      ~only run the rest for the first copy
      doneif (tagis[Helper.FirstCopy] = 0)

Next Part:

~ Generate our damage limit
field[abValue2].value = field[xAllLev].value
field[abValue3].value = hero.child[aWIS].field[aModBonus].value

~ Generate our Bonus Limit
field[abValue].value = minimum(field[abValue3].value, field[abValue2].value)

Here again it "works" but its over kill using the abValue fields. We already have "values" in fields we can just use them directly and the script will be smaller and less confusing. In addition I did a += as this allows outside scripts or adjustments to affect the Bonus value up or down.

~ Generate our Damage Bonus max value.
field[abValue].value += minimum(hero.child[aWIS].field[aModBonus].value, field[xAllLev].value)

Next Part:

~ Generate our summary
field[abSumm].text = "+" & field[abValue].value & " to hit, +" & field[abValue].value & " to damage." & hero.childfound[fcSAElves].field[abValue].value & " minutes to regain use of deadly focus."

My only thought here is that what if abValue is ever negative number? This above would show +-Value which would be confusing.

~ Generate our summary
field[abSumm].text = signed(field[abValue].value) & " to hit, " & signed(field[abValue].value) & " to damage." & hero.childfound[fcSAElves].field[abValue].value & " minutes to regain use of deadly focus."

Here we I replaced the hard-coded + sign with the function signed() allowing the software to correctly place a + or - sign for us.

Next Part:

if (field[abilActive].value <> 0) then
foreach pick in hero from BaseWep where "wClass.Light | IsWeapon.wBolas | IsWeapon.wShuriken"
hero.child[Damage].field[Bonus].value += field[abValue].value
hero.child[Attack].field[Bonus].value += field[abValue].value
nexteach
endif

Now we get to the main issue. This says loop through Light weapons or Bolas or Shruiken on the hero. EACH time we find these weapons we are to add a Bonus to attack and damage to the HERO giving this to ALL WEAPONS. So Lets say abValue was 2 and we had four instances of light weapons then all weapons on the character would get a +8 to hit and a +8 on damage.

The above needs to be changed to ONLY give the attack/damage bonus to the weapon we found in the foreach loop:

~ If we are not active get out now!
doneif (field[abilActive].value = 0)

~ Loop through all light, bolas and shuriken weapons on the charcter
foreach pick in hero from BaseWep where "wClass.Light | IsWeapon.wBolas | IsWeapon.wShuriken"
  ~ Give a bonus to only the weapon we are looping on
  eachpick.field[Bonus].value += field[abValue].value
nexteach

So a final script looks like this:

      ~ If we're not shown, just get out now
      doneif (tagis[Helper.ShowSpec] <> 1)
      ~ if we've been disabled, get out now
      doneif (tagis[Helper.SpcDisable] <> 0)
      ~ Only run the rest for the first copy
      doneif (tagis[Helper.FirstCopy] = 0)

      ~ Generate our Damage Bonus max value.
      field[abValue].value += minimum(hero.child[aWIS].field[aModBonus].value, field[xAllLev].value)

      ~ Generate our summary
      field[abSumm].text = signed(field[abValue].value) & " to hit, " & signed(field[abValue].value) & " to damage." & hero.childfound[fcSAElves].field[abValue].value & " minutes to regain use of deadly focus."

      ~ If we are not active get out now!
      doneif (field[abilActive].value = 0)

      ~ Loop through all light, bolas and shuriken weapons on the charcter
      foreach pick in hero from BaseWep where "wClass.Light | IsWeapon.wBolas | IsWeapon.wShuriken"
        ~ Give a bonus to only the weapon we are looping on
        eachpick.field[Bonus].value += field[abValue].value
      nexteach

[DeltaMasterMind]

You know before I saw this post it hit me that I wasn't using eachpick and was instead doing bonuses for all picks on hero. I realized this when I was taking a piss. xD

I am very grateful Shadow thank you and I went ahead and cleaned up my script manually so I get used to doing it that way. Much appreciated! This alone fixes alot of the problems I ran into today.

[Mathias]

ShadowChemosh - The Helper.SpcDisable test is redundant in almost all cases, as long as you've got the Helper.ShowSpec test. Helper.ShowSpec is not asssigned if Helper.SpcDisable is present, so the only times you also need to test for Helper.SpcDisable is when you know the Helper.SpcDisable will be assigned after the normal test for its presence.

[Aaron]

Mathias and I differ on the necessity of SpcDisable tests. We've argued it multiple times and neither of us seem willing to budge.

[ShadowChemosh]

I had several responses here I have reworded about this so happy I am not the only one not 100% for this idea of not having SpcDisable. Logically I understand and agree 100% with Mathias in that its a "redundant" check.

My main issue is something I drill in my programmers at work which is "Maintenance". At some point someone besides the original coder is going to be reading and changing your code and it should be easy to read. This means I argue allot for being "verbose" in comments and variable names. Making sure we are obvious to the next coder.

In this case by having Helper.SpcDisable check we are 100% clear to the next coder (either new or experienced) in what is happening. No "behind the scenes" knowledge is required to be known. In my case as a outside person I don't have to worry about the "timing" of the behind the scenes code being changed. When I am doing code reviews of the community stuff its easy to make sure we are correct regardless of the scripts timing.

For my very specific case I program in almost a dozen languages and multiple data bases. Trying to keep all these "little" differences straight is not fun. Anything I can do to reduce that need to "memorize" a hidden feature is a plus in my book especially when adding it does not hurt anything.

[DeltaMasterMind]

Hey I just wanted to post to see if anything with this script seems like a bad idea considering how bonuses get added:

eachpick.field[wDamBonus].value += field[abValue3].value
eachpick.field[wAttBonus].value += field[abValue].value

The reason is that adding to the bonus fields doesn't fit the description of the ability as in no bonus type yet I have two different types of values here. Is this the way I should go about it or is there a more preferred way?

[ShadowChemosh]

Originally Posted by DeltaMasterMind

Hey I just wanted to post to see if anything with this script seems like a bad idea considering how bonuses get added:

The reason is that adding to the bonus fields doesn't fit the description of the ability as in no bonus type yet I have two different types of values here. Is this the way I should go about it or is there a more preferred way?

If the values in abValue & abValue3 can be different then that is fine. Using the "bonus" field is useful if you are adding the exact same value to both to hit and to damage.