Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disabling Lepidopterology in Forestry causes Binnie to crash the game #29

Closed
CombatZAK opened this issue Aug 8, 2016 · 5 comments
Closed
Assignees
Labels
Milestone

Comments

@CombatZAK
Copy link

CombatZAK commented Aug 8, 2016

This is definitely a Binnie bug, but really at this point all bugs in Binnie's code are longterm bugs. (Also possible this is causing a negative interaction exposed by Binnie-Patcher due to changes in Forestry).

Basically, when Lepidopterology / Butterflies are disabled via config file in Forestry and Binnie's Genetics is enabled, Forge crashes during startup. This is because Binnie's Analyst item has a recipe that uses the Flutterlyzer from Forestry, and that item is not registered when Lepidopterology isn't enabled.

I decompiled binnie-mods-2.0-pre14 and found the offending code. It's in the binnie.genetics.item.ModuleItem class on line 121:

Item[] lyzers = { Mods.Forestry.item("beealyzer"), Mods.Forestry.item("treealyzer"), Mods.Forestry.item("flutterlyzer") };
    for (Item a : lyzers) {
      for (Item b : lyzers) {
        for (Item c : lyzers) {
          ...

The sad thing is that Binnie has logic in a common class elsewhere to check to see which Forestry plugins are enabled and which ones are not. He makes decisions on which GUIs to show in his machines and which BreedingSystems to enable based on that. Unfortunately, it seems he missed a reference to an item that may or may not be registered depending on configuration.

I'm going to take a crack at using ASM to simply remove the flutterlyzer from the recipe if Lepidopterology is disabled in the Forestry config. If I'm successful, I'll post the code here for Chocohead... unless someone else incorporates it into Binnie-Patcher first 😀

@CombatZAK
Copy link
Author

Should also note that even if a fix gets put in for the above issue, I have no clue how far loading would get following it. This was simply where I followed the crash report.

@CombatZAK
Copy link
Author

CombatZAK commented Aug 13, 2016

Whew, learning ASM on the fly was... an adventure. I went about my changes in a somewhat lazy, hackish way, but I figured I'd share them here in case Chocohead wants to incorporate something more elegant into Binnie Patcher.

Basically I ignored any config settings and just bluntly replaced the flutterlyzer in the Analyst recipe with a vanilla diamond. I then had to do the same thing with the Lepidopterist Database in the Registry recipe. My class transformer does this in all cases - so it's not something you would want to include unmodified in your own mod without tweaking it quite a bit. The inline comments give a clear picture of my thought process as I went about it. I've included the code here.

    /**
     * Executes the transformation on the Genetics mod
     */
    @Override
    public byte[] transform(String name, String transformedName, byte[] basicClass) {
        if (transformedName.equals("binnie.genetics.item.ModuleItem")) //target class found
            return transformModuleItem(basicClass); //modify it and return the edited class

        return basicClass; //not target class, bail out
    }

    /**
     * Transforms the ModuleItem class so that the Flutterlyzer is no longer included in the recipe
     * @param target ModuleItem class
     * @return modified ModuleItem class
     */
    private byte[] transformModuleItem(byte[] target) {
        //create class node to navigate the class
        ClassNode node = new ClassNode();
        ClassReader reader = new ClassReader(target);
        reader.accept(node, 0);

        for (MethodNode method : node.methods) { //iterate across all methods
            if (!method.name.equals("postInit")) continue; //only looking for the postInit method

            boolean lineTargetFound = false; //lets us know when we're on the right line
            int storesFound = 0; //number of array-storage operations located
            AbstractInsnNode instruction; //a cursor to the current instruction

            //start at the beginning of the method and go across all instructions
            for (instruction = method.instructions.getFirst(); instruction != null; instruction = instruction.getNext()) {
                if (!lineTargetFound && instruction.getType() == AbstractInsnNode.LINE && ((LineNumberNode)instruction).line == 121) lineTargetFound = true; //searching for line 121 the array declaration for the Analyst item
                if (!lineTargetFound) continue; //skip till we're at line 121

                //this only happens AT line 121
                if (instruction.getType() == AbstractInsnNode.INSN && instruction.getOpcode() == 83) storesFound++; //count the number of stores
                if (storesFound == 2) { //when we get to the second store...
                    instruction = instruction.getNext().getNext(); //advance the cursor twice (past DUP to ICONST_2 - index)
                    break;
                }
            }

            while (!(instruction.getType() == AbstractInsnNode.INSN && instruction.getOpcode() == Opcodes.AASTORE)) { //going to delete all the instructions that load the flutterlyzer into the array, stopping at the AASTORE instruction
                instruction = instruction.getNext(); //advance the cursor
                method.instructions.remove(instruction.getPrevious()); //delete the isntruction behind it
            } //this deletes everything between DUP and AASTORE

            method.instructions.insertBefore(instruction, new InsnNode(Opcodes.ICONST_2)); //readd the intval 2 to the stack - this is the array index
            String fieldName = BinnieFixes.obfuscated ? "field_151045_i" : "diamond"; //this chooses the right field name based on whether or not we're obfuscated
            method.instructions.insertBefore(instruction, new FieldInsnNode(Opcodes.GETSTATIC, "net/minecraft/init/Items", fieldName, "Lnet/minecraft/item/Item;")); //add an instruction to get the static field field for a vanilla diamond and put it on the stack

            lineTargetFound = false; //reset the line target flag
            storesFound = 0; //reset the AASTORE count flag
            for (;instruction != null; instruction = instruction.getNext()) { //iterate across instructions from the current position
                if (!lineTargetFound && instruction.getType() == AbstractInsnNode.LINE && ((LineNumberNode)instruction).line == 130) lineTargetFound = true; //this time we're looking for line 30, the array for recipe components for the registry
                if (!lineTargetFound) continue; //skip back to the loop till we're at line 130

                //this only happens at line 130
                if (instruction.getType() == AbstractInsnNode.INSN && instruction.getOpcode() == Opcodes.AASTORE) storesFound++; //count all AASTORE instructions, we want the third element, so we need to find two of these
                if (storesFound == 2) { //when we found our second AASTORE...
                    instruction = instruction.getNext().getNext(); //advance past the DUP instruction to ICONST_2
                    break;
                }
            }

            //going to delete all the instructions that add the Lepidopterist Database to the array -- same code as for the analyst here
            while (!(instruction.getType() == AbstractInsnNode.INSN && instruction.getOpcode() == Opcodes.AASTORE)) { //we stop iterating when we find the third AASTORE instruction
                instruction = instruction.getNext(); //advance the cursor first
                method.instructions.remove(instruction.getPrevious()); //then delete the previous instruction
            }

            method.instructions.insertBefore(instruction, new InsnNode(Opcodes.ICONST_2)); //readd ICONST_2 to the stack (array index)
            method.instructions.insertBefore(instruction, new FieldInsnNode(Opcodes.GETSTATIC, "net/minecraft/init/Items", fieldName, "Lnet/minecraft/item/Item;")); //get the static vanilla diamond item and put it on the stack

            break; //all changes are complete here, stop looking at methods
        }

        ClassWriter writer = new ClassWriter(0); //get a writer to serialize the modified class
        node.accept(writer); //serialize the class
        return writer.toByteArray(); //kick it back to ASM process
    }

Feel free to use any or all of the above code in anyway to please. That goes for anyone else viewing this issue.

@Chocohead Chocohead self-assigned this Aug 15, 2016
@Chocohead Chocohead modified the milestone: Final Patch Aug 15, 2016
@henry232323
Copy link

Thank you friend!

@henry232323
Copy link

Thing is, Lepidopterology and everything else is enabled.

@Chocohead
Copy link
Owner

Fixed in next version, thanks for the suggestions :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants