[Migration] Moving repeating sections

Hello. I have a small issue while trying to devise a migration script for a sheet. The thing is that I have the same repeating section used for different things and I want to split them up. I found a way to do it, but I think it is awful and I'd be glad if you could give me a hand there. Here is part of my migration code getSectionIDs(["repeating_Talents"], function(idarray) { for(var i=0; i < idarray.length; i++) { console.log(idarray[i]); getAttrs( ["repeating_Talents_"+idarray[i]+"_mode-solo", "repeating_Talents_"+idarray[i]+"_modePage-solo", "repeating_Talents_"+idarray[i]+"_mode-squad", "repeating_Talents_"+idarray[i]+"_modePage-squad"], function (values) { // RETURNS "undefined" :-( console.log(idarray[i]); var newSoloRowId = getNewRowId("repeating_SoloModeAbilities_"); var newSquadRowId = getNewRowId("repeating_SquadModeAbilities_"); var newrowattrs = {};                         // DOES NOT WORK BECAUSE IDARRAY[I] IS UNDEFINED :-( newrowattrs[newSoloRowId+"_mode-solo"] = values("repeating_Talents_"+idarray[i]+"_mode-solo"); newrowattrs[newSoloRowId+"_modePage-solo"] = values("repeating_Talents_"+idarray[i]+"_modePage-solo"); newrowattrs[newSquadRowId+"_mode-squad"] = values("repeating_Talents_"+idarray[i]+"_mode-squad"); newrowattrs[newSquadRowId+"_modePage-squad"] = values("repeating_Talents_"+idarray[i]+"_modePage-solo"); setAttrs(newrowattrs); }); } }); As you may see in the comments, I cannot get the value names because when I'm in the callback method of getAttrs, I cannot get the value of the id. I was able to circumvent the problem by doing this, but I really don't like it. let modeSolo; let modePageSolo; let modeSquad; let modePageSquad; for (const property in values) { if(property.endsWith("mode-solo")) { modeSolo = values[property]; } if(property.endsWith("modePage-solo")) { modePageSolo = values[property]; } if(property.endsWith("mode-squad")) { modeSquad = values[property]; } if(property.endsWith("modePage-squad")) { modePageSquad = values[property]; } } Do you have an idea to do that better? Any other hint is nice to have to. I'm far from being a professional JS dev. (but I guess you could gather it from what I wrote :-D)

When manipulating repeating sections, it's very common to come up with code that puts a getAttrs and a setAttrs inside a loop. This is a bad idea for efficiency reasons, and as a plus side, you'll end up with better code if you rewrite it to avoid this. I'm off out now, but I have written about this before. Check this out: <a href="https://cybersphere.me/getsectionids-and-the-structure-of-a-repeating-section/" rel="nofollow">https://cybersphere.me/getsectionids-and-the-structure-of-a-repeating-section/</a>

By the way, your code includes these lines var newSoloRowId = getNewRowId("repeating_SoloModeAbilities_"); var newSquadRowId = getNewRowId("repeating_SquadModeAbilities_"); but it doesn't show the contents of the getNewRowId() function. That means we cant tell what is going on in lines that start like this: newrowattrs[newSoloRowId+"_mode-solo"] = The problem you're having might be in that funtion. Also, are you sure the idarray[i] is returning undefined? Unless I'm missing something, it looks to me like both console.log statements there should give a value.

The function getNewRowId just get a new ID for a repeating section. I’m aggregating the new row ids in a list to be sure that I don’t accidentally create two rows with the same id for a repeating section. So this function is more or less what you can see here : <a href="https://wiki.roll20.net/Sheet_Worker_Scripts#generateRowID.28.29" rel="nofollow">https://wiki.roll20.net/Sheet_Worker_Scripts#generateRowID.28.29</a> Also, are you sure the idarray[i] is returning undefined? Absolutely. I tried specifically the code I pasted to be sure it was really undefined. It’s really strange for me, because in a C# code, you can use variables from higher scopes, and there, it seems that for some variables, I can’t. However, on a second thought, I didn’t check if idarray was returning undefined or if it was i .

Hi again. I think the issue was that i was undefined. Since getAttrs is async, there's no way I could retain the value of the index I was iterating over. I followed your advices and here is the full script. Let me know if you think anything's off : on("clicked:migrate", () =&gt; { const newRowIds = []; const newrowattrs = []; function getNewRowId(repeatingSection) { var newRowId = repeatingSection + generateRowID(); while(newRowId in newRowIds){ newRowId = repeatingSection + generateRowID(); } newRowIds.push(newRowId); return newRowId; } function addAttribute(key, value) { if (typeof value === 'undefined') { return; } newrowattrs[key] = value; } getAttrs(["mode-solo", "modePage-solo", "mode-squad", "modePage-squad"], values =&gt; { const newSoloRowId = getNewRowId("repeating_SoloModeAbilities_"); const newSquadRowId = getNewRowId("repeating_SquadModeAbilities_"); addAttribute(`${newSoloRowId}_mode-solo`, values["mode-solo"]); addAttribute(`${newSoloRowId}_modePage-solo`, values["modePage-solo"]); addAttribute(`${newSquadRowId}_mode-squad`, values["mode-squad"]); addAttribute(`${newSquadRowId}_modePage-squad`, values["modePage-squad"]); }); getSectionIDs(["repeating_Talents"], ids =&gt; { const attributes = []; ids.forEach(id =&gt; { attributes.push(`repeating_Talents_${id}_mode-solo`, `repeating_Talents_${id}_modePage-solo`, `repeating_Talents_${id}_mode-squad`, `repeating_Talents_${id}_modePage-squad`); }); console.log(attributes); getAttrs(attributes, values =&gt; { ids.forEach(id =&gt; { const newSoloRowId = getNewRowId("repeating_SoloModeAbilities_"); const newSquadRowId = getNewRowId("repeating_SquadModeAbilities_"); addAttribute(`${newSoloRowId}_mode-solo`, values[`repeating_Talents_${id}_mode-solo`]); addAttribute(`${newSoloRowId}_modePage-solo`, values[`repeating_Talents_${id}_modePage-solo`]); addAttribute(`${newSquadRowId}_mode-squad`, values[`repeating_Talents_${id}_mode-squad`]); addAttribute(`${newSquadRowId}_modePage-squad`, values[`repeating_Talents_${id}_modePage-squad`]); }); setAttrs(newrowattrs); }); }); });

Baleine82 said: Absolutely. I tried specifically the code I pasted to be sure it was really undefined. It’s really strange for me, because in a C# code, you can use variables from higher scopes , and there, it seems that for some variables, I can’t. However, on a second thought, I didn’t check if idarray was returning undefined or if it was i . You can do that in JavaScript, too, which is why I asked. I'd think i would maintain its value, too, so I'm not sure what as going on there.

Baleine82 said: I followed your advices and here is the full script. Let me know if you think anything's off Based on a very quick overview, it looks better. The big question is, does it work?

GiGs said: Based on a very quick overview, it looks better. The big question is, does it work? Ouch, you hurt me by thinking I would not test on my machine before giving away some code. Even if I'm no web developer, I'm still a pro (but I make mistakes :-D, and there is a potential race condition in the code I copied). Granted, that may not cover every condition, but I'll test that more thoroughly. I think I'm on better rails now. I even changed the script further by calling a single getAttrs and setAttrs . Thanks for the help :-)

Did you notice you (still) didn't answer my question? I don't know how to reply, or if a reply is needed, withhout that information.

Oh, sorry, I didn't intend to cross you.:-$ Yeah it works. I did several tries on a sheet Yet, I'm sure I've still got a lot to learn, that's why I asked if you saw something that could be improved

I'd better add that to my signature: don't cross me ;) At the top of your worker, you have const newrowattrs = []; Shouldn't that be an object, not an array, like: const newrowattrs = {}; You have this function: function getNewRowId(repeatingSection) { var newRowId = repeatingSection + generateRowID(); while(newRowId in newRowIds){ newRowId = repeatingSection + generateRowID(); } newRowIds.push(newRowId); return newRowId; } For the record, in the array syntax that would be const getNewRowId = repeatingSection =&gt; repeatingSection + generateRowID(); or const getNewRowId = repeatingSection =&gt; `${repeatingSection}${generateRowID()}`; I wouldnt both defeating repeatingSection with the 'repeating_' part - I'd include that in the function, like so: const getNewRowId = section =&gt; `repeating_${section}_${generateRowID()}`; That brings to mind a function i use a lot, which seems like it would fit in this worker: const section_name = (section, id, field) =&gt; `repeating_${section)_${id}_${field)`; Placed in more familiar syntax: function section_name(section, id, field) { var new_name = 'repeating_' + section + '_' + id + '_' + field; return new_name; } So, you'd call it like section_name('SoloModeAbilities', this_id, 'mode-solo') . this_id here is a variable. Two pieces of general advice, not just for this worker but for everything that is intented to go through sheet workers: Use lower case for everything. Don't have repeating_SoloModeAbilities when you can have repeating_solomodeabilities . Assume everything will be a variable in javascript at some point, and so name it only using letters, numbers, and underscores (and always start with a letter). Don't name something mode-solo when you can use mode_solo Both of these suggestions have good sense reasons - you can ignore them, but it will make your job easier in the long run if you observe them.

GiGs said: At the top of your worker, you have const newrowattrs = []; Shouldn't that be an object, not an array, like: const newrowattrs = {}; Err… I don't know. newrowattrs will get all new attributes that we want to save. I think I saw both [] and {} be used, didn't knew the difference, and thought it was just synctatic differences. So I should use an object? GiGs said: You have this function: function getNewRowId(repeatingSection) { var newRowId = repeatingSection + generateRowID(); while(newRowId in newRowIds){ newRowId = repeatingSection + generateRowID(); } newRowIds.push(newRowId); return newRowId; } For the record, in the array syntax that would be const getNewRowId = repeatingSection =&gt; repeatingSection + generateRowID(); or const getNewRowId = repeatingSection =&gt; `${repeatingSection}${generateRowID()}`; I followed the guidelines given in <a href="https://wiki.roll20.net/Sheet_Worker_Scripts#generateRowID.28.29" rel="nofollow">https://wiki.roll20.net/Sheet_Worker_Scripts#generateRowID.28.29</a> . Do you mean I don't need to check that the id isn't already existing? Thanks for your tips. I'll be applying them :)

Baleine82 said: GiGs said: At the top of your worker, you have const newrowattrs = []; Shouldn't that be an object, not an array, like: const newrowattrs = {}; Err… I don't know. newrowattrs will get all new attributes that we want to save. I think I saw both [] and {} be used, didn't knew the difference, and thought it was just synctatic differences. So I should use an object? The difference between [] and {} is the first is for arrays, the second is for objects. I didn't realise what you were doing (if I am following it) would actually work. If you are using a variable to grab the names of an attribute and its value, and storing them together to send to setAttrs, that should be an object. Maybe JavaScript is transforming your variable type - only using console.log will reveal what is actually happening. GiGs said: You have this function: function getNewRowId(repeatingSection) { var newRowId = repeatingSection + generateRowID(); while(newRowId in newRowIds){ newRowId = repeatingSection + generateRowID(); } newRowIds.push(newRowId); return newRowId; } For the record, in the arrow syntax that would be const getNewRowId = repeatingSection =&gt; repeatingSection + generateRowID(); or const getNewRowId = repeatingSection =&gt; `${repeatingSection}${generateRowID()}`; I followed the guidelines given in <a href="https://wiki.roll20.net/Sheet_Worker_Scripts#generateRowID.28.29" rel="nofollow">https://wiki.roll20.net/Sheet_Worker_Scripts#generateRowID.28.29</a> . Do you mean I don't need to check that the id isn't already existing? Following those guidelines is fine and will work, but bear in mind they predate some significant changes in JavaScript and may be outdated. "Do you mean I don't need to check that the id isn't already existing?" If the structure of your code is solid and you know that id will exist, there's no need to check for it. That advice could be a bit of a slippery slope, but in my experience, character sheet code does not generally include much error-checking (and usually doesn't need much).

Ok, thank you for sharing your insights. I'll try to keep that in mind