Migrate to mapping io #812
Conversation
|
👍, I have one of my mod, Mod Remapping API, which uses the deprecated api. Having some hook to help getting runtime mappings in one way or another would be enough in my case I think 🤔. |
You appear to also be using a lot of loader internals way beyond the old API that this PR removes, thus is likely to break anyway. We cannot and do not spend time to support these kinds of mods. I dont think we want to expose an API for getting the ful set of mappings, for 99% of usecases the MappingResolver API should be enough. There are some future changes we wish to make that would make anything beyond the already exposed API hard to maintain. If its not you should be bundling (and repackaging, or mabe JIJ?) mapping-io and your own mappings. (Or assume they will be on the classpath) |
|
I've followed your advice and moved my mod await from those deprecated apis and I am now shadowing mappingsio under a different package to avoid potential conflicts. |
|
Note to future self: Ensure that the mappings are lazily loaded only when and as required. |
| acceptor.acceptClass(className, dstName); | ||
|
|
||
| for (MappingTree.FieldMapping field : classDef.getFields()) { | ||
| acceptor.acceptField(memberOf(className, field.getName(fromId), field.getDesc(fromId)), field.getName(toId)); |
There was a problem hiding this comment.
Will field.getName(toId) return null and break stuff? Same question for method.getName(toId) below.
There was a problem hiding this comment.
This matches the code used in loom, its possible it could do just not sure if it will in practice?
|
I have updated this PR to also lazily load the mappings, in prod (after the first launch) with no mods the mappings are not loaded into memory at all. |
|
This is still ready to release, due to the possibility of breaking mods that use TMP (Was never part of the API but still) we are going to contiune to hold onto this until a major release. |
| protected final int fromId; | ||
| protected final int toId; | ||
|
|
||
| public MixinRemapper(MappingTree mappings, int fromId, int toId) { |
There was a problem hiding this comment.
It may be nice to move this to Mixin, and take an asm Remapper. MIO could then have an API to create a Remapper from a MappingTree.
|
Updated to mapping-io 0.5.0 :) |
# Conflicts: # build.gradle # src/main/resources/fabric-installer.json # src/main/resources/fabric-installer.launchwrapper.json
|
I have vastly improved the memory usage over the old TMP code :) In prod its now unlikely that the mappings will ever be read. |
This fully removes TMP from the classpath and the long deprecated/internal MappingConfiguration.getMappings() API. Does this break any mods? If so how many. Either way this should be a 0.15.x change.
The mapping tree is lazily loaded, in prod with no mods the mappings are never loaded after the first launch.
Mapping IO is repackaged and bundled within the jar, mods are not expected to make use of this.