Skip to content

Commit

Permalink
Changed to using ScriptObjectMirror again for running functions.
Browse files Browse the repository at this point in the history
Caused errors with some mods that defined their returns oddly.
Next breaking version should change things to return a Function directly.
  • Loading branch information
LexManos committed Jan 12, 2021
1 parent ba397d8 commit 997eab2
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 26 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#occupational hazards
/logs/
/out/
/repo/

#eclipse
**/bin
Expand Down
14 changes: 9 additions & 5 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,15 @@ publishing {
}
repositories {
maven {
credentials {
username project.properties.forgeMavenUser?:'fake'
password project.properties.forgeMavenPassword?:'news'
}
url 'http://files.minecraftforge.net/maven/manage/upload'
if (project.hasProperty('forgeMavenPassword')) {
credentials {
username project.properties.forgeMavenUser
password project.properties.forgeMavenPassword
}
url 'https://files.minecraftforge.net/maven/manage/upload'
} else {
url 'file://' + rootProject.file('repo').getAbsolutePath()
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
package net.minecraftforge.coremod;

import java.util.function.Function;

import javax.script.Bindings;
import javax.script.ScriptEngine;

import org.openjdk.nashorn.api.scripting.NashornScriptEngineFactory;
import org.openjdk.nashorn.api.scripting.ScriptObjectMirror;

class NashornFactory {
static ScriptEngine createEngine() {
return new NashornScriptEngineFactory().getScriptEngine(CoreModEngine::checkClass);
}

@SuppressWarnings("unchecked")
static <A,R> Function<A,R> getFunction(Bindings obj) {
return a -> (R)((ScriptObjectMirror)obj).call(obj, a);
}
}
26 changes: 5 additions & 21 deletions src/main/java/net/minecraftforge/coremod/CoreMod.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,44 +61,28 @@ private ITransformer<?> buildCore(Map.Entry<String,? extends Bindings> entry) {
final Map<String, Object> targetData = (Map<String, Object>)data.get("target");
final ITransformer.TargetType targetType = ITransformer.TargetType.valueOf((String)targetData.get("type"));
final Set<ITransformer.Target> targets;
final Map<String, Object> function = (Map<String, Object>)data.get("transformer");
final Bindings function = (Bindings)data.get("transformer");
switch (targetType) {
case CLASS:
if (targetData.containsKey("names")) {
Function<Map<String, Object>, Map<String, Object>> names = wrap(targetData.get("names"));
Function<Map<String, Object>, Map<String, Object>> names = NashornFactory.getFunction((Bindings)targetData.get("names"));
targets = names.apply(targetData).values().stream().map(o -> (String)o).map(ITransformer.Target::targetClass).collect(Collectors.toSet());
} else
targets = Stream.of(ITransformer.Target.targetClass((String)targetData.get("name"))).collect(Collectors.toSet());
return new CoreModClassTransformer(this, coreName, targets, wrap(function));
return new CoreModClassTransformer(this, coreName, targets, NashornFactory.getFunction(function));
case METHOD:
targets = Collections.singleton(ITransformer.Target.targetMethod(
(String) targetData.get("class"), ASMAPI.mapMethod((String) targetData.get("methodName")), (String) targetData.get("methodDesc")));
return new CoreModMethodTransformer(this, coreName, targets, wrap(function));
return new CoreModMethodTransformer(this, coreName, targets, NashornFactory.getFunction(function));
case FIELD:
targets = Collections.singleton(ITransformer.Target.targetField(
(String) targetData.get("class"), ASMAPI.mapField((String) targetData.get("fieldName"))));
return new CoreModFieldTransformer(this, coreName, targets, wrap(function));
return new CoreModFieldTransformer(this, coreName, targets, NashornFactory.getFunction(function));
default:
throw new RuntimeException("Unimplemented target type " + targetData);
}
}

@SuppressWarnings({ "unchecked" })
private <A,R> Function<A,R> wrap(Object obj) {
/*
* Takes a function returned from a previous invocation of the JS and turns it into a Function we can call from Java.
* The old way to do it was to use the Nashorn API directly, but this isn't something we can rely on as J15 removed nashorn by default
* And the point is to try and uncouple as best we can from nashorn specifics. So what we do is have the JS create a Function for us using the source for the returned function.
* This isn't great as doing new Function(function(){}) IS a Nashorn extension of JS, however it's also supported by other JS providers like Graal.
* So it's technically more compatible then using the hard dep on the java class.
*/
try {
return (Function<A,R>)scriptEngine.eval("new java.util.function.Function(" + obj.toString() + ")");
} catch (ScriptException e) {
throw new RuntimeException(e);
}
}

public boolean hasError() {
return !loaded;
}
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/net/minecraftforge/coremod/NashornFactory.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
package net.minecraftforge.coremod;

import java.util.function.Function;

import javax.script.Bindings;
import javax.script.ScriptEngine;

import jdk.nashorn.api.scripting.NashornScriptEngineFactory;
import jdk.nashorn.api.scripting.ScriptObjectMirror;

class NashornFactory {
static ScriptEngine createEngine() {
return new NashornScriptEngineFactory().getScriptEngine(CoreModEngine::checkClass);
}

@SuppressWarnings("unchecked")
static <A,R> Function<A,R> getFunction(Bindings obj) {
return a -> (R)((ScriptObjectMirror)obj).call(obj, a);
}
}

2 comments on commit 997eab2

@cpw
Copy link
Contributor

@cpw cpw commented on 997eab2 Jan 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious what the problem with the other way was?

@LexManos
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take this for example:

function init() {
  var ASMAPI = Java.type...
  return {
    'foo': {
      ...,
      'transformer': function() {
        ASMAPI.log("test")
      }
    }
}

The eval form of the cast would be:

new Function(function() {
    ASMAPI.log("test")
})

Which is executed on the root context, and that'd fail with unknown reference ASMAPI.
Either way, this bypasses all that and is only a slight reference to Nashorn. It would be nice if in the future we ask the coremod itself to return a java.util.Function reference, and not a js function.

Please sign in to comment.