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

Add Enhanced Hotswapping support (adding/removing members & maybe adding interfaces) #116

Open
XXMA16 opened this issue Jul 30, 2023 · 0 comments

Comments

@XXMA16
Copy link

XXMA16 commented Jul 30, 2023

This would need at least a partial rewrite of the Mixin Agent, but would be a very useful tool for mod developers. This new system would work as follows:

  • keep track of the mixin configs from the mod being developed (would need some changes to the Fabric Loader as well)
  • reload the mod's mixin configs and MixinConfigPlugins on hotswap
  • remove all mixins that were loaded by the mod in development
  • create the new mixins
  • apply the new mixins accordingly (taking preApply and postaApply into account)

While this system is pretty slow, it should be taken into consideration that it is exclusive to development environments and it's way faster than restarting. In order to not mess up non-DCEVM environments, this should be an opt-in (i.e. needing -Dmixin.hotSwap.dcevm=true flag to enable)

A very crude version of hotswapping new methods can be achieved by doing the following:

Reloaded(State previous, ClassNode classNode) {
    super(classNode, previous.getClassInfo());
    this.previous = previous;
    // inserted
    for (MethodNode method : classNode.methods) {
        this.getClassInfo().addMethod(method);
   // end inserted
  }
}

This does indeed work properly when it comes to @Shadowing and injecting, the caveat being that when methods are removed they still remain in the ClassInfo (but are in fact removed from the target classes). Additionally, it is possible to @Override methods of the superclass and add @Unique methods even without this modification and if the Injectors and @Shadowed methods were present on launch they can be reused in a hotswap that adds them back (assuming a previous hotswap removed them) due to the aforementioned fact that members are never added nor removed from ClassInfo.
However, adding @Unique and @Shadow fields will not work unless the fields are added to ClassInfo. To do this, one can add the same code as above to the constructor of Reloaded, but replacing every mention of method with field and creating the following method in ClassInfo:

void addField(FieldNode field) {
    this.fields.add(new Field(field, true));
}

Interaces are tricky since they can be added, but trying to remove them screws stuff up since their removal isn't even supported by JetBrains Runtime (and hasn't been supported since DCEVM 7). To get the interface addition working MixinInfo.validateChanges() needs to throw MixinReloadExceptions when this.interfaces does not contain everything in this.prevoius.interfaces, instead of when they differ, but if the user tries to remove the interface and reload it's already too late as any subsequent hotswap will fail due to internal inconsistency or security problem. Removing the check for instanceof Reloaded in MixinInfo.readImplementation() is not required to get this to work.

Changing, adding or removing the superclass of a Mixin (not the target class; that shouldn't happen anyway) works properly without making any modifications.

As stated above, this is almost certainly a bad way of approaching the problem, but it's a simple proof of concept. In order to tap into the full power of DCEVM hotswapping we'd also need a way of reloading the mixin config (preferably only for the mod being developed in order to not waste time) and adding, removing and changing mixins accordingly. With the previously mentioned patches there arises another issue: there is no guarantee that the MixinConfigPlugin class gets reloaded before the Mixin classes. I don't know if there's a way of controlling the class redefinition order, but even if there is it might not be worth fixing this.

I understand achieving this would involve a lot of work, but the benefits of such a system would be significant.

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

No branches or pull requests

1 participant