LiteLoader / LiteLoader · Issues

Closed
Issue #16 · created by CreativeMD · ·

ItemPhysic Issue

Hello I'm CreativeMD the author of ItemPhysic, you might remember me we have talked to each other some time ago (MinecraftLoader loading screen issue).

Anyways, recently some posted this issue on github https://github.com/CreativeMD/ItemPhysic/issues/22

ItemPhysic removes the vanilla drop item method source code and instead adds it's one way to drop items source code. This is done to implement custom throw, which allows you to hold the drop key and charge to throw, once you will release the key it will drop the item with the charged speed (allows you to throw items over a long distance).

So maybe LiteLoader does the some thing in some way, removes the vanilla code for throwing an item and instead adds it's one way for it. Maybe we can find a solution to fix this problem?

In Regards CreativeMD

2 participants
liteloader/LiteLoader#16
  • Mumfrey @Mumfrey

    I actually don't do anything with dropping items at all, LiteLoader is purely for client-side mods so it doesn't have any interactions with "world"-related classes like items, etities or similar.

    What is more likely is that the presence of Mixin is defeating some of your transformer logic somewhere. A good test would be to see if having another mod which uses Mixin (eg SpongeForge) present causes the same issue. This would give you a good idea as to whether it's Mixin causing the problem or something LiteLoader is specifically injecting.

    Another side consideration would be that the transformer logic you are using is quite opaque and "brute force". You might consider switching to using Mixin in order to leverage the additional features and intrinsic compatibility/stability which is thereby afforded.

    Let me know how you get on. If the problem exists in Mixin or LiteLoader I'm happy to look into it and fix it for you.

  • CreativeMD @creativemd

    It's a little bit difficult to test it using SpongeForge, because this transformation is done on client-side only, but I doubt that this is the problem either, since all other transformations work just fine using SpongeForge.

    A few years ago I got into creating core mods and I never done it different. I have seen that there is a different way to transform a class, but never found enough motivation to learn it :D . It might be a good idea, but i have no idea where to find tutorials for it.

  • Mumfrey @Mumfrey

    I just cited SpongeForge as an example, you can use any mod which makes use of Mixin to test whether Mixin is the issue.

    Mixin has some extensive documentation, found here: https://github.com/SpongePowered/Mixin/wiki

  • CreativeMD @creativemd

    Ok, seems like your mixin thingy causes my transformation to be "ignored" (you are doing something with EntityPlayerSP), although I don't know why.

    Seems like i have to get into mixin first in order to fix this bug :( , but this will take some more time, because i'm rather busy with working on LittleTiles.

    Anyway thanks for your help, i hope i will eventually find time for it.

  • Mumfrey @Mumfrey

    The mixin for EntityPlayerSP in LiteLoader is for the "outbound chat" event. One possible explanation for Mixin upsetting your transformer is that Mixin causes some classes to be loaded in multiple "passes", you can find out some more information about this in this issue reply where I explain about the transformer contract to sfPlayer1.

    If you need any assistance or have any questions about how you might switch your mod to use mixins just let me know, always happy to help.

    Also if you need anything changed on my end, just let me know. I will keep this issue open until everything is resolved.

  • CreativeMD @creativemd

    Even if my transformer would be called multiple times it should still work, because it removes the content of "dropItem()" only.

    Switching to mixins might be a good idea, but unfortunately my time is limited. So if you could help me, it would make things a lot faster and easier. I have read the first part of the tutorial, so i know the basic idea behind mixin, but still have no idea where to start ...

    BTW, ItemPhysic is an old mod and most of the stuff is not rewritten since creation, so expect a lot of bad written code :D

  • Mumfrey @Mumfrey

    I'll take a look when I have some free time and show you at least some examples of how you can change your code to use mixins.

  • CreativeMD @creativemd

    Unfortunately there seems to be another issue. This time between World Generation Manager and LiteLoader. I have no idea why LiteLoader prevents my transformations to be applied, because it does something, but in the end the class has not changed.

    https://github.com/CreativeMD/World-Generation-Manager/blob/1.10.2/src/main/java/com/creativemd/generationmanager/GenerationTransformer.java#L39 This field will not be added if LiteLoader is installed.

    I know my way of doing those things might be not the best, but why does Mixin break some of my transformations?

  • Mumfrey @Mumfrey

    Without digging deeper into the code, my primary suspicion would be that it's something to do with the "two-pass" system described in the issue I linked above. Eg. if for some reason your transformers are set up in a way which causes them to only run once per session, they will run during the metadata-generation pass but fail to run on the real pass.

    Basically the Mixin transformer will never remove members from a class, the most it can do is replace class members. This means it's more likely that your transformer is somehow being inhibited than that the added fields are being removed somehow. If you're able to debug and establish that it's only running once, then there's a simple fix for that.

  • CreativeMD @creativemd ·

    At least my transformer only does it one time:

    [13:46:52] [main/INFO] [STDOUT]: [com.creativemd.creativecore.transformer.CreativeTransformer:transform:52]: [generationmanager] Patched net.minecraft.world.chunk.Chunk ... [13:46:52] [main/INFO] [malisiscore]: Found hooks for net.minecraft.world.chunk.Chunk (asv)

    (MalisisCore is not the problem, works fine alongside World Generation Manager.)

    Unfortunately it's not that easy to setup a develop workspace with LiteLoader, SVN server does not contain any 1.10 versions.

  • Mumfrey @Mumfrey

    Yes that would tend to imply that the transformer might only be running on the metadata pass, you can easily tell by putting a Thread.dumpStack() next to that log message to see if ClassInfo is in the stack trace.

    If this is the case then you can either fix your transformer to support running multiple times, or you can tell Mixin to skip your transformer when generating metadata by simply annotating the transformer class with javax.annotation.Resource (an arbitrary annotation which was chosen simply because it means nothing and is widely available), this acts as a marker to tell Mixin to not process the transformer in the first pass.

    Unfortunately it's not that easy to setup a develop workspace with LiteLoader, SVN server does not contain any 1.10 versions.

    I'm not sure what you mean by that? The SVN server (Assembla) hasn't been used for a few years, all the source code is here, and there absolutely is a 1.10 version, it's on the branch called 1.10 so you can just check it out easily. Alternatively a much easier solution if you're using ForgeGradle is just to add

    apply plugin: 'net.minecraftforge.gradle.liteloader'

    and then run setupDecompWorkspace, it will add liteloader to your workspace entirely automatically. You must be using ForgeGradle 2.1 or later for this to work.

    If you aren't running ForgeGradle, you can still use the dev artifacts directly from my snapshots repository and simply specify a deobfCompile dependency on com.mumfrey:liteloader:1.10-SNAPSHOT and it will put liteloader in your workspace.

    Unfortunately it's not that easy to setup a develop workspace with LiteLoader, SVN server does not contain any 1.10 versions.

  • CreativeMD @creativemd

    The stack trace:

    [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: java.lang.Exception: Stack trace [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at java.lang.Thread.dumpStack(Unknown Source) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at com.creativemd.generationmanager.GenerationTransformer$1.transform(GenerationTransformer.java:40) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at com.creativemd.creativecore.transformer.CreativeTransformer.transform(CreativeTransformer.java:46) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at com.creativemd.creativecore.transformer.CreativeTransformer.transform(CreativeTransformer.java:33) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at net.minecraftforge.fml.common.asm.ASMTransformerWrapper$TransformerWrapper.transform(ASMTransformerWrapper.java:249) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.TreeInfo.applyTransformers(TreeInfo.java:152) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.TreeInfo.loadClass(TreeInfo.java:93) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.TreeInfo.getClassNode(TreeInfo.java:60) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.ClassInfo.forName(ClassInfo.java:1537) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.MixinInfo.getTarget(MixinInfo.java:783) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.MixinInfo.readTargets(MixinInfo.java:773) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.MixinInfo.readTargetClasses(MixinInfo.java:738) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.MixinInfo.<init>(MixinInfo.java:679) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.MixinConfig.prepareMixins(MixinConfig.java:467) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.MixinConfig.prepare(MixinConfig.java:400) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.MixinTransformer.prepareConfigs(MixinTransformer.java:638) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.MixinTransformer.select(MixinTransformer.java:572) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.MixinTransformer.transform(MixinTransformer.java:481) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at org.spongepowered.asm.mixin.transformer.MixinTransformer$Proxy.transform(MixinTransformer.java:185) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at net.minecraft.launchwrapper.LaunchClassLoader.runTransformers(LaunchClassLoader.java:279) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at net.minecraft.launchwrapper.LaunchClassLoader.findClass(LaunchClassLoader.java:176) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at java.lang.ClassLoader.loadClass(Unknown Source) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at java.lang.ClassLoader.loadClass(Unknown Source) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at java.lang.Class.forName0(Native Method) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at java.lang.Class.forName(Unknown Source) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at net.minecraft.launchwrapper.Launch.launch(Launch.java:131) [17:29:02] [main/INFO] [STDERR]: [java.lang.Thread:dumpStack:-1]: at net.minecraft.launchwrapper.Launch.main(Launch.java:28) [17:29:02] [main/INFO] [STDOUT]: [com.creativemd.creativecore.transformer.CreativeTransformer:transform:52]: [generationmanager] Patched net.minecraft.world.chunk.Chunk ... [17:29:02] [main/INFO] [malisiscore]: Found hooks for net.minecraft.world.chunk.Chunk (asv)

    Still only gets called one time.

    I might have read the wrong tutorial (already out of date), i will try to setup a new workspace and do some testing.

  • Mumfrey @Mumfrey

    Yes so that makes sense, your transformer is running during the metadata phase but not the actual transformation phase. As I said, you can inhibit this behaviour by simply decorating your transformer with the javax.annotation.Resource annotation if you want a simple solution.

  • CreativeMD @creativemd

    Unfortunately this does not change anything :(

  • Mumfrey @Mumfrey ·

    Which class are you decorating?

    To be clear, this is the class you need to decorate in order to inhibit the processing.

  • CreativeMD @creativemd

    GenerationTransformer & CreativeTransformer.

  • Mumfrey @Mumfrey ·

    In the log file you should see where Mixin enumerates the transformers to delegate to, you should see

    Rebuilding transformer delegation list:

    followed by a bunch of lines which start with Adding: or Excluding:. If the decoration is working properly then it should show your transformers as Excluding.

    If you could post that section of the log, it would be helpful. Bear in mind that the delegation list may be rebuilt several times during startup.

  • CreativeMD @creativemd

    [18:33:54] [main/DEBUG] [mixin/]: Adding: $wrapper.com.creativemd.generationmanager.GenerationTransformer

    Is there another way to mark the transformer to be excluded?

  • Mumfrey @Mumfrey

    No, that's normally sufficient. What is the $wrapper prefix on the transformer class?

  • CreativeMD @creativemd ·

    I have no idea, might want to take a look yourself? fml-client-latest.log

  • Mumfrey @Mumfrey

    Ok so I did some investigation, there seems to be two problems

    • Forge does something weird with transformers where it "wraps" them for some reason, see here
    • Your transformer can only ever run once because instead of adding transformers to the transformer chain, you're doing some slightly strange "sub transformer" thing where your actual transformers are run by the "main" transformer. This wouldn't be a problem except that you are removing the transformers from your sub-transformer list when they run.

    Basically, the forge issue I will have to ask the Forge guys about, it seems like the "wrapper" thing they are doing defeats the check for the annotation, this is obviously problematic so I will ask them about it.

    It seems like all you need to do to fix your problem is simply not remove the "sub transformers" from the list when the transformer is run.

    To be clear, as I explained at length in the issue mentioned above, the transformer contract does not stipulate any behaviour other than bytecode in -> bytecode out and thus any "stateful" behaviour of a transformer is problematic because the contract allows a transformer to be run as many times as necessary for a given environment. It seems like your transformers are set up to assume they will only be run once, which is the source of the problem because Mixin needs to run them as well.

    Seems like it should be simple to fix though, just get rid of the call to remove to allow the transformers to run as many times as necessary.

  • CreativeMD @creativemd

    Oh yes totally forgot about that one ... sorry ... but unfortunately i cannot fix it that easily. I'm using a rather "stupid" way of transforming names. I'm reading the 'notch-mcp.srg' file and build a cache of all names. This takes some RAM, so i want to make sure that this will be deleted once all transformations have been done (that's why i'm removing them from the list), if i won't remove it will be difficult to determine if the cache is still needed or could be deleted. https://github.com/CreativeMD/CreativeCore/blob/1.10.2/src/main/java/com/creativemd/creativecore/transformer/TransformerNames.java#L26

    I should just try to get into mixin and rework all of my transformers.

  • Mumfrey @Mumfrey

    To be honest with you, the amount of memory consumed will be utterly insignificant in the grand scheme of things, it will be completely eclipsed by things like texture and chunk data. You're better off just keeping it in memory.

    If you are desperate to provide memory cleanup, you're better off storing the map data in a WeakReference and then just lazy-loading it (eg. reload if it expires) but I personally wouldn't bother.

    It does seem like Mixin would be a better solution for you overall though, it takes care of all this kind of thing and is actually easier to maintain and read at the end of the day.

    In the short term though, I would recommend just removing the state check entirely and just keep the (couple of KB) of mapping data in memory, it'll fix the issue and only cost a tiny, insignificant amount of RAM in the time until you can rework your transformers.

  • CreativeMD @creativemd ·

    Yeah might be the best for now. Thanks for all your patience. It's been a pleasure to work with you :D , hope i can get into Mixin soon and rework all this bullshit.

  • Mumfrey @Mumfrey

    Status changed to closed