From b19cdb74dcaf187c48dfe789dcc6233e958c4f0a Mon Sep 17 00:00:00 2001 From: brachy84 <45517902+brachy84@users.noreply.github.com> Date: Fri, 16 Aug 2024 16:33:28 +0200 Subject: [PATCH] Crl fixes and more (#220) * fix classes not finding bindings * clean up some lsp stuff * test builds for this branch * fix issue with traits & auto package declaration * delete script cache when java version changed * explicitly declare lcl has parent loader * some convenience methods for logging * close #215 * dont test build this branch --- .../groovyscript/GroovyScriptConfig.java | 4 ++++ .../groovyscript/api/GroovyLog.java | 19 +++++++++++++--- .../ItemRecipeMakerMixin.java | 8 +++++-- .../core/mixin/groovy/MetaClassImplMixin.java | 8 +++---- .../core/mixin/groovy/ModuleNodeMixin.java | 11 ++++++++++ .../groovyscript/sandbox/CompiledClass.java | 7 ++++++ .../groovyscript/sandbox/CompiledScript.java | 6 ++--- .../groovyscript/sandbox/GroovyLogImpl.java | 9 +++++++- .../groovyscript/sandbox/GroovySandbox.java | 7 +++--- .../sandbox/GroovyScriptSandbox.java | 5 ++++- .../control/GroovyLSCompilationUnit.java | 2 +- .../config/CompilationUnitFactory.java | 2 +- .../config/ICompilationUnitFactory.java | 22 +++++++++---------- 13 files changed, 78 insertions(+), 32 deletions(-) diff --git a/src/main/java/com/cleanroommc/groovyscript/GroovyScriptConfig.java b/src/main/java/com/cleanroommc/groovyscript/GroovyScriptConfig.java index 7159ab83f..d09bafc7e 100644 --- a/src/main/java/com/cleanroommc/groovyscript/GroovyScriptConfig.java +++ b/src/main/java/com/cleanroommc/groovyscript/GroovyScriptConfig.java @@ -19,5 +19,9 @@ public static class Compat { @Config.Comment("Enables DE energy core compat. Config is mainly for other mods compat.") public boolean draconicEvolutionEnergyCore = true; + + @Config.Name("ExtendedCrafting recipe maker makes grs recipes") + @Config.Comment("If this is true, the recipe maker from ExtendedCrafting will produce a script for GroovyScript instead of CraftTweaker.") + public boolean extendedCraftingRecipeMakerMakesGrsRecipes = true; } } diff --git a/src/main/java/com/cleanroommc/groovyscript/api/GroovyLog.java b/src/main/java/com/cleanroommc/groovyscript/api/GroovyLog.java index 9c932c2c9..7e01f6b4c 100644 --- a/src/main/java/com/cleanroommc/groovyscript/api/GroovyLog.java +++ b/src/main/java/com/cleanroommc/groovyscript/api/GroovyLog.java @@ -14,7 +14,7 @@ import java.util.function.Supplier; /** - * A interface for the GroovyScript logger. The log is separate to Minecraft's normal and debug log. + * An interface for the GroovyScript logger. The log is separate to Minecraft's normal and debug log. * The generated log file can be found at "[Minecraft instance]/groovy.log". * All logging methods format its content similarly to how C does it. * Curly braces in the msg parameter get replaced with the given arguments. @@ -269,6 +269,19 @@ interface Msg { */ Msg add(boolean condition, String msg, Object... args); + /** + * Adds a sub message to this message with exactly one parameter, but only if the given condition is true. The arg {@link Supplier} + * is invoked if the condition is true. + * + * @param condition sub message will only be added if this is true + * @param msg sub message + * @param arg sub message argument + * @return this + */ + default Msg add(boolean condition, String msg, Supplier arg) { + return add(condition, msg, (Object) arg); + } + /** * Adds a sub message to this message, but only if the given condition is true. * For convenience. @@ -290,8 +303,8 @@ interface Msg { Msg add(boolean condition, Consumer msgBuilder); /** - * Adds an exception to the message. The exception will always be logged at last. - * The exception counts as a sub message. This message can only have one message at a time. + * Adds an exception to the message. The exception will always be logged at last. The exception counts as a sub message. This + * message can only have one message at a time. * * @param throwable exception. * @return this diff --git a/src/main/java/com/cleanroommc/groovyscript/core/mixin/extendedcrafting/ItemRecipeMakerMixin.java b/src/main/java/com/cleanroommc/groovyscript/core/mixin/extendedcrafting/ItemRecipeMakerMixin.java index 01b4b3d49..390882d3c 100644 --- a/src/main/java/com/cleanroommc/groovyscript/core/mixin/extendedcrafting/ItemRecipeMakerMixin.java +++ b/src/main/java/com/cleanroommc/groovyscript/core/mixin/extendedcrafting/ItemRecipeMakerMixin.java @@ -4,12 +4,14 @@ import com.blakebr0.extendedcrafting.item.ItemRecipeMaker; import com.blakebr0.extendedcrafting.lib.IExtendedTable; import com.blakebr0.extendedcrafting.tile.TileEnderCrafter; +import com.cleanroommc.groovyscript.GroovyScriptConfig; import com.cleanroommc.groovyscript.helper.ingredient.GroovyScriptCodeConverter; import com.google.common.base.Joiner; import net.minecraft.item.ItemStack; import net.minecraftforge.oredict.OreDictionary; import org.spongepowered.asm.mixin.Mixin; import org.spongepowered.asm.mixin.Shadow; +import org.spongepowered.asm.mixin.Unique; import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject; import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; @@ -28,6 +30,7 @@ public abstract class ItemRecipeMakerMixin { @Inject(method = "setClipboard", at = @At("HEAD"), cancellable = true) public void setClipboard(IExtendedTable table, ItemStack stack, CallbackInfoReturnable cir) { + if (!GroovyScriptConfig.compat.extendedCraftingRecipeMakerMakesGrsRecipes) return; if (Desktop.isDesktopSupported()) { boolean ender = table instanceof TileEnderCrafter; StringBuilder string = (new StringBuilder("mods.extendedcrafting.")).append(ender ? "EnderCrafting" : "TableCrafting"); @@ -54,13 +57,13 @@ public void setClipboard(IExtendedTable table, ItemStack stack, CallbackInfoRetu cir.setReturnValue(false); } + @Unique public String groovyscript$makeItemArrayShapeless(IExtendedTable table) { StringBuilder builder = new StringBuilder(); boolean isEmpty = true; for (ItemStack stack : table.getMatrix()) { if (!stack.isEmpty()) { - builder.append(groovyscript$makeItem(stack)) - .append(", "); + builder.append(groovyscript$makeItem(stack)).append(", "); isEmpty = false; } } @@ -68,6 +71,7 @@ public void setClipboard(IExtendedTable table, ItemStack stack, CallbackInfoRetu return builder.delete(builder.length() - 2, builder.length()).toString(); } + @Unique public String groovyscript$makeItemArrayShaped(IExtendedTable table, boolean removeEmpties) { List> matrix = new ArrayList<>(); int row = 0; diff --git a/src/main/java/com/cleanroommc/groovyscript/core/mixin/groovy/MetaClassImplMixin.java b/src/main/java/com/cleanroommc/groovyscript/core/mixin/groovy/MetaClassImplMixin.java index d89a43bfd..6e68b12c7 100644 --- a/src/main/java/com/cleanroommc/groovyscript/core/mixin/groovy/MetaClassImplMixin.java +++ b/src/main/java/com/cleanroommc/groovyscript/core/mixin/groovy/MetaClassImplMixin.java @@ -1,7 +1,6 @@ package com.cleanroommc.groovyscript.core.mixin.groovy; import com.cleanroommc.groovyscript.GroovyScript; -import com.cleanroommc.groovyscript.api.GroovyLog; import com.cleanroommc.groovyscript.api.IDynamicGroovyProperty; import com.cleanroommc.groovyscript.sandbox.security.GroovySecurityManager; import groovy.lang.*; @@ -23,9 +22,6 @@ public abstract class MetaClassImplMixin { @Shadow protected abstract Object doInvokeMethod(Class sender, Object object, String methodName, Object[] originalArguments, boolean isCallToSuper, boolean fromInsideClass); - @Shadow - protected MetaClassRegistry registry; - @Shadow protected abstract Object invokeMissingMethod(Object instance, String methodName, Object[] arguments, RuntimeException original, boolean isCallToSuper); @@ -48,6 +44,8 @@ public abstract class MetaClassImplMixin { @Final private MetaMethod[] additionalMetaMethods; + @Shadow protected MetaClassRegistry registry; + @Inject(method = "(Ljava/lang/Class;[Lgroovy/lang/MetaMethod;)V", at = @At("TAIL")) public void removeBlacklistedAdditional(Class theClass, MetaMethod[] add, CallbackInfo ci) { if (additionalMetaMethods.length > 0) { @@ -132,7 +130,7 @@ private Object invokePropertyOrMissing(Object object, String methodName, Object[ } else if (!isCallToSuper && object instanceof IDynamicGroovyProperty dynamicGroovyProperty) { // TODO remove in 1.2.0 value = dynamicGroovyProperty.getProperty(methodName); - } else if (object.getClass().getClassLoader() instanceof GroovyClassLoader) { + } else if (object instanceof GroovyObject) { value = GroovyScript.getSandbox().getBindings().get(methodName); } diff --git a/src/main/java/com/cleanroommc/groovyscript/core/mixin/groovy/ModuleNodeMixin.java b/src/main/java/com/cleanroommc/groovyscript/core/mixin/groovy/ModuleNodeMixin.java index f097be130..09ad6e0ee 100644 --- a/src/main/java/com/cleanroommc/groovyscript/core/mixin/groovy/ModuleNodeMixin.java +++ b/src/main/java/com/cleanroommc/groovyscript/core/mixin/groovy/ModuleNodeMixin.java @@ -3,6 +3,7 @@ import com.cleanroommc.groovyscript.GroovyScript; import com.cleanroommc.groovyscript.api.GroovyLog; import com.cleanroommc.groovyscript.sandbox.FileUtil; +import com.cleanroommc.groovyscript.sandbox.RunConfig; import org.codehaus.groovy.ast.ModuleNode; import org.codehaus.groovy.ast.PackageNode; import org.codehaus.groovy.control.SourceUnit; @@ -25,6 +26,11 @@ public abstract class ModuleNodeMixin { public void init(SourceUnit context, CallbackInfo ci) { // auto set package name String script = context.getName(); + if (!RunConfig.isGroovyFile(script)) { + // probably not a script file + // can happen with traits + return; + } String rel = FileUtil.relativize(GroovyScript.getScriptPath(), script); int i = rel.lastIndexOf(File.separatorChar); if (i >= 0) { @@ -37,6 +43,11 @@ public void init(SourceUnit context, CallbackInfo ci) { @Inject(method = "setPackage", at = @At("HEAD"), cancellable = true) public void setPackage(PackageNode packageNode, CallbackInfo ci) { if (this.packageNode == null || this.context == null) return; + if (!RunConfig.isGroovyFile(this.context.getName())) { + // probably not a script file + // can happen with traits + return; + } // package name was already set -> only copy data of new node String cur = this.packageNode.getName(); String newName = packageNode.getName(); diff --git a/src/main/java/com/cleanroommc/groovyscript/sandbox/CompiledClass.java b/src/main/java/com/cleanroommc/groovyscript/sandbox/CompiledClass.java index 117e4b606..b11997909 100644 --- a/src/main/java/com/cleanroommc/groovyscript/sandbox/CompiledClass.java +++ b/src/main/java/com/cleanroommc/groovyscript/sandbox/CompiledClass.java @@ -1,6 +1,7 @@ package com.cleanroommc.groovyscript.sandbox; import com.cleanroommc.groovyscript.api.GroovyLog; +import groovy.lang.GroovyClassLoader; import org.apache.commons.lang3.builder.ToStringBuilder; import java.io.File; @@ -47,6 +48,12 @@ public void onCompile(Class clazz, String basePath) { } } + protected void ensureLoaded(GroovyClassLoader classLoader, String basePath) { + if (this.clazz == null) { + this.clazz = classLoader.defineClass(this.name, this.data); + } + } + public boolean readData(String basePath) { if (this.data != null && GroovyScriptSandbox.ENABLE_CACHE) return true; File file = getDataFile(basePath); diff --git a/src/main/java/com/cleanroommc/groovyscript/sandbox/CompiledScript.java b/src/main/java/com/cleanroommc/groovyscript/sandbox/CompiledScript.java index 6b849d0c7..9c771ba8d 100644 --- a/src/main/java/com/cleanroommc/groovyscript/sandbox/CompiledScript.java +++ b/src/main/java/com/cleanroommc/groovyscript/sandbox/CompiledScript.java @@ -47,15 +47,13 @@ public void ensureLoaded(GroovyClassLoader classLoader, String basePath) { for (CompiledClass comp : this.innerClasses) { if (comp.clazz == null) { if (comp.readData(basePath)) { - comp.clazz = classLoader.defineClass(comp.name, comp.data); + comp.ensureLoaded(classLoader, basePath); } else { GroovyLog.get().error("Error loading inner class {} for class {}", comp.name, this.name); } } } - if (this.clazz == null) { - this.clazz = classLoader.defineClass(this.name, this.data); - } + super.ensureLoaded(classLoader, basePath); } @NotNull diff --git a/src/main/java/com/cleanroommc/groovyscript/sandbox/GroovyLogImpl.java b/src/main/java/com/cleanroommc/groovyscript/sandbox/GroovyLogImpl.java index dce75d6cc..df3484b3e 100644 --- a/src/main/java/com/cleanroommc/groovyscript/sandbox/GroovyLogImpl.java +++ b/src/main/java/com/cleanroommc/groovyscript/sandbox/GroovyLogImpl.java @@ -138,7 +138,7 @@ public Path getPath() { } /** - * Logs a info msg to the groovy log AND Minecraft's log + * Logs an info msg to the groovy log AND Minecraft's log * * @param msg message * @param args arguments @@ -347,6 +347,13 @@ public Msg add(String msg, Object... data) { @Override public Msg add(boolean condition, String msg, Object... args) { if (condition) { + if (args != null && args.length > 0) { + for (int i = 0; i < args.length; i++) { + if (args[i] instanceof Supplier s) { + args[i] = s.get(); + } + } + } return add(msg, args); } return this; diff --git a/src/main/java/com/cleanroommc/groovyscript/sandbox/GroovySandbox.java b/src/main/java/com/cleanroommc/groovyscript/sandbox/GroovySandbox.java index 21f525bb5..3bcfe9507 100644 --- a/src/main/java/com/cleanroommc/groovyscript/sandbox/GroovySandbox.java +++ b/src/main/java/com/cleanroommc/groovyscript/sandbox/GroovySandbox.java @@ -11,6 +11,7 @@ import groovy.util.ResourceException; import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; import it.unimi.dsi.fastutil.objects.ObjectOpenHashSet; +import net.minecraft.launchwrapper.Launch; import org.codehaus.groovy.control.CompilerConfiguration; import org.codehaus.groovy.runtime.InvokerHelper; import org.jetbrains.annotations.ApiStatus; @@ -82,7 +83,7 @@ protected void stopRunning() { } protected GroovyScriptEngine createScriptEngine() { - GroovyScriptEngine engine = new GroovyScriptEngine(this.scriptEnvironment); + GroovyScriptEngine engine = new GroovyScriptEngine(this.scriptEnvironment, Launch.classLoader); CompilerConfiguration config = new CompilerConfiguration(CompilerConfiguration.DEFAULT); config.setSourceEncoding("UTF-8"); engine.setConfig(config); @@ -103,11 +104,11 @@ public void load() throws Exception { Binding binding = createBindings(); Set executedClasses = new ObjectOpenHashSet<>(); - running.set(true); + this.running.set(true); try { load(engine, binding, executedClasses, true); } finally { - running.set(false); + this.running.set(false); postRun(); setCurrentScript(null); } diff --git a/src/main/java/com/cleanroommc/groovyscript/sandbox/GroovyScriptSandbox.java b/src/main/java/com/cleanroommc/groovyscript/sandbox/GroovyScriptSandbox.java index 784fb05e1..56472eddd 100644 --- a/src/main/java/com/cleanroommc/groovyscript/sandbox/GroovyScriptSandbox.java +++ b/src/main/java/com/cleanroommc/groovyscript/sandbox/GroovyScriptSandbox.java @@ -29,6 +29,7 @@ import org.codehaus.groovy.control.SourceUnit; import org.codehaus.groovy.control.customizers.ImportCustomizer; import org.codehaus.groovy.runtime.InvokerInvocationException; +import org.codehaus.groovy.vmplugin.VMPlugin; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.Nullable; @@ -115,7 +116,8 @@ private void readIndex() { if (jsonElement == null || !jsonElement.isJsonObject()) return; JsonObject json = jsonElement.getAsJsonObject(); int cacheVersion = json.get("version").getAsInt(); - if (cacheVersion != CACHE_VERSION) { + String java = json.has("java") ? json.get("java").getAsString() : ""; + if (cacheVersion != CACHE_VERSION || !java.equals(VMPlugin.getJavaVersion())) { // cache version changed -> force delete cache deleteScriptCache(); return; @@ -135,6 +137,7 @@ private void writeIndex() { JsonObject json = new JsonObject(); json.addProperty("!DANGER!", "DO NOT EDIT THIS FILE!!!"); json.addProperty("version", CACHE_VERSION); + json.addProperty("java", VMPlugin.getJavaVersion()); JsonArray index = new JsonArray(); json.add("index", index); for (Map.Entry entry : this.index.entrySet()) { diff --git a/src/main/java/net/prominic/groovyls/compiler/control/GroovyLSCompilationUnit.java b/src/main/java/net/prominic/groovyls/compiler/control/GroovyLSCompilationUnit.java index 10ccff0b1..15ef48f85 100644 --- a/src/main/java/net/prominic/groovyls/compiler/control/GroovyLSCompilationUnit.java +++ b/src/main/java/net/prominic/groovyls/compiler/control/GroovyLSCompilationUnit.java @@ -137,4 +137,4 @@ public ASTNodeVisitor recompileAndVisitASTIfContextChanged(@Nullable URI context return compileAndVisitAST(context); } -} \ No newline at end of file +} diff --git a/src/main/java/net/prominic/groovyls/config/CompilationUnitFactory.java b/src/main/java/net/prominic/groovyls/config/CompilationUnitFactory.java index 02e380342..6ac9d5b39 100644 --- a/src/main/java/net/prominic/groovyls/config/CompilationUnitFactory.java +++ b/src/main/java/net/prominic/groovyls/config/CompilationUnitFactory.java @@ -140,4 +140,4 @@ protected void addDirectoryToCompilationUnit(Path dirPath, GroovyLSCompilationUn addOpenFileToCompilationUnit(uri, contents, compilationUnit); }); } -} \ No newline at end of file +} diff --git a/src/main/java/net/prominic/groovyls/config/ICompilationUnitFactory.java b/src/main/java/net/prominic/groovyls/config/ICompilationUnitFactory.java index 231be27bc..20b1859a2 100644 --- a/src/main/java/net/prominic/groovyls/config/ICompilationUnitFactory.java +++ b/src/main/java/net/prominic/groovyls/config/ICompilationUnitFactory.java @@ -27,18 +27,18 @@ import java.util.List; public interface ICompilationUnitFactory { - /** - * If this factory would normally reuse an existing compilation unit, forces - * the creation of a new one. - */ - public void invalidateCompilationUnit(); - public List getAdditionalClasspathList(); + /** + * If this factory would normally reuse an existing compilation unit, forces the creation of a new one. + */ + void invalidateCompilationUnit(); - public void setAdditionalClasspathList(List classpathList); + List getAdditionalClasspathList(); - /** - * Returns a compilation unit. - */ + void setAdditionalClasspathList(List classpathList); + + /** + * Returns a compilation unit. + */ GroovyLSCompilationUnit create(Path workspaceRoot, @Nullable URI context); -} \ No newline at end of file +}