[Sheetworker] Is my else/if too long?

I'm struggling to understand what I'm doing wrong in this script. While I don't really think it is "too long" it works when I make it shorter! The sheetworker is for a Traveller: The New Era sheet I'm working on. The system tracks damage to hit locations and each location can have "scratch", "slight", "serious" or "critical" damage. Penalties are imposed based on the most severe damage level across all locations. If I cut out parts of it, the parts I keep work. But if I keep all the code it is only able to recognize "serious" and "critical" damage levels. This is the code I'm currently using: on("change:head_status change:chest_status change:abdomen_status change:left_arm_status change:right_arm_status change:left_leg_status change:right_leg_status", function() {             getAttrs(["head_status_level", "chest_status_level","strength_max", "abdomen_status_level", "left_arm_status_level","right_arm_status_level","left_leg_status_level","right_leg_status_level"], function(pvalue) {                 var head_status_level = parseInt(pvalue.head_status_level);                 var chest_status_level = parseInt(pvalue.chest_status_level);                 var abdomen_status_level = parseInt(pvalue.abdomen_status_level);                 var left_arm_status_level = parseInt(pvalue.left_arm_status_level);                 var right_arm_status_level = parseInt(pvalue.right_arm_status_level);                 var left_leg_status_level = parseInt(pvalue.left_leg_status_level);                 var right_leg_status_level = parseInt(pvalue.right_leg_status_level);                 var left_leg_status_level = parseInt(pvalue.left_leg_status_level);                 var strength_max = parseInt(pvalue.strength_max);                 var half_strength = Math.floor(strength_max/2);                 var strength;                 if (head_status_level == 4) {                     damage_initiative_penalty = "5";                     strength = 0;                 } else if (chest_status_level == 4) {                     damage_initiative_penalty = "5";                     strength = half_strength;                 } else if (abdomen_status_level == 4) {                     damage_initiative_penalty = "5";                     strength = half_strength;                 } else if (left_arm_status_level == 4) {                     damage_initiative_penalty = "5";                     strength = half_strength;                 } else if (right_arm_status_level == 4) {                     damage_initiative_penalty = "5";                     strength = half_strength;                 } else if (left_leg_status_level == 4) {                     damage_initiative_penalty = "5";                     strength = half_strength;                 } else if (right_leg_status_level == 4) {                     damage_initiative_penalty = "5";                     strength = half_strength;                 } else if (head_status_level == 3) {                     damage_initiative_penalty = "3";                     strength = half_strength;                 } else if (chest_status_level == 3) {                     damage_initiative_penalty ="3";                     strength = half_strength;                 } else if (abdomen_status_level == 3) {                     damage_initiative_penalty ="3";                     strength = half_strength;                 } else if (left_arm_status_level == 3) {                     damage_initiative_penalty ="3";                     strength = half_strength;                 } else if (right_arm_status_level == 3) {                     damage_initiative_penalty ="3";                     strength = half_strength;                 } else if (left_leg_status_level == 3) {                     damage_initiative_penalty ="3";                     strength = half_strength;                 } else if (right_status_level == 3) {                     damage_initiative_penalty ="3";                     strength = half_strength;                 } else if (head_status_level == 3) {                     damage_initiative_penalty = "1";                     strength = strength_max;                 } else if (chest_status_level == 2) {                     damage_initiative_penalty = "1";                     strength = strength_max;                 } else if (abdomen_status_level == 2) {                     damage_initiative_penalty = "1";                     strength = strength_max;                 } else if (left_arm_status_level == 2) {                     damage_initiative_penalty = "1";                     strength = strength_max;                 } else if (right_arm_status_level == 2) {                     damage_initiative_penalty = "1";                     strength = strength_max;                 } else if (left_leg_status_level == 2) {                     damage_initiative_penalty = "1";                     strength = strength_max;                 } else if (right_leg_status_level == 2) {                     damage_initiative_penalty = "1";                     strength = strength_max;                 } else {                     damage_initiative_penalty = "0";                     strength = strength_max;                 } setAttrs({damage_initiative_penalty,strength})                 });             });      The "_status_level" attributes correspond to the damage levels described above. And there is probably a lot of things that is redundant and/or could have been done better - I don't really understand what I'm doing here...

This line is probably your problem } else if (right_status_level == 3) { There is no  right_status_level , it's missing the word arm or leg . You also have this line twice in your starting definitions: var left_leg_status_level = parseInt(pvalue.left_leg_status_level); And you should really add this before your if statement starts: var damage_initiative_penalty; just so its defined properly before assigning it later. I suspect there's a better way to do that if statement, but I think these fixes will get it working.

Thanks, I was sure I had fixed those. I might have copy/pasted the wrong version... I'll try this tomorrow - getting late here. 

You also have this twice } else if (head_status_level == 3) { and no head_status_level == 2

One thing you can do to make the  if statement shorter, is combine several on the same line. This: ||  is the OR operator, and you can use it like this } else if (right_arm_status_level == 3 || left_leg_status_level == 3 || right_status_level == 3) {                     damage_initiative_penalty ="3";                     strength = half_strength; You can include as many expressions in the same if statement as you need.

Here's a streamlined version that I think matches your rules: on('change:head_status change:chest_status change:abdomen_status change:left_arm_status change:right_arm_status change:left_leg_status change:right_leg_status', function() {     getAttrs(['head_status_level', 'chest_status_level','strength_max', 'abdomen_status_level', 'left_arm_status_level','right_arm_status_level','left_leg_status_level','right_leg_status_level'], function(pvalue) {       // first add || 0 to each parseInt, so that if not a number they will be set to zero.         var head_status_level = parseInt(pvalue.head_status_level) ||0;         var chest_status_level = parseInt(pvalue.chest_status_level) ||0;         var abdomen_status_level = parseInt(pvalue.abdomen_status_level) ||0;         var left_arm_status_level = parseInt(pvalue.left_arm_status_level) ||0;         var right_arm_status_level = parseInt(pvalue.right_arm_status_level) ||0;         var left_leg_status_level = parseInt(pvalue.left_leg_status_level) ||0;         var right_leg_status_level = parseInt(pvalue.right_leg_status_level) ||0;         var strength_max = parseInt(pvalue.strength_max);         var half_strength = Math.floor(strength_max/2);         var strength;         var damage_initiative_penalty;         // get the highest status level.         var status_level = Math.max(head_status_level, chest_status_level, abdomen_status_level, left_arm_status_level, right_arm_status_level, left_leg_status_level, right_leg_status_level);                  if (head_status_level == 4) {             damage_initiative_penalty = '5';             strength = 0;         } else if (status_level === 4) {             damage_initiative_penalty = '5';             strength = half_strength;         } else if (status_level == 3) {             damage_initiative_penalty = '3';             strength = half_strength;         } else if (status_level == 2) {             damage_initiative_penalty = '1';             strength = strength_max;         } else {             damage_initiative_penalty = '0';             strength = strength_max;         }          setAttrs({             damage_initiative_penalty,             strength         });     }); });

And just for fun, here's a more compact version. In this version, we start with an array of body parts. Then create an array of proper attribute names using the map function. Then use that array and map again, to create a string suitable for the on(change) line. Then rather than creating a variable for every bodypart, we just create one status_level attribute, and loop through each body part and check if it is the max. If its the head and level 4, we set it to 5, to account for the fact its level 4 hits are more severe. Finally we use ternary operators to assign the strength and damage penalties. They look complex, but they are concise form of if statement, that returns only one value. const bodyparts = ['head', 'chest', 'abdomen', 'left_arm', 'right_arm', 'left_leg', 'left_leg', 'right_leg']; const bodyAttributes = bodyparts.map(part => `${part}_status_level`); const bodyChanges = bodyAttributes.map(part => `change:${part}`).join(' '); on(bodyChanges, function() {     getAttrs(bodyAttributes, function(pvalue) {         let status_level = 0;         bodyparts.forEach(part => {             const this_level = parseInt(pvalue[part + '_status_level']) ||0;             status_level = (part === 'head' && this_level === 4) ? 5 : Math.max(this_level, status_level);         });              const strength_max = parseInt(pvalue.strength_max);         const strength = status_level === 5 ? 0 : (status_level >= 3 ? Math.floor(strength_max/2) : strength_max );         const damage_initiative_penalty = status_level >= 4 ? 5 : (status_level === 3 ? 3 : (status_level === 2 ? 2 : 0));              setAttrs({damage_initiative_penalty,strength});     }); });

Wow! Thank you so much GiGs. The Roll20 community has been my coding school - and this was a great lesson! It was of course the simple errors I had made that threw the sheetworker. I had been staring so long and tried to eliminate all such errors - I even rewrote the whole thing. But I guess sometimes you just have to show it to someone... Now I'm gonna dive into the other examples you've given me and se what other scripts I can shorten. Currently the final version you posted interfers with another sheetworker that sets hit capacity for all locations except for the head - so I'll look into that as well.

Spoke too soon. The bug is somwhere else... argh Edit: And I found it and fixed it and now the final script works beautifully. I had forgotten to change the capacites calculaction to use the "strength_max" and not "strength".

Marius said: Currently the final version you posted interfers with another sheetworker that sets hit capacity for all locations except for the head - so I'll look into that as well. Are saying that another sheet worker is affecting the same attributes as this one? If you have multiple sheet workers applying to the same attributes, it's often a good idea to combine them into one sheet worker. This is required if multiple workers change  the same attribute - but I dont know if thats what you're describing. If you post the html for the attributes used in this worker, i can test it. That would be the locations, strength, and damage capcity.

I found the bug. It had nothing to do with your script, I had simply forgotten to update an older one from before I started using the strength_max to differentiate the attributes that are not affected by changes in the current strenght. However, I'll see if more workers are trying to set the same attribute somwhere. I haven't thought about that so it seems likley

Congrats on finding the bug :)