Merge pull request #15 from PlaceholderAPI/development

[1.5.4] Code cleanup and improved data_example.js

scripts/data_example.js

@@ -1,88 +1,110 @@
 function set(path, data) {
-  Data.set(path, data);
+    Data.set(path, data);
-  return "";
-}
-function add(path, amount) {
-  if (isNaN(amount)) {
     return "";
-  }
-  var amt = Data.exists(path) ? Data.get(path) : 0;
-  if (isNaN(amt)) {
-    amt = 0;
-  }
-  amt = parseInt(amt) + parseInt(amount);
-  Data.set(path, amt.toFixed());
-  return "";
 }
-function subtract(path, amount) {
-  if (isNaN(amount)) {
-    return "";
-  }
-  var amt = Data.exists(path) ? parseInt(Data.get(path)) : 0;
-  if (isNaN(amt)) {
-    amt = 0;
-  }
-  amt = parseInt(amt) - parseInt(amount);
-  Data.set(path, amt.toFixed());
-  return "";
-}
-function get(path)  {
-  return Data.exists(path) ? Data.get(path) : "";
-}
-function getInt(path) {
-  if (Data.exists(path)) {
-    var amt = Data.get(path);
-    if (isNaN(amt)) {
-      return 0;
-    } else {
-      return parseInt(amt).toFixed();
-    }
-  }
-  return 0;
-}
-function getUsage(firstArg) {
-  switch(firstArg) {
-    case "get":
-      return "get,<path>";
-    case "getint":
-      return "getint,<path>";
-    case "add":
-      return "add,<path>,<amount>";
-    case "subtract":
-      return "subtract,<path>,<amount>";
-    case "set":
-      return "set,<path>,<value>";
-    default:
-      return "first argument must be get, getint, set, add, subtract";
-  }
-}
-function runPlaceholder() {
-  if (args.length == 0) {
-    return getUsage("no args");
-  }
-  else if (args.length == 1) {
-    return getUsage(args[0]);
-  }
 
-  if (args.length == 2) {
+function add(path, amount) {
-    if (args[0].equals("get")) {
+    if (isNaN(amount)) {
-      return get(args[1]);
+        return "";
     }
-    else if (args[0].equals("getint")) {
+
-      return getInt(args[1]);
+    var current = Data.exists(path) ? Data.get(path) : 0;
+
+    if (isNaN(current)) {
+        current = 0;
     }
-  }
+
-  else if (args.length == 3) {
+    current = parseInt(current) + parseInt(amount);
-    if (args[0].equals("set")) {
+    Data.set(path, current.toFixed());
-      return set(args[1], args[2]);
+    return "";
-    }
-    else if (args[0].equals("add")) {
-      return add(args[1], args[2]);
-    }
-    else if (args[0].equals("subtract")) {
-      return subtract(args[1], args[2]);
-    }
-  }
-  return getUsage(args[0]);
 }
+
+function subtract(path, amount) {
+    if (isNaN(amount)) {
+        return "";
+    }
+
+    var amt = Data.exists(path) ? parseInt(Data.get(path)) : 0;
+
+    if (isNaN(amt)) {
+        amt = 0;
+    }
+
+    amt = parseInt(amt) - parseInt(amount);
+    Data.set(path, amt.toFixed());
+    return "";
+}
+
+function get(path)  {
+    return Data.exists(path)  ? Data.get(path) : "";
+}
+
+function getInt(path) {
+    if (Data.get(path) != null) {
+        var value = Data.get(path);
+        return isNaN(value) ? 0 : parseInt(value).toFixed();
+    }
+
+    return 0;
+}
+
+function getUsage(action) {
+    switch(action) {
+        case "get":
+            return "get,<path>";
+        case "getint":
+            return "getint,<path>";
+        case "add":
+            return "add,<path>,<amount>";
+        case "subtract":
+            return "subtract,<path>,<amount>";
+        case "set":
+            return "set,<path>,<value>";
+        default:
+            return "First argument must be get, getint, set, add, subtract";
+    }
+}
+
+function runPlaceholder() {
+    if (args.length === 0) {
+        return getUsage("No arguments");
+    }
+
+    var action = args[0].toLowerCase();
+
+    if (args.length === 1) {
+        return getUsage(action);
+    }
+
+    var path = args[1];
+
+    if (args.length === 2) {
+        if (action.equals("get")) {
+            return get(path);
+        }
+
+        if (action.equals("getint")) {
+            return getInt(path);
+        }
+    }
+
+    var value = args[2];
+
+    if (args.length === 3) {
+        if (action.equals("set")) {
+            return set(path, value);
+        }
+
+        if (action.equals("add")) {
+            return add(path, value);
+        }
+
+        if (action.equals("subtract")) {
+            return subtract(path, value);
+        }
+    }
+
+    return getUsage(action);
+}
+
 runPlaceholder();

src/main/java/com/extendedclip/papi/expansion/javascript/JavascriptExpansion.java

@@ -33,6 +33,8 @@ import javax.script.ScriptEngineFactory;
 import javax.script.ScriptEngineManager;
 import java.lang.reflect.Field;
 import java.util.*;
+import java.util.logging.Level;
+import java.util.stream.Collectors;
 
 public class JavascriptExpansion extends PlaceholderExpansion implements Cacheable, Configurable {
 
@@ -50,11 +52,11 @@ public class JavascriptExpansion extends PlaceholderExpansion implements Cacheab
     public JavascriptExpansion() {
         instance = this;
         try {
-            final Field f = Bukkit.getServer().getClass().getDeclaredField("commandMap");
+            final Field field = Bukkit.getServer().getClass().getDeclaredField("commandMap");
-            f.setAccessible(true);
+            field.setAccessible(true);
-            commandMap = (CommandMap) f.get(Bukkit.getServer());
+            commandMap = (CommandMap) field.get(Bukkit.getServer());
-        } catch (Exception e) {
+        } catch (NoSuchFieldException | IllegalAccessException e) {
-            e.printStackTrace();
+            getPlaceholderAPI().getLogger().log(Level.SEVERE, "[JavaScript Expansion] An error occurred while accessing CommandMap.", e);
         }
     }
 
@@ -68,11 +70,6 @@ public class JavascriptExpansion extends PlaceholderExpansion implements Cacheab
         return "javascript";
     }
 
-    @Override
-    public String getPlugin() {
-        return null;
-    }
-
     @Override
     public String getVersion() {
         return VERSION;
@@ -84,7 +81,7 @@ public class JavascriptExpansion extends PlaceholderExpansion implements Cacheab
             try {
                 globalEngine = new ScriptEngineManager().getEngineByName(getString("engine", "nashorn"));
             } catch (NullPointerException ex) {
-                getPlaceholderAPI().getLogger().warning("Javascript engine type was invalid! Defaulting to 'nashorn'");
+                getPlaceholderAPI().getLogger().warning("[JavaScript Expansion] Javascript engine type was invalid! Defaulting to 'nashorn'");
                 globalEngine = new ScriptEngineManager().getEngineByName("nashorn");
             }
         }
@@ -94,10 +91,10 @@ public class JavascriptExpansion extends PlaceholderExpansion implements Cacheab
         config.loadPlaceholders();
 
         if (debug) {
-            System.out.println("Java version: " + System.getProperty("java.version"));
+            getPlaceholderAPI().getLogger().info("[JavaScript Expansion] Java version: " + System.getProperty("java.version"));
             final ScriptEngineManager manager = new ScriptEngineManager();
             final List<ScriptEngineFactory> factories = manager.getEngineFactories();
-            System.out.println("Displaying all script engine factories.");
+            getPlaceholderAPI().getLogger().info("Displaying all script engine factories.");
 
             for (ScriptEngineFactory factory : factories) {
                 System.out.println(factory.getEngineName());
@@ -122,9 +119,10 @@ public class JavascriptExpansion extends PlaceholderExpansion implements Cacheab
     @Override
     public void clear() {
         unregisterCommand();
-        scripts.forEach(s -> {
+
-            s.saveData();
+        scripts.forEach(script -> {
-            s.cleanup();
+            script.saveData();
+            script.cleanup();
         });
 
         if (githubManager != null) {
@@ -138,43 +136,41 @@ public class JavascriptExpansion extends PlaceholderExpansion implements Cacheab
     }
 
     @Override
-    public String onRequest(OfflinePlayer p, String identifier) {
+    public String onRequest(OfflinePlayer player, String identifier) {
-        if (p == null) {
+        if (player == null || scripts.size() == 0) {
             return "";
         }
 
-        if (scripts.isEmpty()) {
-            return null;
-        }
-
         for (JavascriptPlaceholder script : scripts) {
             if (identifier.startsWith(script.getIdentifier() + "_")) {
                 identifier = identifier.replace(script.getIdentifier() + "_", "");
-                return !identifier.contains(",") ? script.evaluate(p, identifier)
+
-                        : script.evaluate(p, identifier.split(","));
+                return !identifier.contains(",") ? script.evaluate(player, identifier) : script.evaluate(player, identifier.split(","));
-            } else if (identifier.equalsIgnoreCase(script.getIdentifier())) {
+            }
-                return script.evaluate(p);
+
+            if (identifier.equalsIgnoreCase(script.getIdentifier())) {
+                return script.evaluate(player);
             }
         }
+
         return null;
     }
 
-    public boolean addJSPlaceholder(JavascriptPlaceholder p) {
+    public boolean addJSPlaceholder(JavascriptPlaceholder placeholder) {
-        if (p == null) {
+        if (placeholder == null) {
             return false;
         }
 
         if (scripts.isEmpty()) {
-            scripts.add(p);
+            scripts.add(placeholder);
             return true;
         }
 
-        if (scripts.stream().filter(s -> s.getIdentifier().equalsIgnoreCase(p.getIdentifier()))
+        if (getJSPlaceholder(placeholder.getIdentifier()) != null) {
-                .findFirst().orElse(null) != null) {
             return false;
         }
 
-        scripts.add(p);
+        scripts.add(placeholder);
         return true;
     }
 
@@ -183,9 +179,9 @@ public class JavascriptExpansion extends PlaceholderExpansion implements Cacheab
     }
 
     public List<String> getLoadedIdentifiers() {
-        List<String> l = new ArrayList<>();
+        return scripts.stream()
-        scripts.forEach(s -> l.add(s.getIdentifier()));
+                .map(JavascriptPlaceholder::getIdentifier)
-        return l;
+                .collect(Collectors.toList());
     }
 
     public JavascriptPlaceholder getJSPlaceholder(String identifier) {
@@ -209,18 +205,20 @@ public class JavascriptExpansion extends PlaceholderExpansion implements Cacheab
 
     @Override
     public Map<String, Object> getDefaults() {
-        Map<String, Object> def = new HashMap<>();
+        final Map<String, Object> defaults = new HashMap<>();
-        def.put("engine", "javascript");
+        defaults.put("engine", "javascript");
-        def.put("debug", false);
+        defaults.put("debug", false);
-        def.put("github_script_downloads", false);
+        defaults.put("github_script_downloads", false);
-        return def;
+
+        return defaults;
     }
 
     protected int reloadScripts() {
-        scripts.forEach(s -> {
+        scripts.forEach(script -> {
-            s.saveData();
+            script.saveData();
-            s.cleanup();
+            script.cleanup();
         });
+
         scripts.clear();
         config.reload();
         return config.loadPlaceholders();
@@ -240,10 +238,7 @@ public class JavascriptExpansion extends PlaceholderExpansion implements Cacheab
 
     @SuppressWarnings("UnusedReturnValue")
     private boolean unregisterCommand() {
-        if (commandMap == null || commands == null) {
+        return commandMap != null && commands != null && commands.unregister(commandMap);
-            return false;
-        }
-        return commands.unregister(commandMap);
     }
 
     @SuppressWarnings("UnusedReturnValue")

src/main/java/com/extendedclip/papi/expansion/javascript/ScriptData.java

@@ -44,7 +44,7 @@ public class ScriptData {
     }
 
     public boolean exists(String key) {
-        return map.containsKey(key) && map.get(key) != null;
+        return map.get(key) != null;
     }
 
     public Object get(String key) {