Reformat JSON by wrapping CachedOutput

Rather than mixing-in to CachedOutput, we just wrap our DataProviders to
use a custom CachedOutput which reformats the JSON before writing. This
allows us to drop mixins for common+non-client code.

projects/common/src/main/java/dan200/computercraft/data/PrettyDataProvider.java

@@ -0,0 +1,48 @@
+// SPDX-FileCopyrightText: 2024 The CC: Tweaked Developers
+//
+// SPDX-License-Identifier: MPL-2.0
+
+package dan200.computercraft.data;
+
+import com.google.common.hash.HashCode;
+import com.google.common.hash.HashFunction;
+import com.google.common.hash.Hashing;
+import net.minecraft.data.CachedOutput;
+import net.minecraft.data.DataProvider;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.concurrent.CompletableFuture;
+
+/**
+ * Wraps an existing {@link DataProvider}, passing generated JSON through {@link PrettyJsonWriter}.
+ *
+ * @param provider The provider to wrap.
+ * @param <T>      The type of the provider to wrap.
+ */
+public record PrettyDataProvider<T extends DataProvider>(T provider) implements DataProvider {
+    @Override
+    public CompletableFuture<?> run(CachedOutput cachedOutput) {
+        return provider.run(new Output(cachedOutput));
+    }
+
+    @Override
+    public String getName() {
+        return provider.getName();
+    }
+
+    private record Output(CachedOutput output) implements CachedOutput {
+        @SuppressWarnings("unused")
+        private static final HashFunction HASH_FUNCTION = Hashing.sha1();
+
+        @Override
+        public void writeIfNeeded(Path path, byte[] bytes, HashCode hashCode) throws IOException {
+            if (path.getFileName().toString().endsWith(".json")) {
+                bytes = PrettyJsonWriter.reformat(bytes);
+                hashCode = HASH_FUNCTION.hashBytes(bytes);
+            }
+
+            output.writeIfNeeded(path, bytes, hashCode);
+        }
+    }
+}

projects/common/src/main/java/dan200/computercraft/data/PrettyJsonWriter.java

@@ -22,11 +22,10 @@ import java.util.List;
 
 /**
  * Alternative version of {@link JsonWriter} which attempts to lay out the JSON in a more compact format.
- * <p>
- * Yes, this is at least a little deranged.
+ *
+ * @see PrettyDataProvider
  */
 public class PrettyJsonWriter extends JsonWriter {
-    public static final boolean ENABLED = System.getProperty("cct.pretty-json") != null;
     private static final Gson GSON = new GsonBuilder().setPrettyPrinting().disableHtmlEscaping().create();
 
     private static final int MAX_WIDTH = 120;
@@ -44,17 +43,6 @@ public class PrettyJsonWriter extends JsonWriter {
         this.out = out;
     }
 
-    /**
-     * Create a JSON writer. This will either be a pretty or normal version, depending on whether the global flag is
-     * set.
-     *
-     * @param out The writer to emit to.
-     * @return The constructed JSON writer.
-     */
-    public static JsonWriter createWriter(Writer out) {
-        return ENABLED ? new PrettyJsonWriter(out) : new JsonWriter(out);
-    }
-
     /**
      * Reformat a JSON string with our pretty printer.
      *
@@ -62,8 +50,6 @@ public class PrettyJsonWriter extends JsonWriter {
      * @return The reformatted string.
      */
     public static byte[] reformat(byte[] contents) {
-        if (!ENABLED) return contents;
-
         JsonElement object;
         try (var reader = new InputStreamReader(new ByteArrayInputStream(contents), StandardCharsets.UTF_8)) {
             object = GSON.fromJson(reader, JsonElement.class);

projects/common/src/main/java/dan200/computercraft/mixin/CacheUpdaterMixin.java

@@ -1,25 +0,0 @@
-// SPDX-FileCopyrightText: 2022 The CC: Tweaked Developers
-//
-// SPDX-License-Identifier: MPL-2.0
-
-package dan200.computercraft.mixin;
-
-import dan200.computercraft.data.PrettyJsonWriter;
-import org.spongepowered.asm.mixin.Mixin;
-import org.spongepowered.asm.mixin.injection.At;
-import org.spongepowered.asm.mixin.injection.ModifyArg;
-
-@Mixin(targets = "net/minecraft/data/HashCache$CacheUpdater")
-public class CacheUpdaterMixin {
-    @SuppressWarnings("UnusedMethod")
-    @ModifyArg(
-        method = "writeIfNeeded",
-        at = @At(value = "INVOKE", target = "Ljava/nio/file/Files;write(Ljava/nio/file/Path;[B[Ljava/nio/file/OpenOption;)Ljava/nio/file/Path;"),
-        require = 0
-    )
-    private byte[] reformatJson(byte[] contents) {
-        // It would be cleaner to do this inside DataProvider.saveStable, but Forge's version of Mixin doesn't allow us
-        // to inject into interfaces.
-        return PrettyJsonWriter.reformat(contents);
-    }
-}

projects/common/src/main/resources/computercraft.mixins.json

@@ -1,13 +0,0 @@
-{
-    "required": true,
-    "package": "dan200.computercraft.mixin",
-    "minVersion": "0.8",
-    "compatibilityLevel": "JAVA_17",
-    "injectors": {
-        "defaultRequire": 1
-    },
-    "mixins": [
-        "CacheUpdaterMixin"
-    ],
-    "refmap": "computercraft.refmap.json"
-}

projects/fabric/build.gradle.kts

@@ -146,7 +146,6 @@ loom {
             client()
 
             runDir("run/dataGen")
-            property("cct.pretty-json")
             property("fabric-api.datagen")
             property("fabric-api.datagen.output-dir", file("src/generated/resources").absolutePath)
             property("fabric-api.datagen.strict-validation")

projects/fabric/src/main/java/dan200/computercraft/data/FabricDataGenerators.java

@@ -36,20 +36,28 @@ import java.util.function.Consumer;
 public class FabricDataGenerators implements DataGeneratorEntrypoint {
     @Override
     public void onInitializeDataGenerator(FabricDataGenerator generator) {
-        var pack = generator.createPack();
-        DataProviders.add(new PlatformGeneratorsImpl(pack));
-        pack.addProvider((out, reg) -> addName("Conventional Tags", new MoreConventionalTagsProvider(out, reg)));
+        var pack = new PlatformGeneratorsImpl(generator.createPack());
+        DataProviders.add(pack);
+        pack.addWithRegistries((out, reg) -> addName("Conventional Tags", new MoreConventionalTagsProvider(out, reg)));
     }
 
     private record PlatformGeneratorsImpl(FabricDataGenerator.Pack generator) implements DataProviders.GeneratorSink {
+        public <T extends DataProvider> T addWithFabricOutput(FabricDataGenerator.Pack.Factory<T> factory) {
+            return generator.addProvider((FabricDataOutput p) -> new PrettyDataProvider<>(factory.create(p))).provider();
+        }
+
+        public <T extends DataProvider> T addWithRegistries(FabricDataGenerator.Pack.RegistryDependentFactory<T> factory) {
+            return generator.addProvider((r, p) -> new PrettyDataProvider<>(factory.create(r, p))).provider();
+        }
+
         @Override
         public <T extends DataProvider> T add(DataProvider.Factory<T> factory) {
-            return generator.addProvider(factory);
+            return generator.addProvider((PackOutput p) -> new PrettyDataProvider<>(factory.create(p))).provider();
         }
 
         @Override
         public <T> void addFromCodec(String name, PackType type, String directory, Codec<T> codec, Consumer<BiConsumer<ResourceLocation, T>> output) {
-            generator.addProvider((FabricDataOutput out) -> {
+            addWithFabricOutput((FabricDataOutput out) -> {
                 var ourType = switch (type) {
                     case SERVER_DATA -> PackOutput.Target.DATA_PACK;
                     case CLIENT_RESOURCES -> PackOutput.Target.RESOURCE_PACK;
@@ -71,7 +79,7 @@ public class FabricDataGenerators implements DataGeneratorEntrypoint {
         @Override
         public void lootTable(List<LootTableProvider.SubProviderEntry> tables) {
             for (var table : tables) {
-                generator.addProvider((FabricDataOutput out) -> new SimpleFabricLootTableProvider(out, table.paramSet()) {
+                addWithFabricOutput((FabricDataOutput out) -> new SimpleFabricLootTableProvider(out, table.paramSet()) {
                     @Override
                     public void generate(BiConsumer<ResourceLocation, LootTable.Builder> exporter) {
                         table.provider().get().generate(exporter);
@@ -82,7 +90,7 @@ public class FabricDataGenerators implements DataGeneratorEntrypoint {
 
         @Override
         public TagsProvider<Block> blockTags(Consumer<TagProvider.TagConsumer<Block>> tags) {
-            return generator.addProvider((out, registries) -> new FabricTagProvider.BlockTagProvider(out, registries) {
+            return addWithRegistries((out, registries) -> new FabricTagProvider.BlockTagProvider(out, registries) {
                 @Override
                 protected void addTags(HolderLookup.Provider registries) {
                     tags.accept(x -> new TagProvider.TagAppender<>(RegistryWrappers.BLOCKS, getOrCreateRawBuilder(x)));
@@ -92,7 +100,7 @@ public class FabricDataGenerators implements DataGeneratorEntrypoint {
 
         @Override
         public TagsProvider<Item> itemTags(Consumer<TagProvider.ItemTagConsumer> tags, TagsProvider<Block> blocks) {
-            return generator.addProvider((out, registries) -> new FabricTagProvider.ItemTagProvider(out, registries, (FabricTagProvider.BlockTagProvider) blocks) {
+            return addWithRegistries((out, registries) -> new FabricTagProvider.ItemTagProvider(out, registries, (FabricTagProvider.BlockTagProvider) blocks) {
                 @Override
                 protected void addTags(HolderLookup.Provider registries) {
                     var self = this;
@@ -113,7 +121,7 @@ public class FabricDataGenerators implements DataGeneratorEntrypoint {
 
         @Override
         public void models(Consumer<BlockModelGenerators> blocks, Consumer<ItemModelGenerators> items) {
-            generator.addProvider((FabricDataOutput out) -> new FabricModelProvider(out) {
+            addWithFabricOutput((FabricDataOutput out) -> new FabricModelProvider(out) {
                 @Override
                 public void generateBlockStateModels(BlockModelGenerators generator) {
                     blocks.accept(generator);

projects/fabric/src/main/resources/fabric.mod.json

@@ -37,7 +37,6 @@
         ]
     },
     "mixins": [
-        "computercraft.mixins.json",
         "computercraft.fabric.mixins.json",
         {
             "config": "computercraft-client.mixins.json",

projects/forge/build.gradle.kts

@@ -58,7 +58,6 @@ minecraft {
                 "--existing", project(":common").file("src/main/resources/"),
                 "--existing", file("src/main/resources/"),
             )
-            property("cct.pretty-json", "true")
         }
 
         fun RunConfig.configureForGameTest() {
@@ -110,7 +109,6 @@ mixin {
     add(sourceSets.main.get(), "computercraft.refmap.json")
     add(sourceSets.client.get(), "client-computercraft.refmap.json")
 
-    config("computercraft.mixins.json")
     config("computercraft-client.mixins.json")
     config("computercraft-client.forge.mixins.json")
 }

projects/forge/src/main/java/dan200/computercraft/data/Generators.java

@@ -51,12 +51,12 @@ public class Generators {
     ) implements DataProviders.GeneratorSink {
         @Override
         public <T extends DataProvider> T add(DataProvider.Factory<T> factory) {
-            return generator.addProvider(factory);
+            return generator.addProvider(p -> new PrettyDataProvider<>(factory.create(p))).provider();
         }
 
         @Override
         public <T> void addFromCodec(String name, PackType type, String directory, Codec<T> codec, Consumer<BiConsumer<ResourceLocation, T>> output) {
-            generator.addProvider(out -> {
+            add(out -> {
                 Map<ResourceLocation, T> map = new HashMap<>();
                 output.accept(map::put);
                 return new JsonCodecProvider<>(out, existingFiles, ComputerCraftAPI.MOD_ID, JsonOps.INSTANCE, type, directory, codec, map);