Some cleanup

bukkit/src/main/java/us/myles/ViaVersion/ViaVersionPlugin.java

@@ -84,7 +84,7 @@ public class ViaVersionPlugin extends JavaPlugin implements ViaPlatform {
         }
 
         if (getServer().getPluginManager().getPlugin("ViaBackwards") != null) {
-            MappingDataLoader.setCacheJsonMappings(true);
+            MappingDataLoader.enableMappingsCache();
         }
 
         // Generate classes needed (only works if it's compat or ps)

bungee/src/main/java/us/myles/ViaVersion/BungeePlugin.java

@@ -66,7 +66,7 @@ public class BungeePlugin extends Plugin implements ViaPlatform, Listener {
     @Override
     public void onEnable() {
         if (ProxyServer.getInstance().getPluginManager().getPlugin("ViaBackwards") != null) {
-            MappingDataLoader.setCacheJsonMappings(true);
+            MappingDataLoader.enableMappingsCache();
         }
 
         // Inject

common/src/main/java/us/myles/ViaVersion/api/data/MappingDataLoader.java

@@ -1,16 +1,24 @@
 package us.myles.ViaVersion.api.data;
 
-import com.google.gson.*;
+import com.google.gson.JsonArray;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonIOException;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonSyntaxException;
 import us.myles.ViaVersion.api.Via;
 import us.myles.ViaVersion.util.GsonUtil;
 
-import java.io.*;
+import java.io.File;
-import java.util.HashMap;
+import java.io.FileReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 public class MappingDataLoader {
 
-    private static final Map<String, JsonObject> MAPPINGS_CACHE = new HashMap<>();
+    private static final Map<String, JsonObject> MAPPINGS_CACHE = new ConcurrentHashMap<>();
     private static boolean cacheJsonMappings;
 
     /**
@@ -19,22 +27,26 @@ public class MappingDataLoader {
      *
      * @return true if mappings should be cached
      */
-    public static boolean cacheJsonMappings() {
+    public static boolean isCacheJsonMappings() {
         return cacheJsonMappings;
     }
 
-    public static void setCacheJsonMappings(boolean cacheJsonMappings) {
+    public static void enableMappingsCache() {
-        MappingDataLoader.cacheJsonMappings = cacheJsonMappings;
+        cacheJsonMappings = true;
-        Via.getPlatform().getLogger().info("Enabled caching of mappingdata for ViaBackwards!");
     }
 
     /**
-     * @see #cacheJsonMappings()
+     * Returns the cached mappings. Cleared after Via has been fully loaded.
+     *
+     * @see #isCacheJsonMappings()
      */
     public static Map<String, JsonObject> getMappingsCache() {
         return MAPPINGS_CACHE;
     }
 
+    /**
+     * Loads the file from the plugin folder if present, else from the bundled resources.
+     */
     public static JsonObject loadFromDataDir(String name) {
         File file = new File(Via.getPlatform().getDataFolder(), name);
         if (!file.exists()) return loadData(name);
@@ -52,11 +64,26 @@ public class MappingDataLoader {
         return null;
     }
 
+    /**
+     * Loads the file from the bundled resources. Uses the cache if enabled.
+     */
     public static JsonObject loadData(String name) {
         return loadData(name, false);
     }
 
+    /**
+     * Loads the file from the bundled resources. Uses the cache if enabled.
+     *
+     * @param cacheIfEnabled whether loaded files should be cached
+     */
     public static JsonObject loadData(String name, boolean cacheIfEnabled) {
+        if (cacheJsonMappings) {
+            JsonObject cached = MAPPINGS_CACHE.get(name);
+            if (cached != null) {
+                return cached;
+            }
+        }
+
         InputStream stream = getResource(name);
         InputStreamReader reader = new InputStreamReader(stream);
         try {

common/src/main/java/us/myles/ViaVersion/api/protocol/ProtocolRegistry.java

@@ -31,8 +31,21 @@ import us.myles.ViaVersion.protocols.protocol1_9_3to1_9_1_2.Protocol1_9_3To1_9_1
 import us.myles.ViaVersion.protocols.protocol1_9to1_8.Protocol1_9To1_8;
 import us.myles.ViaVersion.protocols.protocol1_9to1_9_1.Protocol1_9To1_9_1;
 
-import java.util.*;
+import java.util.ArrayList;
-import java.util.concurrent.*;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.SynchronousQueue;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
 
 public class ProtocolRegistry {
     public static final Protocol BASE_PROTOCOL = new BaseProtocol();
@@ -44,8 +57,10 @@ public class ProtocolRegistry {
     private static final Set<Integer> supportedVersions = new HashSet<>();
     private static final List<Pair<Range<Integer>, Protocol>> baseProtocols = Lists.newCopyOnWriteArrayList();
 
-    private static Map<Class<? extends Protocol>, CompletableFuture<Void>> mappingLoaderFutures = new ConcurrentHashMap<>();
+    private static final Object MAPPING_LOADER_LOCK = new Object();
+    private static Map<Class<? extends Protocol>, CompletableFuture<Void>> mappingLoaderFutures = new HashMap<>();
     private static ThreadPoolExecutor mappingLoaderExecutor;
+    private static boolean mappingsLoaded;
 
     static {
         mappingLoaderExecutor = new ThreadPoolExecutor(5, 16, 45L, TimeUnit.SECONDS, new SynchronousQueue<>());
@@ -165,10 +180,11 @@ public class ProtocolRegistry {
             List<Pair<Integer, Protocol>> paths = getProtocolPath(versions.getId(), ProtocolRegistry.SERVER_PROTOCOL);
             if (paths == null) continue;
             supportedVersions.add(versions.getId());
-            for (Pair<Integer, Protocol> path : paths)
+            for (Pair<Integer, Protocol> path : paths) {
                 supportedVersions.add(path.getKey());
             }
         }
+    }
 
     /**
      * Get the versions compatible with the server.
@@ -298,47 +314,45 @@ public class ProtocolRegistry {
      * @param protocolClass protocol class
      */
     public static void completeMappingDataLoading(Class<? extends Protocol> protocolClass) throws Exception {
-        if (mappingLoaderFutures == null) return;
+        if (mappingsLoaded) return;
 
-        CompletableFuture<Void> future = mappingLoaderFutures.remove(protocolClass);
+        CompletableFuture<Void> future = getMappingLoaderFuture(protocolClass);
         if (future == null) return;
 
         future.get();
 
+        synchronized (MAPPING_LOADER_LOCK) {
+            if (mappingsLoaded) return;
+
+            // Remove only after execution to block other potential threads
+            mappingLoaderFutures.remove(protocolClass);
             if (mappingLoaderFutures.isEmpty()) {
                 shutdownLoaderExecutor();
             }
         }
-
-    /**
-     * Ensure that all mapping data has already been loaded, completes it otherwise.
-     */
-    public static void completeMappingDataLoading() throws Exception {
-        if (mappingLoaderFutures == null) return;
-
-        for (CompletableFuture<Void> future : mappingLoaderFutures.values()) {
-            future.get();
-        }
-
-        mappingLoaderFutures.clear();
-        shutdownLoaderExecutor();
     }
 
     private static void shutdownLoaderExecutor() {
+        mappingsLoaded = true;
         mappingLoaderExecutor.shutdown();
         mappingLoaderExecutor = null;
         mappingLoaderFutures = null;
-        if (MappingDataLoader.cacheJsonMappings()) {
+        if (MappingDataLoader.isCacheJsonMappings()) {
             MappingDataLoader.getMappingsCache().clear();
         }
     }
 
     public static void addMappingLoaderFuture(Class<? extends Protocol> protocolClass, Runnable runnable) {
+        synchronized (MAPPING_LOADER_LOCK) {
             CompletableFuture<Void> future = CompletableFuture.runAsync(runnable, mappingLoaderExecutor);
             mappingLoaderFutures.put(protocolClass, future);
         }
+    }
 
     public static CompletableFuture<Void> getMappingLoaderFuture(Class<? extends Protocol> protocolClass) {
+        synchronized (MAPPING_LOADER_LOCK) {
+            if (mappingsLoaded) return null;
             return mappingLoaderFutures.get(protocolClass);
         }
     }
+}

sponge/src/main/java/us/myles/ViaVersion/SpongePlugin.java

@@ -83,7 +83,7 @@ public class SpongePlugin implements ViaPlatform {
     @Listener
     public void onServerStart(GameAboutToStartServerEvent event) {
         if (game.getPluginManager().getPlugin("ViaBackwards").isPresent()) {
-            MappingDataLoader.setCacheJsonMappings(true);
+            MappingDataLoader.enableMappingsCache();
         }
 
         // Inject!

velocity/src/main/java/us/myles/ViaVersion/VelocityPlugin.java

@@ -76,7 +76,7 @@ public class VelocityPlugin implements ViaPlatform<Player> {
                 .injector(new VelocityViaInjector()).build());
 
         if (proxy.getPluginManager().getPlugin("ViaBackwards").isPresent()) {
-            MappingDataLoader.setCacheJsonMappings(true);
+            MappingDataLoader.enableMappingsCache();
         }
 
         Via.getManager().init();