Great idea for a script, and very nicely commented! i havent seen anyone create this before. Some suggestions. I would make PlayerList an array. _.each( is a function from the underscore library, a suite of add on functions to make Javascript easier to use. That particular function isnt needed anymore as javascript has a native forEach function, which would change your code like this: msg.selected.forEach(function(obj) { I think its a bit easier to understand, or at least guess, what that's doing. Essentially it takes msg.selected, and loops through every selected thing. In each loop, it puts the next selected thing in something called obj, and you can access that things properties by using obj, as the script does later on. Once all things in the selected group have been dealt with, the forEach loop is finished, and the code moves past it. _.each and forEach do the same work, just the syntax is slightly different. If you setup playerList as an array, you'd have to initialise it as so: var PlayerList = []; Then you'd need to edit this section: if (character) { PlayerList = PlayerList + character.get('controlledby')+ ','; } one way would be: if (character) {
// get the comma separated list of players let controlled = character.get('controlledby').split(',');
// loop through the players.
controlled.forEach(function(char) {
// check if the player is already in PlayerList if(!PlayerList.includes(char)) {
//if not, add to the playerlist PlayerList.push(char); } }); } This routine ensures your PlayerList only has unique entries from the outset, and is already an array, which simplifies the follow code. Note: if controlledby contains "All", this would need work to get the player list. One approach would be to simply check if the playerlist includes "All" , and then change the ending to not bother whispering if true, just sending chat to everyone. You'll see I've done that in the code below. Here's a quick update of your code with the above tweaks. i made some other minor tweaks I'll explain below. on('chat:message', function(msg) { if (msg.type !== 'api') return; if (msg.content.indexOf("!gwisp ") === -1) return; let PlayerList = []; msg.selected.forEach(function(obj) { let token, character; token = getObj('graphic', obj._id); if (token) { character = getObj('character', token.get('represents')); } if (character) { // get the comma separated list of players const controlled = character.get('controlledby').split(','); // loop through the players. controlled.forEach(function(char) { // check if the player is already in PlayerList if(!PlayerList.includes(char)) { //if not, add to the playerlist PlayerList.push(char); } }); } });
const TheMessage = msg.content . split ( ' ' ). slice ( 1 ). join ( ' ' ) ;
if(PlayerList.includes('all')) { sendChat('The Moderator', TheMessage); } else { PlayerList.forEach(function(player) { var PlayerVar = getObj("player", player); var PlayerName = PlayerVar.get("_displayname"); sendChat('The Moderator', '/w "' + PlayerName + '" ' + TheMessage); }); } }); I switched most declarations from var to either let or const. This is a more modern syntax and there are some advantages to doing so, plus also some pitfalls. const is for variables that will not change, and let is for variables that might change. These variables respect scope, which means they only exist within the block of code in which they are declared. For example, the Playervar variable only exists within that final PlayerList.foreach section. Variables declared with var exist from the beginning of the code, even if declared late in the function, and this is messy from an efficiency point of view, and can be a source of subtle errors. So its better to use the more modern let and const. But you have to be careful to declare them where needed. A common mistake would be to do something like this: let token = getObj('graphic', obj._id); if (token) { let character = getObj('character', token.get('represents')); }
if (character) {
This would appear to work, but character would be erased as soon as the } was passed, so that if( character ) statement would always be false. Luckily the way youve declared them avoids that problem. I also changed that for loop to a forEach loop. These are usually much easier to use once youre familiar with them - no need to mess around with indexes to get the item in the array, a foreach loop gives you the item directly. for getting rid of the first word in the msg.content, I used a different method: split on spaces to turn it into an array, remove the first item with slice, and rejoin into an array. Finally, there's a way around the way player names break on spaces: if you put quotes around the name, then whispers work properly. I've done that here: sendChat ( 'The Moderator' , '/w "' + PlayerName + '" ' + TheMessage ); Javascript allows you to use different types of quotes, and the inner quotes will be treated as part of the string. So here PlayerName gets quotes put around it, and avoids the "breaking on spaces" problem. Hope this helps, and feel free to use or discard any of these changes :)
