Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes to re-enable JavaAdapter support #50

Merged
merged 3 commits into from
Jan 21, 2025

Conversation

space928
Copy link
Contributor

This PR re-enables support for the JavaAdapter, this allows users to implement abstract classes and specific interfaces in JavaScript.

Example:

// Import necessary classes
const Runnable = Java.loadClass('java.lang.Runnable');

console.setDebugEnabled(true);
console.log("Testing JavaAdapter...");

// The JavaAdapter is added to the global context by calling `initStandardObjects()` in the KubeJSContext ctor
const test = new JavaAdapter(Runnable, {
	run: function () {
		console.log("Hello from the JavaAdapter!");
	}
})

test.run()

The JavaAdapter is needed to interface with certain mods that don't yet have a specific KubeJS integration such as CC-Tweaked which requires implementing the IPeripheral interface to define new peripherals.

It might be worth putting a disclaimer on the JavaAdapter support for now, as it has a mediocre surface area to test and for now should be considered experimental.

 - Added static conversion methods to Context
 - Added new wrapAny method to Context emulating the old WrapFactory method
 - Suppressed unused warnings on methods referenced by generated bytecode
 - Re-added the getFunction and callFunction methods used by the JavaAdapter
 - The context instance is now passed in to by the generated bytecode to the needed helper methods
 - Refactoring & comments
@LatvianModder
Copy link
Member

Looks good! I'll still probably have to clean up formatting afterwards, but I only have 2 comments so far:

  • Do the newly added static boolean toBoolean etc. methods in Context have to be static? Could you change them to be virtual? I like to have the option to override them in context implementations
  • Why did you swap the order of arguments in convertResult(Context cx, Object result, TypeInfo c)? I usually prefer Context to be the first argument in any method its present

@space928
Copy link
Contributor Author

The main reason for having context as the last argument for convertResult and toBoolean[etc] methods was to make calling them in the generated bytecode easier. The way the code is structured at the moment, the first argument for these methods is already put on the stack well before they are called (generateReturnResult() the method that generates the bytecode that uses these methods expects the Object to be converted to already be on the stack).

That being said it's not too much trouble to move context to the first argument (and hence make toBoolean etc virtual) by adding a SWAP instruction (which would presumably have a negligible if anything at all performance cost).

I'm happy to implement that if you want!

 - Converted the primitive conversion methods in Context to virtual methods
 - Reordered arguments in JavaAdapter.convertResult
 - Updated bytecode generation as needed to reflect the above changes
 - Fixed a couple of (insignificant) mistakes related to bytecode locals counting
 - Removed ScriptRuntime.getGlobal, we don't use it at the moment getTopCallScope should be sufficient and I believe getGlobal is currently broken in other ways
 - Tiny bit of refactoring
 - Changed the signature of JavaAdapter.convertResult and the generated bytecode to match
@amathieson
Copy link

Tested this on my instance by adding a ComputerCraft peripheral, seems to work well!

+1

@LatvianModder LatvianModder merged commit cf62b4b into KubeJS-Mods:main Jan 21, 2025
tomprince pushed a commit to tomprince/KubeJS-Rhino that referenced this pull request Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants