Basic mapping migration #57
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eigenraven
reviewed
Feb 11, 2024
plugin/src/main/java/com/gtnewhorizons/retrofuturagradle/modutils/ModUtils.java
Outdated
Show resolved
Hide resolved
eigenraven
reviewed
Feb 11, 2024
plugin/src/main/java/com/gtnewhorizons/retrofuturagradle/modutils/MigrateMappingsTask.java
Outdated
Show resolved
Hide resolved
19 tasks
makamys
force-pushed
the
migrate-mappings
branch
from
db5f47e to
612411e
Compare
Doesn't migrate yet, it just creates a diff between the current mappings and the target (hardcoded for now) and writes it to a file. I had to add the Paper and Sonatype repos for each subproject so they wouldn't complain about not being able to find Mercury. I don't know if there's a better to solve this.
Fixes Lorenz's service files not getting relocated
- Fix wrong dependency names - Fix absolute paths not working
Automatically wires up stuff like error messages and path resolution
makamys
force-pushed
the
migrate-mappings
branch
from
612411e to
262117c
Compare
|
Found some things I want to change:
|
Fixes task not rerunning if only the source mappings changed since the last run
makamys
commented
Feb 21, 2024
| cp.from(project.files(project.getTasks().named("packagePatchedMc", Jar.class))); | ||
| cp.from( | ||
| project.getExtensions().getByType(JavaPluginExtension.class).getSourceSets() | ||
| .getByName("injectedTags").getOutput()); |
There was a problem hiding this comment.
Not entirely sure if I'm supposed to use .named here, when I tried using it it gave me an inscrutable compiler error so I went with getting it directly.
There was a problem hiding this comment.
ConfigurableFileCollection#from evaluates whatever is passed as-if they're passed to Project#files already, no need to also call Project#files here.
eigenraven
approved these changes
Jun 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the beginnings of a
migrateMappingstask inspired by Loom's one. It can remap regular Java source code as well as mixin targets. Currently only paths to local CSV files are supported as target mappings.Testing
I tested it with Hodgepodge in the following way:
./gradlew migrateMappings --mcpDir "~/.gradle/caches/minecraft/de/oceanlabs/mcp/mcp_stable/12"minecraft.useForgeEmbeddedMappings = falseto the buildscript (causing the default ofstable_12to be used) and reimportrunClientI got the game to run with only some minor adjustments.
Hodgepodge details
@Mixin(PlacementLib$.class)to@Mixin(.class). I simply reverted this, but it could also be worked around by changing them to string constants.rayTraceBlockstarget inMixinWorld_FixXrayis ambiguous with the new mappings, so it needed to be made explicit (rayTraceBlocks(Lnet/minecraft/util/Vec3;Lnet/minecraft/util/Vec3;ZZZ)Lnet/minecraft/util/MovingObjectPosition;).