Roll20 uses cookies to improve your experience on our site. Cookies enable you to enjoy certain features, social sharing functionality, and tailor message and display ads to your interests on our site and others. They also help us understand how our site is being used. By continuing to use our site, you consent to our use of cookies. Update your cookie preferences .
×
Create a free account

[Help] Sheet Worker behaving strangely

1612616218

Edited 1612618606
CKenny
Sheet Author
Hello everyone, I am trying to set up a sheet worker that should set the number of ammo for the ammo API on("sheet:opened change:repeating_rangedweapons", function() {     getSectionIDs("rangedweapons", function(idarray) { for(var i=0; i < idarray.length; i++) {        let row = 'repeating_rangedweapons_'+idarray[i]; getAttrs([`${row}_R_weapon_attack`,`${row}_R_weapon_burst`,`${row}_R_weapon_auto`], (values) => { let ammo, attack = values[`${row}_R_weapon_attack`], burst = values[`${row}_R_weapon_burst`], auto = values[`${row}_R_weapon_auto`];          if (attack == '0') { //semi-auto ammo=-burst; } else if (attack == '-10') { // full-auto ammo=-auto; } else { // anything else default to single fire ammo=-1; } let attr = {};           attr[`${row}_id`]=idarray[i]; // id of the repeating element attr[`${row}_ammo`]=ammo; // number of ammo to use setAttrs(attr);      }); } }); }); it work but, for lack of a better term, it 'lag' behind meaning it require 2 executions to apply the change and as the rate of fire is decided by a <select> element (only one execution of the sheet worker) when the user try to switch from single shot (1 ammo) to full auto (7 ammo) the API receive -1 instead of -7 Is there an error in my code? or it is something I have overlooked?
1612632490

Edited 1612632553
GiGs
Pro
Sheet Author
API Scripter
The problem is likely because you have your getAttrs and setAttrs functions inside a loop. This is very bad practice on roll20, due to the asynchronous nature of these functions and how slow they are (especially setAttrs). Part of it might also be that fact your change line is watching the whole repeating section, so every time a value is changed (by setAttrs) it triggers the worker again. It's best to be precise with your on(change) line, and make sure you target only the attributes you want to. Here's a quick edit of your code to solve both of thos issues: on ( 'sheet:opened change:repeating_rangedweapons:r_weapon_attack change:repeating_rangedweapons:r_weapon_burst change:repeating_rangedweapons:r_weapon_auto  remove:repeating_rangedweapons' ,  function  () {      getSectionIDs ( 'rangedweapons' ,  function  ( idarray ) {          // first get the attribute names for all rows in put in one array          const   fieldnames  = [];          idarray . forEach ( id   =>   fieldnames . push (              `repeating_rangedweapons_ ${ id } _R_weapon_attack` ,              `repeating_rangedweapons_ ${ id } _R_weapon_burst` ,              `repeating_rangedweapons_ ${ id } _R_weapon_auto`         ));          // use that array so you can do a single getAttrs statement          getAttrs ( fieldnames ,  values   =>  {              // create a variable to hold all the attribute values you re going to create.              const   attr  = {};              // now loop through the rows again              idarray . forEach ( id   =>  {                  let   row  =  'repeating_rangedweapons_'  +  id ;                  let   ammo ,                      attack  =  values [ ` ${ row } _R_weapon_attack` ],                      burst  =  values [ ` ${ row } _R_weapon_burst` ],                      auto  =  values [ ` ${ row } _R_weapon_auto` ];                  if  ( attack  ==  '0' ) {  //semi-auto                      ammo  = - burst ;                 }  else   if  ( attack  ==  '-10' ) {  // full-auto                      ammo  = - auto ;                 }  else  {  // anything else default to single fire                      ammo  = - 1 ;                 }                  attr [ ` ${ row } _id` ] =  id ;  // id of the repeating element                  attr [ ` ${ row } _ammo` ] =  ammo ;  // number of ammo to use             });              setAttrs ( attr );         });     }); }); Notice the structure: Immediately after getSectionIDs, loop through the entire repeating section, build an array of attribute names including the row id Then end the loop, and do getAttrs just once (not in  loop) on the full set of attributes. create an attribute to hold the saved attribute values for a single setAttrs later. Now do a second loop, again through the repeating section, and calculate your attributes Then after that loop has ended, do the setAttrs. Your layout was very close, but this process of splitting into two loops to isolate the getAttrs and setAttrs functions is not intuitive. It's necessary because those functions are very slow, and dont happen linearly. Once the command is initiated, it goes off and does its own thing, and the rest of the code continues. This - combined with not being precise in your on(change) line, is what causes the lag you saw and the wrong values being set. PS: Note that attribute names on the event line must be in lower case always.
1612635854
CKenny
Sheet Author
Thanks GiGs, your solution is way cleaner than the one I just found : on("sheet:opened change:repeating_rangedweapons", function() { getSectionIDs("rangedweapons", function(idarray) { let setOps={}; let rows = []; let data = []; for(var i=0; i < idarray.length; i++) { let row = 'repeating_rangedweapons_'+idarray[i] rows.push(row); setOps[`${row}_id`]=idarray[i]; data.push(...[`${row}_R_weapon_attack`,`${row}_R_weapon_burst`,`${row}_R_weapon_auto`]); } getAttrs(data, (values) => { let attr = {}; for (var row of rows) { let ammo, attack = values[`${row}_R_weapon_attack`], burst = values[`${row}_R_weapon_burst`], auto = values[`${row}_R_weapon_auto`]; if (attack == '0') { //semi-auto ammo=-burst; } else if (attack == '-10') { // full-auto ammo=-auto; } else { // anything else default to single fire ammo=-1; } attr[`${row}_ammo`]=ammo; } setAttrs(attr); }); setAttrs(setOps); }); }); I'll use your version instead PS: how did you color the code block?
1612637123
GiGs
Pro
Sheet Author
API Scripter
CKenny said: PS: how did you color the code block? I wrote the code in an editor called VS Code, and thats the code highlighting it applies. Copying to roll20 kept the formatting. Its a good idea to write your code in a programming editor, like CS Code, Sublime Text, Atom, or others - they help spot easy syntax errors, letting you focus on real issues.
1612638218
CKenny
Sheet Author
GiGs said: I wrote the code in an editor called VS Code, and thats the code highlighting it applies. Copying to roll20 kept the formatting. Its a good idea to write your code in a programming editor, like CS Code, Sublime Text, Atom, or others - they help spot easy syntax errors, letting you focus on real issues. Weird, that does not seem to be the case with Atom (Roll20 keeping the color formatting I mean)
1612638233
GiGs
Pro
Sheet Author
API Scripter
There are a couple of things that concern me in tis section of the code:                 let ammo,                     attack = values[`${row}_R_weapon_attack`],                     burst = values[`${row}_R_weapon_burst`],                     auto = values[`${row}_R_weapon_auto`];                 if (attack == '0') { //semi-auto                     ammo = -burst;                 } else if (attack == '-10') { // full-auto                     ammo = -auto;                 } else { // anything else default to single fire                     ammo = -1;                 } You are treating burst and auto as if they were numbers (ammo = -burst), which makes me worry what will happn if they ahve an empty string or text value. Also evaluating attack using exact values, instead of < for instance.  If these values are entered via a select and have very limited options, the second one isnt a problem, but the first one might still be. Assuming that, I would coorce the attribute values.stat to numbers and use code like this:                  let   attack  = + values [ ` ${ row } _R_weapon_attack` ] ||  0 ,                      burst  = + values [ ` ${ row } _R_weapon_burst` ] ||  0 ,                      auto  = + values [ ` ${ row } _R_weapon_auto` ] ||  0 ;                  let   ammo  =  attack  === - 10  ? - auto  : ( attack  ===  0  ? - burst  : - 1 ); The first part here, using +values.stat || 0, coerces that attribute to a number, and if its not recognised, sets it to zero. This avoids the empty string problem, and allows you to use triple === in the if statement (comparison of value and data type). The ammo line replaces your if statement with a ternary operator  (worth googling, if you're not familiar), a way to set values concisely. 
1612638292
GiGs
Pro
Sheet Author
API Scripter
CKenny said: GiGs said: I wrote the code in an editor called VS Code, and thats the code highlighting it applies. Copying to roll20 kept the formatting. Its a good idea to write your code in a programming editor, like CS Code, Sublime Text, Atom, or others - they help spot easy syntax errors, letting you focus on real issues. Weird, that does not seem to be the case with Atom (Roll20 keeping the color formatting I mean) I guess its something to do with the way each program formats their files.
1612639098

Edited 1612639205
CKenny
Sheet Author
GiGs said: The ammo line replaces your if statement with a ternary operator  (worth googling, if you're not familiar), a way to set values concisely.  Ternary operators, now that's a thing that I remember to use only one third of the time.