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

Only validate loom version when mixins are to be remapped with TinyRemapper #1168

Merged
merged 6 commits into from
Sep 8, 2024

Conversation

mezz
Copy link
Contributor

@mezz mezz commented Sep 7, 2024

I think there is an inversion in the logic introduced in #980

The comments for MixinRemapType.STATIC say "Jar does not use refmaps, so will be remapped by tiny-remapper"
And the comments for validateLoomVersion say "This is only done for jars with tiny-remapper remapped mixins."

So I think the logic should be checking == STATIC instead of != STATIC

@mezz
Copy link
Contributor Author

mezz commented Sep 7, 2024

I was able to add new failing tests, and confirm that now they pass with the PR changes

I also had to change two tests in order to match the new code changes,
"Invalid loom version" and "Valid loom version"
used to check for remap type "mixin" and now they test for type "static"

@mezz mezz requested a review from Juuxel September 7, 2024 14:12
@modmuss50 modmuss50 merged commit 36a199f into FabricMC:dev/1.7 Sep 8, 2024
89 checks passed
@mezz mezz deleted the remap-compat branch September 8, 2024 10:02
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

Successfully merging this pull request may close these issues.

3 participants