Allow for coremod "sandboxing"

[SoniEx2]

Really just lets me stop coremods from loading minecraft classes while they're still being constructed. With a proper coremod API I can cache classes to speed up loading, while still allowing turing-complete class transformation.

[liach]

The code looks fine, but I am not very sure with the usage.

[SoniEx2]

Forge doesn't want turing-complete coremods. So I'm making my own modloader with turing-complete coremods.

One of the drawbacks of Forge is that you can load Minecraft classes while your transformer is being constructed, which leads to those classes being inaccessible to other transformers. By controlling when Minecraft classes are accessible, I can avoid that issue.

[Dockter]

Isn't @Mumfrey and @cpw working on this issue? (There may be others).

[SoniEx2]

@mcsnetworks They are not working on my own modloader, as far as I'm aware.

[jamierocks]

No - he meant work on 'modlauncher' a replacement for LaunchWrapper (as far as I can tell).

Not quite sure if the endeavour is supported by Mojang or not, or if they're going to continue to support legacy software (I'd guess not).

[SoniEx2]

I don't know anything about that. Doesn't sound like it has something to do with LegacyLauncher, tho.

[jamierocks]

I wasn't aware being a potential replacement for x, means the replacement doesn't have something to do with x

[SoniEx2]

Why should it affect LegacyLauncher development?

[jamierocks]

That wasn't the point as far as I can tell.

I think Docker was trying to convey that work on a project, with similar reasoning behind it as this pr is under works and that perhaps you should go about assisting them.

I have no clue about the ramifications of the development of modlauncher may or may not have on the development of LaunchWrapper.

[SoniEx2]

Well if modlauncher is what I think it is then it probably won't allow turing-complete coremods.

[jamierocks]

Not long ago cpw streamed some of the development of it, may be worth a watch - https://www.twitch.tv/voxcpw/v/110262934

[Deamon5550]

@SoniEx2 I don't think you know what turing-complete means.

[SoniEx2]

@Deamon5550 turing-complete means Java/ASM instead of patch files.

[simon816]

++++[>++++>++++<<-]>>[>+++>++>++++<<<<+>-]<[>+++<-]>[-<+<+>>]>>>.<<<++++[<+++<--->>-]<<-.>+++.-.-----.>>>>+++++.<<<+++++[<+++>-]<.>>++.>.<<<-.>++++[<<+++>>-]<<++.>---.

[SoniEx2]

@simon816 Java/ASM is dynamic and can be modified based on the environment, while patch files cannot be modified based on the environment.

[me4502]

But coremods are specifically FOR ASM and transformers. I don't understand what you mean.

[SoniEx2]

@me4502 Just isolate Minecraft classes from transformer classes.

You see, when transformers are being loaded, it goes like this:

construct transformer A load transformer A construct transformer B load transformer B

And thus you can get things like this:

construct transformer A load transformer A construct transformer B -> load minecraft class X -> call transformer A with class X load transformer B (issue: transformer B doesn't get called for class X) construct transformer C load transformer C (issue: transformer C doesn't get called for class X)

With the "sandboxing" - which I know sandboxing isn't the right term for it, but I can't find a better one - the action of transformer B would throw a ClassNotFoundError (or w/e it's called), and thus all transformers would get called for class X.

Or uh is this not what you were talking about? .-. Ppl get confused when I talk about the sandboxing so I thought you were talking about the sandboxing .-.

[SoniEx2]

@me4502 If you mean "turing-complete coremods", well uh clearly I meant "turing-complete class transformers" - you could also use non-turing-complete patches to transform classes.

[DenWav]

I really don't know how your brain still functions at such a low level.

[SoniEx2]

You know patch files?

Now imagine a patch file using ASM syntax.

It'd be basically a regex. And regex isn't turing-complete. Thus, ASM patches aren't turing-complete.

Then you have full blown java code.

Full blown java code is turing-complete. Unlike the sed-like solution above.

[Zidane]

Just so we're all clear here...you solution is better because...

[SoniEx2]

Would you like to transform Minecraft classes with patch files like those used by MinecraftForge?

With no option to toggle patches on or off?

Or do you prefer transforming Minecraft classes with a turing-complete programming language such as Java?

My solution is better because it's not. It's different. It just gives me more options.

[grum]

I have no idea what I am looking at. The description/commit is not clear and I am not sure what the original problem is. From what I understand however, it should be able to be solved much easier than trying to work around it by simply throwing an exception when it happens.

Don't attempt to fix broken things by other people, just have it break when they do something broken :)

[SoniEx2]

@grum The idea is to have it break when they do something broken.

You see, I don't want the game to run in a broken state. I want it to break completely.

Sure, I could just let the game run. It'll run fine, but with some quirks that shouldn't be there. It only crashes in the worst case, and that's the issue.

I want it to crash in all cases.

[grum]

I would expect a boolean flag somewhere indicating you cannot load certain classes, it getting toggled on and off surrounding the operation that cannot load these classes and a check for this flag throwing an exception. I see nothing of this in the code :(

Maybe a simpler implementation might be useful?

[SoniEx2]

@grum Oh yeah I also wanted coremods to be unable to see eachother. So transformer class A cannot interact with transformer class B (well, except through class loading, I guess).

[SoniEx2]

I could just break backwards compatibility and make the LaunchClassLoader take Class instead of String for addClassTransformer. I'd be able to do the same thing, but then I'd force it on everyone.

[grum]

It seems the code is not matching with what the intention is. Closing it, feel free to reopen one where this is fixed.

[SoniEx2]

What? The code does exactly what I need it to do. Everyone just disagrees with my way of doing it.

[Aaron1011]

@SoniEx2: I'm not entirely certain what you think your PR is doing, but it certainly doesn't 'sandbox' coremods in any way.

Your PR makes Launch.classLoader an instance variable, and moves some instance variables out of LaunchClassLoader into a helper class. That's it. It in no way prevents Minecraft classes from being loaded by transformers, nor does it 'prevent coremods from seeing each other'.

[SoniEx2]

@Aaron1011 It allows a custom mod loader to load coremods in a separate classloader.

I'm not gonna force sandboxing on everyone. IF you want sandboxing, make a modloader for it.

You know, like I'm doing.

This just lets my modloader do the sandboxing.

I just thought it'd be a better idea to do the actual sandboxing in my own modloader instead of forcing everything using LegacyLauncher to be sandboxed.

[SoniEx2]

Can this please be reopened?

[Mumfrey]

Can this please be reopened?

I think it's time to come clean now as to whether you are seriously trolling or just genuinely don't realise how ridiculous this is?

[SoniEx2]

How is it ridiculous to stop coremods from force-loading minecraft classes?

[Mumfrey]

How is it ridiculous to stop coremods from force-loading minecraft classes?

It's not ridiculous at all. Your "solution", however, is.

I don't wish to be mean, but if you fundamentally fail to understand the architecture at work here, then writing a mod system of your own is probably not the best pursuit. Moreover the changes you've proposed are at best ineffective and at worst actually just serve to make things... let's just say "less good". Certain responses you've made strongly imply that you don't fully comprehend the underlying principles of the classloader you're fiddling with.

Examples:

I could just break backwards compatibility and make the LaunchClassLoader take Class instead of String for addClassTransformer. I'd be able to do the same thing, but then I'd force it on everyone.

Open the box with the crowbar that's inside?

It allows a custom mod loader to load coremods in a separate classloader.

Except that a) this is already possible, and b) your changes don't actually achieve this in the slightest.

I think the fact that clarification was needed as to whether you are actively trolling should tell you something.

[SoniEx2]

I guess I could load a proxy IClassTransformer for every class transformer. But that just makes my code look messy.

I fully understand how ClassLoaders work. I fully understand how ClassLoader-based sandboxing works. I fully understand how class transformation works.

It's much better to apply these changes here than to add silly workarounds to my code. I hate having to add workarounds for things that I could just change.

I guess alternatively I can load LegacyLauncher inside a LaunchClassLoader, then transform the LegacyLauncher with these changes, then load the modloader inside that modified LegacyLauncher... Doesn't Forge do something like that, anyway?

[DenWav]

I fully understand how ClassLoaders work. I fully understand how ClassLoader-based sandboxing works. I fully understand how class transformation works.

Saying you know something you clearly don't doesn't make you know it. Nor does it do anything to help your credibility.

[SoniEx2]

@DemonWav Saying I know something "I clearly don't"? Excuse me?

I've worked with classloaders before.

[Mumfrey]

@DemonWav Saying I know something "I clearly don't"? Excuse me?

I've worked with classloaders before.

He was referencing the fact that the PR content itself appears to contradict you.

[SoniEx2]

It contradicts letting mod loaders create separate LaunchClassLoaders for each coremod/class transformer?

[Mumfrey]

It contradicts letting mod loaders create separate LaunchClassLoaders for each coremod?

Okay now you are just either being deliberately obtuse or actually 100% trolling and we've all fallen for it. Either way, have fun with your PR here, I'm out.

[SoniEx2]

Whereas Forge uses one global classloader in which it puts the game and the coremods in, I was allowing for separate classloader instances, where one of them would load the game's .classes, and others would load the coremods' .classes, and attempting to load the game's .classes from a coremod's loader would fail because the game classloader isn't a parent of the coremod classloader.

SharedLaunchData allows loaded class transformers to be shared with the game classloader - because while classloaders can't load classes they don't know about, they can use classes/objects from non-parent classloaders.