You're getting there, Isaac... let's clean up a few things. First, it looks like you're trying to follow a namespace pattern: var inlineTest = inlineTest || {}; ...except that after that, you don't assign your functions to that namespace. handleInput() is in the same global namespace as where you declared inlineTest, instead of attached to your inlineTest object like this: inlineTest.handleInput = function(msg) { ... } or inlineTest.handleInput = msg => { ... } With unattached functions, you're beginning to clutter the global space. Many scripts might want a function named "handleInput", and if they all register to the global namespace, only the last one to load will win (provided they're all declared with var or let; const would throw an error; more on that in a minute). That's why when people use the namespace pattern like this, they typically write it: var inlineTest = inlineTest || (function () { // ... code goes here })(); That creates an IIFE (immediately invoked function expression), which does a number of things... importantly, it carves out a corner of the namespace for your objects so your script doesn't step on (or get stepped on by) other scripts. Within that object, you're going to want individual functions. Right now, you're missing the right hand brace closing your functions, so they are running together. I'm not even sure you have the right number of closures. A good IDE should be able to alert you to that fact, or you can always use the closure compiler to check. So, within your script closure, you're going to want your handleInput(); you're going to want your registerEventHandlers() (which registers handleInput() to answer a chat event); you're going to want your chatLog() function to output/report the status of things, and you're going to want a function that will actually run the registerEventHandlers() function when everything is ready. Lucky for us, there is a 'ready' event that we can execute when the sandbox is loaded. Altogether, that would look more like this: var inlineTest = inlineTest || (function () { 'use strict' const handleInput = function(msg){ if (msg.type !== 'api' || !/^!inlineTest\b/.test(msg.content)) return; function chatLog(){ sendChat("GM", `chat test args array ${args} + msg.content array ${msg.content}`); }; let args = msg.content .split(/\s+--/) .map(a => a.split(/=/)) .map(a => [a[0].toLowerCase(),a[1]]) .filter(a => ["level", "roll1", "roll2"].includes(a[0])); }; msg.content = msg.inlinerolls .reduce((memo, current, index) => { memo['$[['+index+']]']=current.results.total || 0; return memo; },{}) .reduce((memo, current, index) => { return memo.replace(index, current); },msg.content); chatLog(); }; const registerEventHandlers = () => { on('chat:message', handleInput); };
on('ready',() => {
registerEventHandlers();
}); })(); I would point out, too, that I renamed the "previous" argument in your reduce() statements... in the context of "current" and "previous" that might mean something a little different from what it means in a reduce() function. I named it "memo", because that argument refers to the single object that is being passed through every iteration... ie, the object you are building. The reduce() function will take the first item in the array as the memo if you don't supply a starting value for the memo, but that means you're working with the "previous" item only during the first pass. After that, you're working with whatever you have returned as the memo. Then... ...once all of that makes sense, I would point out that this namespace pattern has a few weaknesses, still. For instance, the entire structure of it (built with a "var" declaration that can be redeclared) means that you don't know if your script has been stepped on by something that comes later with the same name. You'd probably much rather know immediately if you had a name collision, like that, so you can figure out a workaround and get both scripts functional again. Most larger scripts people write these days follow something called the Revealing Module Pattern. Aaron shared a good starting template for that. You should be able to map the component parts of your script over the top of what he provided. Also, if you look at that template, you'll see an instance where he used the var namespace pattern (with API_Meta ). This is a good example (in fact, the only example I've encountered on Roll20) where this pattern is necessary. The idea is that many scripts might need to access the API_Meta object, so they might all try to declare it. If another script has already declared it (and stashed information, there), we don't want to lose that when the next script tries to make sure it exists and stash its own information there, too. The API_Meta object provides many benefits, which Aaron discusses in that link. The benefits are also leveraged in the ScriptInfo script (and the ways to implement it in your script are discussed in that thread, too). In Aaron's template, it is already fully implemented, so you don't have to go through the steps, but reading through the pieces can help you understand what they do. As always, post back with more questions!
