JnCrMx / discord-game-sdk4j

Java bindings for Discord's Game SDK
MIT License
121 stars 23 forks source link

Starting the API? The read me says it packs the natives with the JAR, but Core.init requires the natives file. #36

Open SkyAphid opened 2 years ago

SkyAphid commented 2 years ago

Hello,

I need this API to set up a discord rich presence with my game. However, I can't even get your Activity example working the way I'd like it to.

1) I managed to get it to download the natives with DownloadNativeLibrary, but it takes a few seconds and I don't want to make the player sit at a loading screen waiting for it to download. It's a single player game, so I don't want to work under the assumption their internet is fast or that the server DownloadNativeLibrary is connected to will be up/running quickly. So...

2) I tried using the natives included with your release, but get this error when I used the natives I thought was the closest approximation to my machine. This is likely a user error and just me not having the full natives that I need, but here is the error I get:

DRPC: Start true Using Discord Library: windows-amd64-discord_game_sdk_jni.dll Exception in thread "main" java.lang.UnsatisfiedLinkError: D:\Projects\Eclipse Workspace\RobotFarm\RobotFarm\discord_natives\windows-amd64-discord_game_sdk_jni.dll: Can't find dependent libraries at java.base/jdk.internal.loader.NativeLibraries.load(Native Method) at java.base/jdk.internal.loader.NativeLibraries$NativeLibraryImpl.open(NativeLibraries.java:383) at java.base/jdk.internal.loader.NativeLibraries.loadLibrary(NativeLibraries.java:227) at java.base/jdk.internal.loader.NativeLibraries.loadLibrary(NativeLibraries.java:169) at java.base/java.lang.ClassLoader.loadLibrary(ClassLoader.java:2383) at java.base/java.lang.Runtime.load0(Runtime.java:746) at java.base/java.lang.System.load(System.java:1857) at de.jcm.discordgamesdk.Core.init(Core.java:58) at com.nokoriware.robotfarm.util.DiscordRichPresence.start(DiscordRichPresence.java:51) at com.nokoriware.robotfarm.test.DiscordTest.main(DiscordTest.java:13)

3) Your readme says I shouldnt even need to keep track of the natives anyway since they're packaged with the JAR. However, there's not a version of Core.init() that doesn't require you to pass the files in, and on top of that, letting the API handle it automatically doesn't seem to be documented since all of your examples use download natives anyway.

So basically, I'm going to try and hook up the natives included in the file that DownloadNativeLibrary uses. But my issue comes down to these points:

1) I think you should maybe update your readme/release to explain a bit better how to hook up the natives locally. It seems that the natives included in the release aren't the ones you actually need. You should include a download link somewhere for the correctones.

2) Update the readme/examples to show how to load the natives automatically if they are included in the JAR and if this is an option.

I'm using the release version of the API and hooking the JAR up in my build path manually.

I appreciate the examples, but some more detailed documentation on using this for RPC would be helpful.

Thanks!

JnCrMx commented 2 years ago

There are two separate native libraries that need to be loaded:

Currently you would need to unpack the platform specific discord_game_sdk library from your JAR manually and pass the location, where you extracted it to, to Core.init(File). In the next update, it will be possible to pass an arbitrary URL to Core.init(URL), so you will be able to do something like this:

Core.init(getClass().getResource("./discord_game_sdk.dll"));

That would automatically extract the library from the classpath, assuming that you packaged it into your JAR.

I will be adding an example to further document that way of loading the library.

SkyAphid commented 2 years ago

Okay! Sorry for the slow reply, but I had gotten it to work shortly after making this post by downloading the libraries manually from the downloader class by using the link there. I;'ll try out the code you supplied. However I highly recommend making a cleaner/prettier way of doing it.

You could turn

Core.init(getClass().getResource("./discord_game_sdk.dll"));

Into

Core.initDefault()

or something.

Personally when I write my own APIs, I try to make things as straightforward and clean as possible so people can guess what a function does simply by its name. Requiring getResource() and a path still requires you to read the tutorial and copy and paste that code. It's best if you can initialize the API in as few lines as possible.

JnCrMx commented 2 years ago

Core.init(getClass().getResource("/discord_game_sdk.dll")); requires you to have the DLL available from your class path, which might not be the default for everyone (especially since you probably cannot ship out that DLL, it is from Discord and not free software after all).

A Core.initDefault() would most likely cause more harm than good, as it makes people believe that the /discord_game_sdk.dll will be available without any extra steps, while in reality it will not.

There are many different way of loading the DLL, e.g. downloading it as in the example, shipping it with your application (not sure if that wouldn't violate the license), downloading it once and loading it from there then or even asking the user to download it manually. I do not think that the library should make any decisions regarding the loading of the DLL, especially because I cannot say for sure which of the ways is the best one.

Because I cannot ship the discord_game_sdk.dll the library will sadly always require some extra steps to work.

SkyAphid commented 2 years ago

Hello again, when you've completed your changes here and pushed it, can you please leave a comment on here about how the new setup works? This is so I can be notified in the future and check it out. Thank you! Good luck!

JnCrMx commented 2 years ago

Yes, I will make a comment and close this issue once I'm done. Sadly, I don't have much free time to work on the library :( If you want to add the necessary bits to the documentation and/or create some examples, I'd appreciate that very much, just make a pull request then, but of course you don't have to.

I don't think that I will change the setup code much (maybe I could add a initFromClasspath() method) for the reasons I explained in my last comment above. I planned to add more clarifying documentation and create a quick-start example (perhaps a Gradle project you can just copy to get started).

JnCrMx commented 2 years ago

Not fully fixed, examples are still missing, so I am reopening this, until I added examples.

dsgraham81 commented 2 years ago

Are you planning on building another release with these code changes?

JnCrMx commented 2 years ago

Yes, I originally wanted to write an example first, but I guess I will just do a release now. I can always add examples later.

SkyAphid commented 2 years ago

We've been having issues getting the code to work on specific machines in general and have had to remove this API from our project, but I'll make a separate issue for that later.