RobinKuiper
[BeyondImporter] Attack Spells show as Spellcards
Attack Spells (Eldritch Blast, Fireball, etc) import showing "Output: Attack", but show spell card output until the spell is edited and toggled to "Output: Spellcard" and back to "Attack".
This problem is documented here: https://app.roll20.net/forum/post/6248700/script-beta-beyondimporter-import-dndbeyond-character-sheets/?pageforid=7268275#post-7268275
This functionality is broken by changes the Roll20 5e OGL Sheet team is making. https://github.com/Roll20/roll20-character-sheets/commit/089e7255ec2df26bf8e1ea4ca94baf6bee4a8cfa#diff-2af1f0641d1837ed6ada964688728104R6505
I have discussed it with them and they are aware. There is nothing we can do about this immediately.
Note to devs: The best case is that Roll20 core changes the eventinfo in sheet worker change notifications to correctly pass additional info in 'API' cases as well, then everything will just start working again correctly in spite of OGL's changes. Otherwise, we need to wait until OGL is done making their changes and then change how we do things. Unfortunately, Roll20 is pushing changes to OGL sheet to the live environment without any API testing.
Background for devs:
BeyondImporter stopped working when OGL sheet was updated to 2.6 or 2.7. I investigated the source of the problem, I found the changes that Cassie made to the OGL sheet. They are trying to reduce the number of database reads in order to speed up interactive use. One of the ways he is doing that is by relying on 'eventinfo' to carry the new value of the attribute, instead of reading it in the event handler.
Of course that is a race condition, where you could actually react to a stale value. But that's their problem :). I get what they are trying to do. The problem is that 'eventinfo' does not contain this new value when the change comes from 'api' sourceType. This is just a bug, where the Roll20 core added additional fields to the event for sheet workers but not API?
As a result, the sheet no longer reacts to changes from the API, as the new value field is undefined. Cassie says they are pressing ahead with changing all their code to rely on this field being populated.
He rejected my request to at least read the field from the database if sourceType is set to 'api' so our scripts don't break.
The only reasonable fix would be to add any extra fields in 'eventinfo' in all code paths in the roll20 core, so they will be present for events from the API also. Obviously, that should have already been the case :)
----- copy from a message to cassie ----
When a change event is delivered from the 'api' source, it does NOT populate eventinfo.newValue at all. It is just undefined. So the previous 2.5 code worked because it actually retrieved the new value of the attribute explicitly via getAttrs(...). In this case, it is the value of 'spelloutput' below:
2.5:
on("change:repeating_spell-cantrip:spelloutput change:repeating_spell-1:spelloutput change:repeating_spell-2:spelloutput change:repeating_spell-3:spelloutput change:repeating_spell-4:spelloutput change:repeating_spell-5:spelloutput change:repeating_spell-6:spelloutput change:repeating_spell-7:spelloutput change:repeating_spell-8:spelloutput change:repeating_spell-9:spelloutput", function(eventinfo) {
if(eventinfo.sourceType && eventinfo.sourceType === "sheetworker") {
return;
}
var repeating_source = eventinfo.sourceAttribute.substring(0, eventinfo.sourceAttribute.lastIndexOf('_'));
getAttrs([repeating_source + "_spellattackid", repeating_source + "_spelllevel", repeating_source + "_spelloutput", repeating_source + "_spellattackid", repeating_source + "_spellathigherlevels","character_id"], function(v) {
var update = {};
var spelloutput = v[repeating_source + "_spelloutput"];
var spellid = repeating_source.slice(-20);
var attackid = v[repeating_source + "_spellattackid"];
var lvl = v[repeating_source + "_spelllevel"];
if(spelloutput && spelloutput === "ATTACK") {
create_attack_from_spell(lvl, spellid, v["character_id"]);
}
else if(spelloutput && spelloutput === "SPELLCARD" && attackid && attackid != "") {
remove_attack(attackid);
var spelllevel = "@{spelllevel}";
if(v[repeating_source + "_spellathigherlevels"]) {
var lvl = parseInt(v[repeating_source + "_spelllevel"],10);
spelllevel = "?{Cast at what level?";
for(i = 0; i < 10-lvl; i++) {
spelllevel = spelllevel + "|Level " + (parseInt(i, 10) + parseInt(lvl, 10)) + "," + (parseInt(i, 10) + parseInt(lvl, 10));
}
spelllevel = spelllevel + "}";
}
update[repeating_source + "_rollcontent"] = "@{wtype}&{template:spell} {{level=@{spellschool} " + spelllevel + "}} {{name=@{spellname}}} {{castingtime=@{spellcastingtime}}} {{range=@{spellrange}}} {{target=@{spelltarget}}} @{spellcomp_v} @{spellcomp_s} @{spellcomp_m} {{material=@{spellcomp_materials}}} {{duration=@{spellduration}}} {{description=@{spelldescription}}} {{athigherlevels=@{spellathigherlevels}}} @{spellritual} {{innate=@{innate}}} @{spellconcentration} @{charname_output}";
}
setAttrs(update, {silent: true});
});
});
2.7:
['spell-cantrip','spell-1','spell-2','spell-3','spell-4','spell-5','spell-6','spell-7','spell-8','spell-9'].forEach(attr => {
on(`change:repeating_${attr}:spelloutput`, (eventinfo) => {
if(eventinfo.sourceType && eventinfo.sourceType === "sheetworker") {return;}
const spellid = eventinfo.sourceAttribute.split("_")[2], repeating_source = `repeating_${attr}_${spellid}`;
getAttrs([`${repeating_source}_spellattackid`, `${repeating_source}_spelllevel`, `${repeating_source}_spellathigherlevels`, "character_id"], function(v) {
const attackid = v[repeating_source + "_spellattackid"];
const higherlevels = v[repeating_source + "_spellathigherlevels"];
const spelloutput = eventinfo.newValue;
let lvl = v[repeating_source + "_spelllevel"];
if (spelloutput && spelloutput === "ATTACK") {
create_attack_from_spell(lvl, spellid, v["character_id"]);
} else if (spelloutput && spelloutput === "SPELLCARD" && attackid && attackid != "") {
let lvl = parseInt(v[repeating_source + "_spelllevel"], 10);
remove_attack(attackid);
update_spelloutput(higherlevels, lvl, repeating_source, spelloutput);
}
});
});
});
There are many places in the code, even in 2.5, where newValue is used and I suspect none of those work if the change is from the API. That would explain some of the weird crap we have observed in the past. A good fix would be to fix roll20 core to deliver newValue also on changes from the API.