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

Rewrite Assembly Transformers #349

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

iHDeveloper
Copy link
Contributor

@iHDeveloper iHDeveloper commented Jul 26, 2020

Info

Assembly transformers are a huge part of the SkyblockAddons. And, the current architecture of it doesn't provide the flexibility for the mod to support multiple versions of Minecraft. Since the mod is going to support 1.12.2, 1.15.2 (#334 #236 #315)

There are many duplications when writing a transformer. And, it's hard to keep track of it, especially when supporting a new version of the game. 😐

This PR provides a new architecture that will solve most of these problems. And, It's very flexible and smooth to work with. And, It will allow us to support multiple versions of Minecraft. 😄

Also, this PR is part of #337. Since the mod features depend on the assembly transformers.

Note: This PR rewrites a huge part of the Mod. And, it should be dealt with very carefully. In order not to break any features.

Architecture

image

General

The architecture will isolate the assembly transformers from the mod in a standalone module. So, the mod can focus on the features while the assembly transformers focus on injecting hooks that trigger those features by safely transforming classes.

Name Description
Forge It will give us the bytecode of each class
SkyblockAddons Parse the bytecode as Class Node. Usually, we don't need to change this part in every version we will support.
ASM-API Pass the Class Node through the transformers to change how it behaves. This part is dynamic since each Minecraft version has big changes than the old one.

Assembly Transformers

image
The architecture will break the transformation process into parts explained in the table below. Each part has its task to do. Which will provides us the flexibility that we need in making new features.

Name Description
Assembly API An interface for the mod to interact with. Without specifying a version
Engine It takes the Class Node as input and transforms it through registered transformers. And, register hook for each transformer if required
Transformer Inject hooks using Assembly Helper to change how certain class or method behaves into triggering those hooks.
Assembly Helper Apply a change into the bytecode of class, method, etc... It's included in the API since it's not a dynamic part in the architecture
Hook An easy way to listen to triggered actions by a trigger that was injected by Transformer

Note about hooks: In the architecture I'm assuming that each feature of the mod is cross-version. So, the mod should tell the engine what are the hooks to trigger.
Basically, the assembly transformer API doesn't hold any hooks for each version. Instead of that, the mod will hold them.

Reference: #236

Getting Started

W.I.P

Checklist

API

  • Create module ASM-API
    • Implement an identifier for class, method, etc...
    • Provide an interface for the transformer
    • Implement functionality to the engine
    • Implement assembly helpers in the API
      • Assembly finder
      • Assembly inserter
      • Assembly replacer
      • A helper that connects all of them together
    • Implement an interface for the hook in the API
    • Implement each hook in the engine
    • Implement an interface for each transformer

Minecraft 1.8.9

  • Create module ASM-V18 for Minecraft 1.8.9
    • Implementation for each transformer
    • Implement the engine

Integrate the mod with the API

  • Compile ASM-V18 with Forge 1.8.9
  • Transform using ASM-V18 in 1.8.9

Note

This PR is under heavy development. And, anything in this description is subject to change at any time.

This PR was planned before the multiVersion branch came out. That's why it's based on the development branch.

I'm open for any discussion about this PR to see how can we improve it 😃

The assembly transformer api goal is to provide a flexible way to transform classes. The current one doesn't help the mod to support multiple versions. And, It has many duplicates to maintain and other issues. The new api takes the class node as input. And, changes the node by the transformers.
Lombok is a powerful tool. And, since the project is growing to be multi-module. It would be powerful to have this tool applied to every module on it.
The implementation contains class, method and field identifiers. The class identifier is extendable to provide flexibility for the architecture. The identifiers are helpers since they help us orgnaize what to transform. The identifiers support any class and they're not stricted to Minecraft classes only.
@iHDeveloper
Copy link
Contributor Author

The new architecture will allow us to define transform classes with the same name as they are. Since the module doesn't have decompiled classes as a dependency. And, it doesn't need to be. 😁

Each class that's going to be transformed. We are going to define a class for it. We don't have to define a class for the fields or methods. This will give us flexibility in transforming the class.

The write of the transform class shouldn't be in the ASM-API module.

Example of Transform Class

Class: net/minecraft/client/Minecraft
This implementation doesn't have searge, notch18 name identifiers
I'm still trying to find a work around that

import codes.biscuit.skyblockaddons.asm.api.helper.TransformClassHelper;
import codes.biscuit.skyblockaddons.asm.api.helper.TransformFieldHelper;
import codes.biscuit.skyblockaddons.asm.api.helper.TransformFieldTypes;
import codes.biscuit.skyblockaddons.asm.api.helper.TransformMethodHelper;
import lombok.Getter;

@Getter
public final class Minecraft extends TransformClassHelper {
    private final String IReloadableResourceManager = "net/minecraft/client/resources/IReloadableResourceManager";

    // Fields
    private TransformFieldHelper mcResourceManager = new TransformFieldHelper(this, "mcResourceManager", IReloadableResourceManager);

    // Methods
    private TransformMethodHelper refreshResources = new TransformMethodHelper("refreshResources");
    private TransformMethodHelper rightClickMouse = new TransformMethodHelper("rightClickMouse");
    private TransformMethodHelper isIntegratedServerRunning = new TransformMethodHelper("isIntegratedServerRunning", TransformFieldTypes.BOOLEAN);
    private TransformMethodHelper runTick = new TransformMethodHelper("runTick");
    private TransformMethodHelper clickMouse = new TransformMethodHelper("clickMouse");
    private TransformMethodHelper sendClickBlockToController = new TransformMethodHelper("sendClickBlockToController", TransformFieldTypes.BOOLEAN);

    @Override
    public String getName() {
        return "net/minecraft/client/Minecraft";
    }

}

The transformer is designed in a way to identify its target. And, to transform those targets by receiving the class node of those targets.
- Change the OW2-ASM from 9.0-beta to 5.0.3
- Add Google Guava 17.0
- Make all dependencies compile only. Since the module is going to be included with the main module
The engine is the core of the transformation process. The engine now is able to register transformers and check if the class needs to transform by any transformer or not.
@iHDeveloper
Copy link
Contributor Author

This is a simple implementation of a transformer.

Example of Transformer

import codes.biscuit.skyblockaddons.asm.api.helper.TransformClassHelper;
import org.objectweb.asm.tree.ClassNode;

public final class MinecraftTransformer extends Transformer {

    // Class Helper
    private Minecraft minecraft = new Minecraft();

    @Override
    public TransformClassHelper[] targets() {
        return single(minecraft);
    }

    @Override
    public void transform(TransformClassHelper classHelper, ClassNode node) {
        // TODO Transform the class's node using assembly helpers
    }

}

- Replace 'asm' and 'asm-tree' with 'asm-debug-all'
- Add Apache Commons Lang v3
- Implement a way to change the write flags and read them in the engine
- Give access to the engine from the transformer in order to change the write flags
- Add Apache Log4J v2.0-beta9
- Give access to the engine's logger. For the transformers to log there.
- Create interface for hook in order to identify the hooks in the engine
- A helper for hook if it has result. Even if there's no value but the hook has action to do without value.
@biscuut
Copy link
Collaborator

biscuut commented Jul 29, 2020

Hey there @iHDeveloper . This looks pretty cool and I appreciate that you put a lot of time into it.

However, I don't really think this is really necessary for this single project, ASM allows me to modify the code at a very low level, and I really don't mind that. In fact, we used to use a simpler framework for ASM called mixins: https://github.com/SpongePowered/Mixin but we ended up switching to plain old ASM. There's definitely some thingswe could add to make the ASM a bit easier to use, but I don't think a whole rewrite is necessary.

However, you seem to have a lot of cool ideas here and I really think you should consider making this into its own project!

@iHDeveloper
Copy link
Contributor Author

Hello @biscuut.

Thank you for your response. 😄

The goal of the rewrite is to support multiple Minecraft versions. Since the features will be cross-version, transformation is a dynamic part that will require to be changed for each version.

Also, the rewrite will allow us to balance between flexibility and performance. Since the transformation process happens on the runtime.

@iHDeveloper
Copy link
Contributor Author

Hello, again @biscuut.

Right now I'm not sure if I should continue working on this PR or not? 😅

The comment above describes the reason of why I did it as a rewrite.

@ILikePlayingGames ILikePlayingGames added the enhancement New feature or request label Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants