Project Red - Illumination

Project Red - Illumination

37M Downloads

Project Red Compat includes old ComputerCraft API, ignores @API annotations.

dan200 opened this issue ยท 18 comments

commented

Running the latest Project Red Compat with ComputerCraft 1.73 causes ComputerCraft to crash when CC calls an API method that was introduced after ComputerCraft 1.65. As far as I can tell, this is for two reasons:

  1. Project Red Compat includes a copy of the ComputerCraft 1.65 API, despite assurances on this bug tracker that it would stop doing so. This wouldn't be a problem, except:

  2. Project:Red's version of the API is being loaded instead of ComputerCrafts newer version, in spite of the presence of @API annotations on both with correct version numbers. I can't figure out how, but it seems something is causing the @API annotations on Project Reds classes to be ignored. I asked a forge dev about this a while ago, and the theory then was the use of core modding may cause this.

commented

I can't figure out how, but it seems something is causing the @API annotations on Project Reds classes to be ignored.

@dan200 They are not. the version part of the annotation just isn't used for anything other than dependency management (i.e. the dependencies part in your @Mod). FML ignores the version number when loading APIs, it will simply load the first one it finds, which, in this case, is apparently P:Red. Do not ask me why FML isn't checking versions and loading the latest API.

FYI: If a coremod is shipping an API in its .jar file, it will indeed be always the one being loaded. But if there is no coremod shipping that API, the case explained above applies.

commented

If that is the case, then I have been mislead completely on the purpose of the @API annotation. I was first advised to use it by cpw himself :/

commented

@API simply is there to check if an API is present and add mod integration based on the API's presence instead of a mod's presence (for example if you want to use the CoFH API without CoFHLib being there). It also allows you to depend on a certain version/version range of an API. That's basically all it does right now.

commented

Temporarily updated API with bbb24bb. Released a patch version. Will continue to hunt for a solution.

commented

Oh, another note: If the mod specified as the owner of the @API is present, its API will always be loaded instead of the one found first (at least it should), unless, again, there is a coremod shipping the API, that one always has priority due to it being injected into the classpath much more early.

commented

Vexatos: It sounds like you've solved the problem. ComputerCraft is the owner of the API, but PR:Compat's version of the API is being loaded instead, because it's a core mod. Does it need to be?

commented

If P:Red is a coremod (specifically, if the jar containing the API also contains a coremod), that will ALWAYS be loaded first. Coremods should never be shipping APIs because of the issues it's causing. They shouldn't ever do that and rather use the @Optional annotations for integration.

commented

Agreed. Let's leave this to @MrTJP to look into.

commented

@MrTJP You said in #652 (comment) that you are still getting crashes. If you do, could you please post the crash log(s)? I am pretty experienced with the use of @Optional annotations, so I might be able to help you make P:Red not ship any APIs.

commented

It's interesting that you bring this up. I looked into it and realized the compatibility jar's core mod is dormant ever since CC added the redstone APIs, and will be removed ASAP. This will fix the issues?

And @dan200, sorry for the problems, I really did spend quite a bit of time trying to fix this.

On Feb 26, 2015, at 11:58 AM, Vexatos [email protected] wrote:

@MrTJP You said in #652 (comment) that you are still getting crashes. If you do, could you please post the crash log(s)? I am pretty experienced with the use of @optional annotations, so I might be able to help you make P:Red not ship any APIs.

โ€”
Reply to this email directly or view it on GitHub.

commented

(A stopgap solution to this problem is just to ship the 1.73 API, but that will only postpone the bug until later)

commented

@MrTJP if you remove the coremod in that particular .jar file and add CC, it should work. Still, you should get rid of as many shipped APIs as possible and use @Optional instead. If it keeps crashing without the API, you simply didn't use enough @Optionals ๐Ÿ˜„

commented

@MrTJP I'm extremely grateful for your help here! My apologies if my tone ever seemed to suggest otherwise. Hooray for fixes!

commented

@Vexatos even with the core mod removed, it seems as though FML is completely ignoring the optionals. Here is the full client log if it helps. http://paste.ee/p/HdnqT

Weird though. Don't know where else to stick optionals into.
https://github.com/MrTJP/ProjectRed/blob/d8afcfd3a1f738c6427783e7ae4c8d2b8300e779/src/mrtjp/projectred/compatibility/computercraft/PluginCC_BundledCable.scala

commented

The client log doesn't contain any reference to any "Optional" thing at all. It should print something in either case (Should log that an optional removal has been skipped as well as an optional thing being removed). That's really weird. As if it never detected the annotation at all.

commented

This is an issue with how Java loads classes in general. On line 20 of the PluginCC_BundledCable class there's a method call passing an interface. Java will try to classload that interface, even if the code is never reached. There's some discussion on this behavior over on stackoverflow if you're interested.

One workaround is to add a layer of indirection, i.e. add the init method to a singleton, for example, and call that from the actual init method.

commented

Thanks @fnuecke, it worked! Just used a static methods to load the interfaces instead of doing it directly.

Fixed via 1c566e8

commented

hooray!