Thats exactly the kind of thing I was warning against. In traditional computing its normal to separate bits of code into separate functions, but thats a bad practice to use with network-based code like roll20. Every one of those functions has its own getAttrs and setAttrs function within it - and the problem is, each of those functions has to send code across the internet to contact roll20's servers, and wait for roll20 to send information back. Aside from the variability of the internet, that puts a load on roll20's own servers - they might have hundreds or thousands of such requests coming in from other games, and your game was to wait in line till roll20 has handled all those. That can be a slow process, especially if you have multiple such commands, and can lag your sheet while its waiting to be updated. its better to structure your code so that you perform only one setAttrs and on getAttrs (though the setAttrs is more important). See the version Kyarnic wrote which didnt have a separate functioncall, so you can see exactly what's going on in that change event. The fact that those change events all have this at the start: if (eventinfo.sourceType && eventinfo.sourceType === "sheetworker") {
return;
} is worrying too. That suggests the writer is trying to keep lag down via a brute force approach, but not really addressing what is causing the lag to happen. That code should not be necessary in normal sheet workers (there are places you'd include something like that, but it should not be a universal thing). If you really needed to put code into functions, imagine instead you had something like: on("change:jack_of_all_trades", function(eventinfo) {
// call functions which don't have setAttrs commands within them // instead they create an object variable containing the updated attributes and values, and return that var jack = update_jack_attr();
var abilities = update_all_ability_checks(); // now merge those objects into a single variable. You can do this with any number of objects var output = {...jack, ...abilities}; // finally use a single setAttrs to update the sheet. setAttrs(output, {silent:true}); // silent:true here does the same work as that code at the start of the worker, blocking sheet workers from continuing.
}); Ideally you'd remove the getAttrs from the functions as well and have a single getAttrs in this code, and pass any attributes needed into the function. But that's not as big a deal as cutting down on the setAttrs functions. The best approach though is to do what kraynic has done, and dispense with the separate function entirely, and copy out the code that is needed from them and up it in the on(change) block - you might need to be thoughtful about combining code if you have two or more functions as above. The only reason to use separate functions is to handle repeated code - if you are doing the same calculation in multiple different workers. And even there, there are often alternative approaches (like the Universal Sheet Worker approach i have on the wiki, or using your function just to perform calculations and return their results: dont set values on the sheet). The conclusion: roll20 character sheets run in a network environment, so code that is good when run on your home computer can be bad for roll20. You haveto think about how to approach that to make your sheet more optimal.
