diff --git a/src/main/java/electroblob/wizardry/Settings.java b/src/main/java/electroblob/wizardry/Settings.java index 52546851..21fe7a1d 100644 --- a/src/main/java/electroblob/wizardry/Settings.java +++ b/src/main/java/electroblob/wizardry/Settings.java @@ -17,6 +17,7 @@ import net.minecraftforge.fml.common.event.FMLPreInitializationEvent; import net.minecraftforge.fml.common.network.simpleimpl.IMessage; import net.minecraftforge.fml.relauncher.Side; +import java.io.File; import java.util.*; import java.util.regex.Pattern; @@ -364,7 +365,7 @@ public final class Settings { */ void initConfig(FMLPreInitializationEvent event){ - config = new Configuration(event.getSuggestedConfigurationFile()); + config = new Configuration(new File(Wizardry.configDirectory, Wizardry.MODID + ".cfg")); config.load(); Wizardry.logger.info("Setting up main config"); diff --git a/src/main/java/electroblob/wizardry/Wizardry.java b/src/main/java/electroblob/wizardry/Wizardry.java index f73b173d..7d32b707 100644 --- a/src/main/java/electroblob/wizardry/Wizardry.java +++ b/src/main/java/electroblob/wizardry/Wizardry.java @@ -31,6 +31,8 @@ import net.minecraftforge.fml.common.network.NetworkRegistry; import net.minecraftforge.fml.common.registry.GameRegistry; import org.apache.logging.log4j.Logger; +import java.io.File; + /** * "Electroblob's Wizardry adds an RPG-like system of spells to Minecraft, with the aim of being as playable as * possible. No crazy constructs, no perk trees, no complex recipes - simply find spell books, cast spells, and master @@ -82,6 +84,10 @@ public class Wizardry { * - INFO: Anything that might happen during normal mod operation that the user needs to know about. */ public static Logger logger; + /** A {@link File} object representing wizardry's config folder, {@code config/ebwizardry}). As of wizardry 4.2.4, + * this folder contains the main config file and the global spell properties folder, if used. */ + public static File configDirectory; + // The instance of wizardry that Forge uses. @Instance(Wizardry.MODID) public static Wizardry instance; @@ -97,6 +103,7 @@ public class Wizardry { proxy.registerResourceReloadListeners(); + configDirectory = new File(event.getModConfigurationDirectory(), Wizardry.MODID); settings.initConfig(event); // Capabilities diff --git a/src/main/java/electroblob/wizardry/spell/Spell.java b/src/main/java/electroblob/wizardry/spell/Spell.java index 228c5d44..91406f63 100644 --- a/src/main/java/electroblob/wizardry/spell/Spell.java +++ b/src/main/java/electroblob/wizardry/spell/Spell.java @@ -29,6 +29,10 @@ import net.minecraft.util.text.ITextComponent; import net.minecraft.util.text.TextComponentTranslation; import net.minecraft.world.World; import net.minecraftforge.event.RegistryEvent; +import net.minecraftforge.event.world.WorldEvent; +import net.minecraftforge.fml.common.Mod; +import net.minecraftforge.fml.common.eventhandler.SubscribeEvent; +import net.minecraftforge.fml.common.network.FMLNetworkEvent; import net.minecraftforge.fml.relauncher.Side; import net.minecraftforge.fml.relauncher.SideOnly; import net.minecraftforge.registries.ForgeRegistry; @@ -84,6 +88,7 @@ import java.util.stream.Collectors; * @see ItemScroll ItemScroll * @see Spells */ +@Mod.EventBusSubscriber public abstract class Spell extends IForgeRegistryEntry.Impl implements Comparable { // Spell checklist: @@ -117,6 +122,8 @@ public abstract class Spell extends IForgeRegistryEntry.Impl implements C private final String unlocalisedName; /** This spell's associated SpellProperties object. */ private SpellProperties properties; + /** A reference to the global spell properties for this spell, so they are only loaded once. */ + private SpellProperties globalProperties; /** Used in initialisation. */ private Set propertyKeys = new HashSet<>(); @@ -250,6 +257,7 @@ public abstract class Spell extends IForgeRegistryEntry.Impl implements C if(!arePropertiesInitialised()){ this.properties = properties; + if(this.globalProperties == null) this.globalProperties = properties; }else{ Wizardry.logger.info("A mod attempted to set a spell's properties, but they were already initialised."); } @@ -266,9 +274,14 @@ public abstract class Spell extends IForgeRegistryEntry.Impl implements C .map(s -> s.properties).toArray(SpellProperties[]::new))); }else{ // On the client side, wipe the spell properties so the new ones can be set - for(Spell spell : registry){ - spell.properties = null; // TESTME: Can we guarantee this happens before the packet arrives? - } + // TESTME: Can we guarantee this happens before the packet arrives? + clearProperties(); + } + } + + private static void clearProperties(){ + for(Spell spell : registry){ + spell.properties = null; } } @@ -871,4 +884,32 @@ public abstract class Spell extends IForgeRegistryEntry.Impl implements C && (this.element == null || spell.getElement() == this.element); } } + + // ============================================ Event handlers ============================================== + + // Not ideal but it solves the reloading of spell properties without breaking encapsulation + + @SubscribeEvent + public static void onWorldLoadEvent(WorldEvent.Load event){ + if(!event.getWorld().isRemote){ + if(event.getWorld().provider.getDimension() != 0) return; // Only do it once per save file + clearProperties(); + SpellProperties.loadWorldSpecificSpellProperties(event.getWorld()); + for(Spell spell : Spell.registry){ + if(!spell.arePropertiesInitialised()) spell.setProperties(spell.globalProperties); + } + } + } + + @SubscribeEvent + public static void onClientDisconnectEvent(FMLNetworkEvent.ClientDisconnectionFromServerEvent event){ + // Why does the world UNLOAD event happen during world LOADING? How does that even work?! + clearProperties(); + for(Spell spell : Spell.registry){ + // If someone wants to access them from the menu, they'll get the global ones (not sure why you'd want to) + // No need to sync here since the server is about to shut down anyway + spell.setProperties(spell.globalProperties); + } + } + } diff --git a/src/main/java/electroblob/wizardry/util/SpellProperties.java b/src/main/java/electroblob/wizardry/util/SpellProperties.java index 55e7d00b..f868abc2 100644 --- a/src/main/java/electroblob/wizardry/util/SpellProperties.java +++ b/src/main/java/electroblob/wizardry/util/SpellProperties.java @@ -13,13 +13,16 @@ import electroblob.wizardry.spell.Spell; import io.netty.buffer.ByteBuf; import net.minecraft.util.JsonUtils; import net.minecraft.util.ResourceLocation; +import net.minecraft.world.World; import net.minecraftforge.common.crafting.CraftingHelper; import net.minecraftforge.fml.common.Loader; import net.minecraftforge.fml.common.ModContainer; +import org.apache.commons.io.FileUtils; import org.apache.commons.io.FilenameUtils; import org.apache.commons.io.IOUtils; import java.io.BufferedReader; +import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.util.*; @@ -40,9 +43,6 @@ import java.util.stream.Collectors; * @author Electroblob * @since Wizardry 4.2 */ -// There is not a particular semantic reason to separate this from the Spell class itself. However, doing so means that -// everything related to the JSON spell system is kept in one place and doesn't clutter the (already long) Spell class. -// Additionally, it allows SpellProperties objects to be passed around during loading and syncing. public final class SpellProperties { private static final Gson gson = new Gson(); @@ -245,32 +245,57 @@ public final class SpellProperties { // things don't work as expected then that may be why - pretty sure it's fine though since the properties get // wiped client-side on each login anyway. public static void init(){ + // Collecting to a set should give us one of each mod ID Set modIDs = Spell.getSpells(Spell.allSpells).stream().map(s -> s.getRegistryName().getNamespace()).collect(Collectors.toSet()); - boolean flag = true; + boolean flag = loadConfigSpellProperties(); for(String modID : modIDs){ - flag &= loadSpellProperties(modID); // Don't short-circuit, or mods later on won't get loaded! + flag &= loadBuiltInSpellProperties(modID); // Don't short-circuit, or mods later on won't get loaded! } if(!flag) Wizardry.logger.warn("Some spell property files did not load correctly; this will likely cause problems later!"); } - // Sooooooo I just realised that resource packs - you know, that famously client-side thing - can now define - // stuff that should be specified by the server. No wonder it all got moved to data packs in 1.13... - // Anyway, for the time being we're in 1.12 so we're gonna have to do this instead. + // There are now three 'layers' of spell properties - in order of priority, these are: + // 1. World-specific properties, stored in saves/[world]/data/spells + // 2. Global overrides, stored in config/ebwizardry/spells + // 3. Built-in properties, stored in mods/[mod jar]/assets/spells + // There's a method for loading each of these below, because that makes sense to me! + + public static void loadWorldSpecificSpellProperties(World world){ + + Wizardry.logger.info("Loading custom spell properties for world {}", world.getWorldInfo().getWorldName()); + + File spellJSONDir = new File(new File(world.getSaveHandler().getWorldDirectory(), "data"), "spells"); + + if(spellJSONDir.mkdirs()) return; // If it just got created it can't possibly have anything inside + + if(!loadSpellPropertiesFromDir(spellJSONDir)) Wizardry.logger.warn("Some spell property files did not load correctly; this will likely cause problems later!"); + } + + private static boolean loadConfigSpellProperties(){ + + Wizardry.logger.info("Loading spell properties from config folder"); + + File spellJSONDir = new File(Wizardry.configDirectory, "spells"); + + if(!spellJSONDir.exists()) return true; // If there's no global spell properties folder, do nothing + + return loadSpellPropertiesFromDir(spellJSONDir); + } // For crafting recipes, Forge does some stuff behind the scenes to load recipe JSON files from mods' namespaces. // This leverages the same methods. - private static boolean loadSpellProperties(String modID){ + private static boolean loadBuiltInSpellProperties(String modID){ // Yes, I know you're not supposed to do orElse(null). But... meh. ModContainer mod = Loader.instance().getModList().stream().filter(m -> m.getModId().equals(modID)).findFirst().orElse(null); if(mod == null){ - Wizardry.logger.warn("Tried to load spell properties for mod with ID '" + modID + "', but no such mod was loaded"); + Wizardry.logger.warn("Tried to load built-in spell properties for mod with ID '" + modID + "', but no such mod was loaded"); return false; // Failed! } @@ -279,7 +304,7 @@ public final class SpellProperties { List spells = Spell.getSpells(s -> s.getRegistryName().getNamespace().equals(modID)); if(modID.equals(Wizardry.MODID)) spells.add(Spells.none); // In this particular case we do need the none spell - Wizardry.logger.info("Loading spell properties for " + spells.size() + " spells in mod " + modID); + Wizardry.logger.info("Loading built-in spell properties for " + spells.size() + " spells in mod " + modID); // This method is used by Forge to load mod recipes and advancements, so it's a fair bet it's the right one // In the absence of Javadoc, here's what the non-obvious parameters do: @@ -307,12 +332,16 @@ public final class SpellProperties { return true; } - BufferedReader reader = null; - // We want to do this regardless of whether the JSON file got read properly, because that prints its // own separate warning if(!spells.remove(spell)) Wizardry.logger.warn("What's going on?!"); + // Ignore spells overridden in the config folder + // This needs to be done AFTER the above line or it'll think there are missing spell properties files + if(spell.arePropertiesInitialised()) return true; + + BufferedReader reader = null; + try{ reader = Files.newBufferedReader(file); @@ -348,6 +377,54 @@ public final class SpellProperties { return success; } + + private static boolean loadSpellPropertiesFromDir(File dir){ + + boolean success = true; + + for(File file : FileUtils.listFiles(dir, new String[]{"json"}, true)){ + + // The structure in world and config folders is subtly different in that the "spells" and mod id directories + // are in the opposite order, i.e. it's spells/modid/whatever.json instead of modid/spells/whatever.json + String relative = dir.toPath().relativize(file.toPath()).toString(); // modid\whatever.json + String nameAndModID = FilenameUtils.removeExtension(relative).replaceAll("\\\\", "/"); // modid/whatever + String modID = nameAndModID.split("/")[0]; // modid + String name = nameAndModID.substring(nameAndModID.indexOf('/') + 1); // whatever + + ResourceLocation key = new ResourceLocation(modID, name); + + Spell spell = Spell.registry.getValue(key); + + // If no spell matches a particular file, log it and just ignore the file + if(spell == null){ + Wizardry.logger.info("Spell properties file " + nameAndModID + ".json does not match any registered spells; ensure the filename is spelled correctly."); + continue; + } + + BufferedReader reader = null; + + try{ + + reader = Files.newBufferedReader(file.toPath()); + + JsonObject json = JsonUtils.fromJson(gson, reader, JsonObject.class); + SpellProperties properties = new SpellProperties(json, spell); + spell.setProperties(properties); + + }catch(JsonParseException jsonparseexception){ + Wizardry.logger.error("Parsing error loading spell property file for " + key, jsonparseexception); + success = false; + }catch(IOException ioexception){ + Wizardry.logger.error("Couldn't read spell property file for " + key, ioexception); + success = false; + }finally{ + IOUtils.closeQuietly(reader); + } + } + + return success; + } + } // We probably could have used the attribute system for all of this, but I am reluctant to do so for a number of