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

Sheetworkers - setAttrs and silent option

1626295900
Jiboux
Pro
Sheet Author
Compendium Curator
Hello community, I am currently chasing a bug in my character sheet, and am not sure if it is a me-bug or a roll20-bug, so I would like some confirmation on the way setAtttrs works. My understanding is the following: if you have on('change:test',function fnTest(){ ... }); And in another routine setAttrs({ "test" : value }, {silent :true}); The fnTest should not trigger. Is it the correct interpretation ? Is the syntax to set the silent option the right one ? ( I saw this syntax on another post, the code I started with had "silent" instead of  {silent :true}, and I also tried  {"silent" :true}  None of those syntax gave me the expected result. Is anyone using this with success and could provide an example ? Now a bit more explanation on my use case, so that maybe someone has an idea what happens if it is not a bug from roll20. In the Earthdawn ruleset, each attribute has a value (3-18, similar to DnD), and a Step (equivalent to the attribute modifier in DnD) that affects the roll that will be done. The value is just for character creation/evolution, but I keep the traceability of how we came to this value so we have, and we are supposed to only edit this value: So I have Attrib-Dex-Curr = Attrib-Dex-Orig + Attrib-Dex-Race + Attrib-Dex-Increases (for the value set at creation, race bonuses and increases when levelling) Attrib-Dex-Step = a function of Attrib-Dex-Curr. Normally, you are only allowed to update Attrib-Dex-Orig, Attrib-Dex-Race and Attrib-Dex-Increases and the rest will be calculated by the sheet-workers. BUT (and that's where I need the "silent" option to avoid a circular loop), for NPC creation, it is more practical to be able to edit directly Attrib-Dex-Step, and in this case another sheetworker function will backfill the right value in Attrib-Dex-Orig to keep the data consistency). This is how it looks like: var updateAttribCurr = function fnUpdateAttribCurr( attrib, silent ) { /// ... I removed here the code to create the string of each attribute depending on the attribute it is called on    getAttrs( [ curr, race, orig, inc, adjust ], function gaUpdateAttribCurr(values) {     let vals = {};     vals[ curr ] = getInt( values, race) + getInt( values, orig) + getInt( values, inc) + getInt( values, adjust );       if( !silent )       setAttrsLog( vals );       else       setAttrsLog( vals, "silent", function saUpdateAttribCurr() { //Call manually other updates for stuff that depend on Attrib-Dex-Step because you don't want to trigger again backfillOrigAttrib      }       });     }   }); }; // End updateAttribCurr() on("change:attrib-dex-orig change:attrib-dex-race change:attrib-dex-increases", fn(_.bind(updateAttribCurr, {}, "Dex"))); var backfillOrigAttrib = function fnBackfillOrigAttrib( attrib ) {   // I removed the parts to build the strings for the name of each variable   getAttrs( [ step, orig, race, inc, adjust, adjuststep ], function gabackfillOrigAttrib(values) {       var t = ((getInt( values, step) - getInt( values, adjuststep) -2) * 3) + 2 - (getInt( values, inc) + getInt( values, race) + getInt( values, adjust));     var cval = getInt( values, orig);     if( cval < (t - 1) || (t + 1) < cval) {       let vals = {};       vals [ orig ] = t;       setAttrsLog( vals, "silent" );       updateAttribCurr( attrib, true);     }   }); }; // End backfillOrigAttrib()   on("change:dex-step", fn(function ocDexStep(eventInfo) {        if( eventInfo.sourceType === "player" )     backfillOrigAttrib( "Dex" );   })); The behaviour that I expect is that if I change Attrib-Dex-Step by a user action (the test  eventInfo.sourceType === "player"  is another safeguard against a circular loop), backfillOrigAttrib will recalculate the right value for Attrib-Dex-Orig so that the whole dataset is consistent, and write it in silent mode to avoid updateAttribCurr to trigger, and have an infinite loop of update between the 2 functions. Still there are some "depending values" that also have to be upddated so updateAttribCurr will be called manually with a "silent" flag that will inform it to make the necessary updates "with caution" and to not reupdate Attrib-Dex-Step. This corresponds to these lines of code       setAttrsLog( vals, "silent" );       updateAttribCurr( attrib, true); The behaviour that I have in the console is updateAttribCurr is launched twice, one of them being the manual call, and the second being triggered by Attrib-Dex-Orig being updated.  Nota : setAttrsLog is an internal function that is just the same thing as setAttrs, with just a few commented out lines in case I want to send everything to the console. Any idea/recommendation ?
1626309856

Edited 1626310117
Oosh
Sheet Author
API Scripter
The option does need to be provided as a key/value pair in an Object, so: {silent:true} | {['silent']: true} | let options={}; options.silent = true All of those should work. The key needs to be a string match of "silent", and the value needs to be a boolean true. Other truthy values don't seem to work here, including the minified "!0" . I can't see any issue with your 'change:test' code above, though it's obviously incomplete. What happens if you run this (obviously with accompanying HTML for the two number input Attributes): on('change:test', (ev) => { console.info(ev); getAttrs(['test2'], (attrs) => { setAttrs({test2: (parseInt(attrs.test2, 10)||0) + 1}, {silent:true}); }); }); on('change:test2', (ev) => { console.warn(`test2 EV`, ev); }) Does this still trigger for you? I see you're using the callback option for setAttrs as well, worth noting that if you cascade another setAttrs in there it won't be silent unless the option is provided again. What's the contents of setAttrsLog? You say it's the same as setAttrs, but you're passing the parameter "silent" which won't work. Off-topic, I'm also curious about the partial application of underscore's _.bind used here: fn(_.bind(updateAttribCurr, {}, "Dex") My understanding of partial binding is that it's usually used to create a new function, I'm trying to work out how calling it anonymously is any different to just calling the function directly with updateAttribCurr('Dex');
1626365605

Edited 1626365670
GiGs
Pro
Sheet Author
API Scripter
I don't have an answer for your issue, and I think it's because there's a lot of code missing. I am curious why your functions are written the way they are.Just in case they are relevant, why is this function on("change:dex-step", fn(function ocDexStep(eventInfo) {        if( eventInfo.sourceType === "player" )     backfillOrigAttrib( "Dex" );   })); not written like this on("change:dex-step", function ocDexStep(eventInfo) {        if( eventInfo.sourceType === "player" )             backfillOrigAttrib( "Dex" );   }); And why do these lines  getAttrs( [ curr, race, orig, inc, adjust ], function gaUpdateAttribCurr(values) {   getAttrs( [ step, orig, race, inc, adjust, adjuststep ], function gabackfillOrigAttrib(values) { have function names when you could write them like  getAttrs( [ curr, race, orig, inc, adjust ], function (values) {   getAttrs( [ step, orig, race, inc, adjust, adjuststep ], function (values) { I also have the same question as Oosh about the bind function. This line: on("change:attrib-dex-orig change:attrib-dex-race change:attrib-dex-increases", fn(_.bind(updateAttribCurr, {}, "Dex"))); could just as easily be on("change:attrib-dex-orig change:attrib-dex-race change:attrib-dex-increases", updateAttribCurr("Dex")); with the updateAttribCurr possibly being rewritten to handle that format, if necessary. It seems like that would be fine though. So a lot of your code seems a bit more convoluted than it needs to be, and if there's no compelling reason for it to be written that way, I'd simplify the code and see if your problem goes away. You never know. I also second Oosh's interest in setAttrsLog - since it seems to be doing important work, the problem might be in that function, and we can't see its code.
1626472806
Jiboux
Pro
Sheet Author
Compendium Curator
Thanks to both of you for your support on this. 1- I have tried Oosh test code, and it works as intended, which confirms the problem is in my code. 2-Regarding the convoluted syntaxes that surprise both of you, like setAttrslog, or the  fn(function ocDexStep(eventInfo)  both follow the same intention. First it must be understood that I didn't develop this sheet originally; and I partner with its original developper for a new release. He is more expert than me in JS, and has worked on the sheet since several years. The intention in both cases is instead of using the "normal" syntax, to go through a locally defined function that calls back the default syntax, but includes some commented out console logging lines, that can conveniently be commented back in to do debugging. // This is just like setAttrs(), except that before it sets the attrs, it writes net new values to the console log. // NOTE That when not actively debugging, all lines except the last can/should be commented out. var setAttrsLog = function fnlogvalues( vals, opt, cb ) {   // var txt = "setAttrs -";   // for(var key in vals)   // txt += " " + key + ": " + vals[ key ] ;   // console.log( txt );   setAttrs( vals, opt, cb ); }; // This is a wrapper for call backs that log when callback is entered and exited. // The console.log lines should be commented out when not actively  debugging. var fn = function fnfn(cb){   'use strict';   return function fnfn2(){     //            console.log('Entering: '+(cb.name||'(anonymous function)'));     cb.apply({},arguments);     //            console.log('Exiting: '+(cb.name||'(anonymous function)'));   }; }; I have to tried to bypass on or the other, to no improvement 3- Regarding the _.bind, this has been a point of discussion with the original developper, as I also found it convoluted. The direct syntax you proposed poses other problems, but I also did via a direct call to the function, and getting the appropriate data from eventInfo. Didn't improve either. Just in case you would see something out of the lines I removed for filtering, here is a more complete code. Thanks A LOT for your support // calculate the final Value for the @attrib attribute // silent means that we are coming from backfillOrigAttrib() and to avoid any possibility of a race condition we need to make subsequent calls manually instead of using events. var updateAttribCurr = function fnUpdateAttribCurr( attrib, silent ) {   'use strict';    log ( "EarthDawn : UpdateAttribCurr( " + attrib +" , " + silent + " )"  );   var curr = "Attrib-" + attrib + "-Curr";   var orig = "Attrib-" + attrib + "-Orig";   var race = "Attrib-" + attrib + "-Race";   let inc = "Attrib-" + attrib + "-Increases";   var adjust = attrib + "-Adjust-attr";   getAttrs( [ curr, race, orig, inc, adjust ], function gaUpdateAttribCurr(values) {     'use strict';     logvalues( values);     let vals = {};     vals[ curr ] = getInt( values, race) + getInt( values, orig) + getInt( values, inc) + getInt( values, adjust );     if( vals[ curr ] != getInt( values, curr) ) {       if( !silent )       setAttrsLog( vals );       else       setAttrsLog( vals, {silent:true}, function saUpdateAttribCurr() {         'use strict';         // updateAttribStep( attrib, silent); JBF@CD 20210713 We don't want this line. When silent, the step update triggered the whole chain, so we don't want to update it a second time         switch ( attrib ) {           case "Dex":           updateDefensebase( "Dex", "PD");           break;           case "Str":           updateCarry();           break;           case "Tou":           updateHealth();           break;           case "Per":           updateDefensebase( "Per", "MD");           break;           case "Wil":           updateMaBase();           break;           case "Cha":           updateDefensebase( "Cha", "SD");           break;         }       });     }   }); }; // End updateAttribCurr() on("change:attrib-dex-orig change:attrib-dex-race change:attrib-dex-increases change:dex-adjust-attr", fn(_.partial(updateAttribCurr, "Dex", false))); on("change:attrib-str-orig change:attrib-str-race change:attrib-str-increases change:str-adjust-attr", fn(_.partial(updateAttribCurr, "Str", false))); on("change:attrib-tou-orig change:attrib-tou-race change:attrib-tou-increases change:tou-adjust-attr", fn(_.partial(updateAttribCurr, "Tou", false))); on("change:attrib-per-orig change:attrib-per-race change:attrib-per-increases change:per-adjust-attr", fn(_.partial(updateAttribCurr, "Per", false))); on("change:attrib-wil-orig change:attrib-wil-race change:attrib-wil-increases change:wil-adjust-attr", fn(_.partial(updateAttribCurr, "Wil", false))); on("change:attrib-cha-orig change:attrib-cha-race change:attrib-cha-increases change:cha-adjust-attr", fn(_.partial(updateAttribCurr, "Cha", false))); //JBF other triggers backfillOrigAttrib and Recalc // If somebody has changed the attribute step, go back and change the orig to match, // Note: if you change this code, make sure that updateAttribStep() is also updated and tested to not make an infinite loop due to circular reference does not result. // IE: make sure that this produces a correct value that updateAttribStep() will not attempt to "correct" and visa versa. // Also, to ensure no circular loop, backfillOrigAttrib won't be triggered by the API. the API Routine modify() in textImport() will have to be in sync with backfillOrigAttrib and updateAttribStep var backfillOrigAttrib = function fnBackfillOrigAttrib( attrib ) {   'use strict';   log ( "EarthDawn : BackfillOrigAttrib( "+ attrib + " )");   var step = attrib + "-Step";   var orig = "Attrib-" + attrib + "-Orig";   var race = "Attrib-" + attrib + "-Race";   let inc = "Attrib-" + attrib + "-Increases";   var adjust = attrib + "-Adjust-attr";   var adjuststep = attrib + "-Adjust";   getAttrs( [ step, orig, race, inc, adjust, adjuststep ], function gabackfillOrigAttrib(values) {     'use strict';     logvalues( values);     var t = ((getInt( values, step) - getInt( values, adjuststep) -2) * 3) + 2 - (getInt( values, inc) + getInt( values, race) + getInt( values, adjust));     var cval = getInt( values, orig);     if( cval < (t - 1) || (t + 1) < cval) {       let vals = {};       vals [ orig ] = t;       setAttrsLog( vals, {silent:true} );       updateAttribCurr( attrib, true);     }   }); }; // End backfillOrigAttrib() on("change:dex-step", fn(function ocDexStep(eventInfo) {     'use strict';     updateAttribFinal( "Dex" );        if( eventInfo.sourceType === "player" )     backfillOrigAttrib( "Dex" );   })); The reason I know something is wrong, is that in the console, I get. I should not get the second UpdateAttribCurr [Log] EarthDawn : updateAttribFinal( Str ) (sheetsandboxworker.js, line 126) [Log] EarthDawn : BackfillOrigAttrib( Str ) (sheetsandboxworker.js, line 126) [Log] EarthDawn : UpdateAttribCurr( Str , true ) (sheetsandboxworker.js, line 126) [Log] EarthDawn : UpdateAttribCurr( Str , false ) (sheetsandboxworker.js, line 126)
1626476927
GiGs
Pro
Sheet Author
API Scripter
The problem has to be at least partly in the code that writes values to the character sheet, the setAttrsLog function, and we can't see that so I don't see how we can help you.
1626478890

Edited 1626478902
Scott C.
Forum Champion
Sheet Author
API Scripter
Compendium Curator
I mean, the setAttrsLog function is right at the top of Jiboux's last post. I don't see anything in it that would cause a problem. Looks very like how I alias setAttrs in my sheetworkers as well. The thing that jumps out at me is to check if your check for silent or not: if( !silent ) setAttrsLog( vals ); else setAttrsLog( vals, {silent:true}, function saUpdateAttribCurr() { is actually working. I'd suggest rewriting it with the block sytnax and adding some debugs/logs to make sure the if you think is being passed, is actually being passed. e.g.: if( !silent ){ console.log(`silent was not found`); setAttrsLog( vals ); }else{ console.log(`silent was found); setAttrsLog( vals, {silent:true}, function saUpdateAttribCurr() { //Rest of your function call back code here Now, assuming that none of that is the issue, I'm having a little trouble following the updateAttribCurr code, but I'm wondering if your callback on the setAttrs in the silent case is where you are seeing your trigger happen. Is the setAttrs used in those functions (e.g. updateDefensebase) using silent?
1626486480
GiGs
Pro
Sheet Author
API Scripter
Oh well spotted, Scott - I somehow missed it was there!
1626486532
Jiboux
Pro
Sheet Author
Compendium Curator
Thanks Scott... In fact in the code I showed, the problem with setAttrs silent is present twice. The one I was chasing is the one at the end of BackfillOrigAttrib. if( cval < (t - 1) || (t + 1) < cval) {       let vals = {};       vals [ orig ] = t;       setAttrsLog( vals, {silent:true} );       updateAttribCurr( attrib, true);     } But you rightfully pointed out a second occurence, where exactly the same happens. The point of this is that I have a chain of dependencies, Orig+Race+Adjust===> Curr (UpdateAttribCurr) ===> Step and Defense (UpdateDefenseBase) In a normal behaviour, only Orig/Race/Adjust can be edited, and this will trigger a chain of update functions. BackfillOrigAttrib is intended to allow an exceptional case of editing the end value, and backfilling the appropriate values at the beginning of the chain. Hence why we try to control the triggering of each subsequent function via a silent setAttrs, and a manual call to it, in order to avoid potential racing conditions and circular loops. The abnormal behaviour is that the logs demonstrate that the function ran twice, once for the manual call (as intended) and once by change trigger (which should not happen) Regarding the tests done: -Following the remark of GiGs I tried to replace setAttrsLog by setAttrs. No improvement. -I checked that the triggers were originated where I thought by removing completely the setAttrs. -I placed logs to follow that the code was going through the actual (silent) and it does... No clue...
1626489360
Jiboux
Pro
Sheet Author
Compendium Curator
OK, I think I got it, and it reminds me of another post I read somewhere else... At the end I begin to think again that this is a roll20 thing and not mine... I began to carve out as much code as I could until I could have logs that had a normal behaviour (i.e. the updateAttribCurr being called only once, and not twice). When I finally got to a normal behaviour, I tried re-introducing the deleted pieces of code until the abnormal behaviour re-appeared... It ended up that the difference between a functional code, and a non-functional code were 2 other functions that shared the same original trigger (dex-step) , but acted and completely different variables... When the change:dex-step &nbsp;was removed for BOTH of these function, I had the normal behaviour I expected with the silent.&nbsp; Interesting fact I had a third function that had that had the&nbsp; change:dex-step; &nbsp;but this one didn't seem to have any influence... But my tests being in a new character, it didn't trigger any SetAttrs. This looks a lot like what was described in the attached thread that never got responded <a href="https://app.roll20.net/forum/post/8974469/slug%7D" rel="nofollow">https://app.roll20.net/forum/post/8974469/slug%7D</a> It looks like if you have several functions triggering at once, and one is trying to make a silent setAttrs, but the others are also making a setAttrs, it "interferes"
1626493112

Edited 1626493891
Scott C.
Forum Champion
Sheet Author
API Scripter
Compendium Curator
Wait, you have multiple listeners for the same attribute that trigger different function calls which all do a setAttrs (and a getAttrs)? That's not a Roll20 thing, that is just how event listeners work. The listener listens for what you tell it to listen to. I think you found the cause, but misdiagnosed it. I'm willing to bet that what you have happening is a race condition (or something similar) where you have a thing triggering multiple functions which are all doing separate getAttrs and setAttrs. Since these are asynchronous they are completing in a more or less random order (although likely to frequently resemble the expected order) and are each then calling a set so that the code you think was causing the reaction despite having silent:true &nbsp;on is in fact not doing anything while the other listeners and their associated functions are the ones triggering changes that then cause your listener that shouldn't be triggered to run. (For instance, what's the code for updateDefensebase look like?) GiGs may argue with me*, but the code here is a prime example of what I argue against when I say that sheetworkers should have one setAttrs, one getAttrs, and have NO cascades of setAttrs -&gt; getAttrs -&gt; setAttrs (or setAttrs -&gt; setAttrs -&gt; setAttrs -&gt; etc as is seen here). From what I'm seeing here, it looks like you probably have a cascade of at least 3, and probably more like 4 or 5 setAttrs triggering one after the other. This is not only inefficient (you're probably introducing a several second lag to the sheet), it's also damn hard to track down bugs and errors in a set up like this as you are experiencing. EDIT: * Actually I think you'd agree with me in this instance GiGs ;)
1626496951
GiGs
Pro
Sheet Author
API Scripter
Scott, you're correct, I do agree with you here :) I do try to avoid having multiple different workers triggering off the same change event. You should combine such workers into one when you can.
1626529828
Jiboux
Pro
Sheet Author
Compendium Curator
Yes, our sheet is built around chain triggers... I am not sure why it should be such a bad practice if the functions like setting settatrs in silent are working as they should... I can tell you for sure that from the chopping I did, and the logs I have it is not a race condition where one of the function would rewrite one of the attributes already triggered... It really looks a lot alike the other bug report that I attached. Note that with appropriate logging we are usually able to track down bugs (but this is one of the reason why we name functions, so that the name of the function where something happened appears in the console)... Up to now the hardest bug we had to track were when we assumed some roll20 function behaved a certain way and found out that it didn't (like this case, or like vandalized rowids) There are a LOT of dependencies in our system, that make the whole variable system like a tree of dependencies that have to be explored... The split between the routines and triggers is done to explore the tree in a certain direction so that you don't have race conditions. If a same variable is going to influence different branches of the tree that are completely independant, we made the choice to explore these branches asynchronously with various routine triggering simultaneously and updating the rest. By design, we split the branches of the tree in a way that they don't overlap, and that the tree is explored unidirectionally. The ONLY case where we want to go through the tree in reverse direction is this one where precisely we wanted to make sure to avoid double triggers by setting in silent. At the end the sourcetype test is a workaround to deal with the fact that a setAttrs in silent mode actually triggers the function &nbsp; &nbsp; &nbsp; &nbsp;if( eventInfo.sourceType === "player" ) &nbsp; &nbsp; backfillOrigAttrib( "Dex" ); I understand your recommendation to group all under a single setAttrs and getAttrs, but seems like working around limitations of the system... At the end it means that if you have dependencies like: A+B+C =D E=F+G D+E = H ...etc Instead of using the events to explore the tree of dependencies (if A changed, D has to be recalculated, if D changed H is recalculated, but that doesn meant you have to go back to check F and G), you will have one single wrapper function that whenevr there is one thing changing will have to check ALL the conditions and replace the asynchronous change : by synchronous if () ? The way I would code the above is (I write quick, so don't mind the syntax) UpdateD(){ getattrs(A,B,C) let D=A+B+C seattrs (D) } on(change:A change:B &nbsp; change:C) &nbsp;updateD UpdateE(){ getattrs(F,G) let E=F+G seattrs (E) } on(change:F&nbsp; change: G ) &nbsp;updateE ... What I understand is your recommandation is UpdateAll(){ let E=F+G let H=D+G getattrs(All) let D=A+B+C seattrs (All) } on(change:Any ) &nbsp;updateAll As our code is nearly finished, and this is the only case we have, that we knew had to take care, I'll trust the eventInfo information instead of the silent. In any case, even if I seem to argue on the choices we made, I want to thank you 2 a lot for the time you took. It is very nice !
1626537293

Edited 1626537434
GiGs
Pro
Sheet Author
API Scripter
At the risk of giving Scott a heart attack, I want to try to explain why he is completely correct in his advice in this case. But first, to those reading along who have seen us butt heads (politely) over the topic of cascades, I want to clarify something. Most of my advice on the forums is focused on beginners. I've had a surprising amount of IMs over the years from people who have told me that they really didn't understand other people's attempts to help them, but my help had opened their eyes and let them make progress. So when I advise people to not worry about cascades too much, it's because I don't want really complicated coding practices to be recommended to beginners as a first practice when for most of them, it's not necessary. Yes, it's better in an ideal world, but it creates a larger barrier for entry - a lot of beginner coders will find the coding too hard and give up. The Roll20 character sheet repository is as successful and as big as it is because it is friendly to beginners. Lots of sheets are full of bad programming practices, but in most cases, it doesn't matter - they still work fine (because most sheets arent as complex as, say, D&amp;D or pathfinder, and can bear to be inefficient). But this sheet - this sheet is written by people who are very competent, and who are more than capable of following Scott's advice on code. The problem in this sheet might be that you are very familiar with writing code for local clients, but not familiar with the needs of network code. I'll snip out two comments, the second being most important: Jiboux said: Yes, our sheet is built around chain triggers... I am not sure why it should be such a bad practice if the functions like setting settatrs in silent are working as they should... &lt;snip&gt; I understand your recommendation to group all under a single setAttrs and getAttrs, but seems like working around limitations of the system... Yes, Scott's advice is indeed about working around the limitations of the system. But these are very real limitations, and unavoidable , so you do need to take them into account. Do you understand why getAttrs and setAttrs are asynchronous? This is because you are working in a network environment. Most of the code is running on the client computer, but whenever you make a getAttrs or setAttrs call, at that point the client has to contact roll20's servers, across the internet, wait for a response, and then receive the response. This process is much, much slower than anything running on the client, and it's asynchronous so the rest of the code doesn't hang while waiting for the response. It means that you can have multiple getAttrs and setAttrs operations running at the same time, and every single one introduces lag, and sometimes unplanned interactions because of the unpredictable ordering - you cant tell which getAttrs and which setAttrs will finish first. So, if you have a very complex sheet, and you know how to write the code, you really should follow Scott's advice about rewriting the code to get rid of cascades as much as you can. Scott's method as I understand (feel free to correct me, Scott) it is to basically turn the entire set of sheet workers into a single sheet worker, with one getAttrs that captures all the attributes on the sheet, and when any one attribute changes, calculates all interactions from that change, builds a set of all attributes that have changed, and then use one setAttrs to update the sheet. That used to sound bonkers to me, so I understand the hesitation to adopt such a process, but its the crazy kind of genius that works magnificently when you have the sheet set up correctly (and are working with a sheet complex enough to demand such a process). It does require a coder or two that really know their stuff, and will mean that sheet will probably get abandoned if you ever move away, because no one who comes in to try to take over will understand it, but your sheet has that problem already because the code is already complex. With a process like this, you can still follow the individual changes with your logging, and keep track of everything that is going on, but you have much more control over the procedure and ordering, and I have no doubt that the current issue would go away - or you'd be able to spot what was causing it.
1626542340

Edited 1626542392
Scott C.
Forum Champion
Sheet Author
API Scripter
Compendium Curator
Heh, nailed it in one GiGs. Although I'll note I don't necessarily run everything through the same sheetworker as some times it just doesn't make sense (e.g. no reason to grab character only attributes if you change something starship related on the Starfinder sheet). But, that's the essence of it, and really the important part is the getAttrs. There's really no reason not to grab every single attribute that you may need before starting to calculate attributes as the time to get the attribute values is not affected by the number of attributes you are getting. Now, at some sort of unreasonable extreme (say 1,000,000 attributes for hyperbole's sake) you may run into a memory issue, but that's unlikely to occur with any sheet I've ever seen. The data contained in attributes and returned by getAttrs is relatively simple when compared to what modern browsers and computers are capable of; it's just numbers and strings after all. As for the ease of collaboration and others coding, I may be biased, but I think that it can actually make the code more maintainable. When done correctly, it reduces the number of places where edits need to be made to accommodate any given change. And, to bring this back around to Jiboux's original request for help. I agree with GiGs, this code really needs a serious refactor to make it simpler to understand and more efficient. I know that is a ton of work, I'm working on a major refactor of another big sheet as well, but the amount of time that it will save you in the long run is immeasurable when plotted out over the maintenance life of the sheet.
1626584101

Edited 1626584276
Chris D.
Pro
Sheet Author
API Scripter
Compendium Curator
Maybe my issues with regard to this discussion is that I have not really spent much time looking at how other sheets are put together, I just started putting my sheet together from scratch (using cascading triggers), but I want to clarify the technique that you are talking about (probably not to refactor the sheet immediately, but just to understand what would need to be done if I were to attempt it).&nbsp; So it seems to me (without ever having written a sheet that avoids cascades) that there are two ways to avoid cascades.&nbsp; (a) On any Trigger from the player (not sheetworkers or api), Get everything, Recalculate Everything, write everything that changed.&nbsp; (b) basically recreate the trigger mechanism in your own code.&nbsp; On a trigger, see what attributes are triggered by that, and see what those triggered events might result in, then see what is triggered by those things and what they might in turn trigger, repeat until run out of dependencies. Then do one getattrs, recalculate synchronously the various triggers in order. Do one write.&nbsp; So basically everywhere that I currently have one routine and its triggers, I would have two routines (or two ways a single routine could be called. I would have something that reports its dependencies and its outputs, to be used by the master routine to walk my triggers. And the routine that actually calculates my values. If the user changes G, then the system checks and discovers that G is used in H and M. H is used nowhere else, but M is used to calculate N, O, and P. None of which are used elsewhere. So the system reads all the inputs to H, M, N, O, and P, Calculates them and writes the new values. Correct? So maybe my problem is that I tend to think that (a) is overkill and keep thinking that (b) is the appropriate way to do it, but that seems like more coding work. ie: (a) does much bigger reads and more calculations. (b) requires a master routine and a structure that the master routine can walk, more complex but certainly much less reading and calculating of values that don't actually change.&nbsp; So what method do most people use? (or something else altogether?) Also, with ether (a) or (b), how do people do repeating sections? Lets say I change something outside of a repeating section such as "Dex", but attributes such as that are used inside of several repeating sections. Using both (a) or (b) do you just get a list of all repeating rowIDs and then get all things inside the repeating section that could depend on attributes? If using (a) or (b) then I would think that would made for some truly massive reads.&nbsp;
1626584247

Edited 1626584368
GiGs
Pro
Sheet Author
API Scripter
Chris, that might be better asked in a new more general thread, to avoid further derailing Jiboux's question. Maybe with a title like Discussion on Best Practices for Cascading Sheet Workers, though that's a mouthful.
1626586633
Chris D.
Pro
Sheet Author
API Scripter
Compendium Curator
Fair enough. <a href="https://app.roll20.net/forum/post/10254338/discussion-on-best-practices-for-cascading-sheet-workers/?pageforid=10254338#post-10254338" rel="nofollow">https://app.roll20.net/forum/post/10254338/discussion-on-best-practices-for-cascading-sheet-workers/?pageforid=10254338#post-10254338</a>
1626621610
Jiboux
Pro
Sheet Author
Compendium Curator
Just to clarify that Chris is not derailing... Chris is the original coder of the sheet I am working on, and we are cooperating on this one... But true we are moving away from the original question... Thanks GiGs for taking the time to explain your point of view so clearly...
1626622695
GiGs
Pro
Sheet Author
API Scripter
ah, I didnt realise Chris was the original author, so it would have been very relevant to this sheet. Oops.