This is a great first script! Very clean. I do have a few suggestions for you, since you asked for feedback. • I noticed you have a list of player ids that you're checking against. You can probably replace all that with the playerIsGM() function to determine if the chat message is from a GM. Note that this respects if the GM rejoins as a player and reports them as not a GM in that case. • You might consider adding checking for crits in ranges, say for things like [[1d20cs>18]]. That information is provided in the roll data, though it's a bit complicated to parse. I write a short example for someone a few weeks ago which you might find useful to look at: on('ready',function(){
'use strict';
var getCritComparitor = function(roll){
let comp=[];
if(_.has(roll,'mods') && _.has(roll.mods,'customCrit')){
_.each(roll.mods.customCrit,function(cc){
switch(cc.comp){
case '<=':comp.push((o)=>o<=cc.point);break;
case '==':comp.push((o)=>o==cc.point);break;
case '>=':comp.push((o)=>o>=cc.point);break;
}
});
} else {
comp.push((o)=>o==roll.sides);
}
return function(v){
let crit=false;
_.find(comp,(f)=>crit=crit||f(v));
return crit;
};
};
var isCrit = function(roll){
let comp=getCritComparitor(roll);
_.each(roll.results,(r)=>{
if(comp(r.v)){
sendChat('isCrit',`The ${r.v} is a critical!`);
}
});
};
on('chat:message',function(msg){
if(msg.inlinerolls){
_.each(msg.inlinerolls,(ir)=>_.each(ir.results.rolls,(irrr)=>isCrit(irrr)));
}
});
});
That is only the criticals, not the failures, but you get the idea. • I think the implementation of JSON.parse() in the API returns undefined on failure, rather than throwing an exception. I'll have to check that, but I think your isJson() function will always return true. • You might enjoy rewriting your song lookup using the filterObjs() function, which allows you to supply a function for matching. You could even forgo the return with something akin to: var criticalHit = {},
criticalFail = {};
filterObjs((o)=>{
if( 'jukeboxtrack' === o.get('type')) {
if('Critical Hit' === o.get('title')){
criticalHit = o;
} else if('Critical Fail' === o.get('title')){
criticalFail = o;
}
}
}),
Not much different, but an option (and only a single traversal). • It's always nice to include a version number in the code, just so if you end up helping someone with it and you've made modifications you can know what one they've got. I hope you go on to write more scripts! Happy Rolling!
