LordFokas / StargateTech2

http://mod-stargatetech.com
Other
29 stars 10 forks source link

CC module is not optional #94

Closed LizzyTrickster closed 9 years ago

LizzyTrickster commented 9 years ago

As shown in http://pastebin.com/qxxi2j36 by a user on the MCF thread, CC isn't optional and crashes when trying to access CC stuff when it's not present. I think this can be worked around by having @Optional notations so that if CC isnt there then the methods get stripped from SGTech

LordFokas commented 9 years ago

That doesn't make any sense. That method doesn't get called in the first place if the mod isn't there!

fnuecke commented 9 years ago

Welcome to Java being weird. I had similar issues, which is why I'm now keeping any references to other mods' stuff in a separate class to my equivalent of your plugin classes. The added layer of indirection did the trick in my case (I'm guessing that for some reason Java already looks for the missing classes/interfaces, even if it's not initializing them at that point).

Kubuxu commented 9 years ago

I was helping @marcin212 and thought I had known classloading. It is JVM bug or I have no idea. Might even build JVM in debug mode to understand what is going on.

fnuecke commented 9 years ago

If you do, I'd be very interested in your findings!

Kubuxu commented 9 years ago

Turns out it is class verifier: https://stackoverflow.com/questions/8185543/why-does-a-class-containing-a-method-call-to-a-missing-interface-within-unused-c My advice is one more class separation.

LordFokas commented 9 years ago

I do know it is a JVM issue. That never gave me a problem, nor to many other users. From what I've read in the thread @Kubuxu posted, it depends on the JVM's class loading strategy. This is mostly my fault since I took for granted that any JVM would load classes as lazily as possible (maybe because I observed that behavior in mine [Sun JVM]).

@Kubuxu how would you further separate these classes?

Kubuxu commented 9 years ago

@LordFokas I would shift methods that use unavailable API to separate class that is never being loaded as it is not being constructed or staticly referenced.

You can see not so fancy example here. We are loosing static typing of plug-ins but we never reference plug-ins' classes directly. It could be done nicer but \It works/.

LordFokas commented 9 years ago

The only thing I see there is a huge mess :| But I think I understand how to split the classes in order to fix these issues...

fnuecke commented 9 years ago

FWIW, as a more minimalistic example, instead of

class SomeModPlugin implements YourPluginInterface {
    @Override
    public void init() {
        SomeClass.methodThatTakesAnInterfaceFromAnotherMod(new ClassImplementingInterface());
    }
}

I'm doing the equivalent of

class SomeModPlugin implements YourPluginInterface {
    @Override
    public void init() {
        SomeModStuff.init();
    }
}
class SomeModStuff {
    public static void init() {
        SomeClass.methodThatTakesAnInterfaceFromAnotherMod(new ClassImplementingInterface());
    }
}

in OC, which looks utterly ridiculous, but works, and without reflection and casts and such.

Kubuxu commented 9 years ago

@LordFokas @fnuecke We did so we can choose dependencies with logic operations. But I must agree that in most cases it is too much.

LordFokas commented 9 years ago

Alright that should have fixed it. I decided to go with the most generic Java-can't-fuck-me-over-now approach I could think of while retaining minimalism. It actually ended up cleaning my plugin code.

Since it didn't give me any problems before, I need people who HAD these issues to test so we can see if it's fixed or not. Should totally be fixed though.

EDIT: Just noticed I can totally get rid of those extra final keywords now though, forgot to clean them up. Whooops.

LordFokas commented 9 years ago

Alright this should fix it. For real this time. I just need people to test it, to be sure.

LordFokas commented 9 years ago

Yup that did the job. CC is optional again :D