Fix Forge 50 (1.20.6) #219
Conversation
|
1.21 exists now but this would still be very nice |
| @@ -148,3 +148,53 @@ jobs: | |||
| with: | |||
| name: Reproducible Build ${{ matrix.os }} (${{ matrix.java }}) Results | |||
| path: build/reports/ | |||
|
|
|||
| # Special case this test to run on Java 21 | |||
There was a problem hiding this comment.
Note (mostly for myself): this can be removed in 1.7 since the Fabric API test requires it
| @@ -37,6 +37,14 @@ public static String intermediary(Project project) { | |||
| return intermediaryNamespace(project).toString(); | |||
| } | |||
|
|
|||
| /** | |||
| * Returns the runtime intermediary namespace of the project. | |||
There was a problem hiding this comment.
Nit: maybe production instead? There is a dev runtime too.
There was a problem hiding this comment.
this also applies in dev runtime I believe
| @@ -50,10 +50,10 @@ public abstract class MixinExtensionApiImpl implements MixinExtensionAPI { | |||
| public MixinExtensionApiImpl(Project project) { | |||
| this.project = Objects.requireNonNull(project); | |||
| this.useMixinAp = project.getObjects().property(Boolean.class) | |||
| .convention(project.provider(() -> !LoomGradleExtension.get(project).isNeoForge())); | |||
| .convention(project.provider(() -> !LoomGradleExtension.get(project).isNeoForge() && (!LoomGradleExtension.get(project).isForge() || !LoomGradleExtension.get(project).getForgeProvider().usesMojangAtRuntime()))); | |||
There was a problem hiding this comment.
The logic here is a bit ugly, it could be better for readability to use a block lambda.
@shedaniel Do you remember if there's any reason why we have this set to false for Fabric? Upstream 1.6 has it on true: https://github.com/FabricMC/fabric-loom/blob/c4d36fac4ea7ccd1ef9526aa138139a154c8f581/src/main/java/net/fabricmc/loom/extension/MixinExtensionApiImpl.java#L51-L52
(possibly for common projects that need their refmap to be transformed? But that's not needed if refmaps are removed altogether)
There was a problem hiding this comment.
I believe old forge mods should have this as false
There was a problem hiding this comment.
Yeah, that's fine. I think we could change it for Fabric if it didn't break common projects not getting refmaps on old Forge (arch plugin could set it to true?)
There was a problem hiding this comment.
Formatting these to match our checkstyle rules (or at least have proper indentation and brace placement) would be good but not strictly necessary
There was a problem hiding this comment.
It's a copy paste of Forge's MDK
currently blocked by architectury/architectury-loom#219
Co-authored-by: Juuz <[email protected]>
Co-authored-by: Juuz <[email protected]>
Co-authored-by: Juuz <[email protected]>
Co-authored-by: shedaniel <[email protected]> Co-authored-by: Juuz <[email protected]>
Co-authored-by: shedaniel <[email protected]> Co-authored-by: Juuz <[email protected]>
currently blocked by architectury/architectury-loom#219
Blocked by Juuxel/union-relauncher#1