diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 113b6525..3b2feb02 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -4,51 +4,75 @@ body: - type: markdown attributes: value: >- - **Note: This issue tracker is not intended for support requests!** If you need help with crashes or other issues, then - you should [ask on our Discord server](https://discord.gg/rN9Y7caguP) instead. Unless you are certain that you - have found a defect, and you are able to point to where the problem is, you should not open an issue. -

- Additionally, please make sure you have done the following: + **Need help?** Ask on [Discord](https://discord.gg/rN9Y7caguP) instead of opening an issue. - - **Have you ensured that all of your mods (including ModernFix) are up-to-date?** The latest version of ModernFix - can always be found [on Modrinth](https://modrinth.com/mod/modernfix). - - - **Have you used the [search tool](https://github.com/embeddedt/ModernFix/issues) to check whether your issue - has already been reported?** If it has been, then consider adding more information to the existing issue instead. - - - **Have you determined the minimum set of instructions to reproduce the issue?** If your problem only occurs - with other mods installed, then you should narrow down exactly which mods are causing the issue. Please do not - provide your entire list of mods to us and expect that we will be able to figure out the problem. + **Issues that do not meet the requirements below (or are otherwise impossible to address with the given info) will be closed without investigation.** + - type: checkboxes + id: confirmations + attributes: + label: Checklist + options: + - label: I am reporting a defect, not asking for help + required: true + - label: I have searched existing issues and this has not been reported + required: true + - label: I have reduced my mod list to the minimum required to reproduce this issue (see below) + required: true - type: textarea id: description attributes: label: Bug Description description: >- - Use this section to describe the issue you are experiencing in as much depth as possible. The description should - explain what behavior you were expecting, and why you believe the issue to be a bug. If the issue you are reporting - only occurs with specific mods installed, then provide the name and version of each mod. + Describe the issue in detail. Be sure to include what you expected to happen and what actually happened. + validations: + required: true + - type: textarea + id: minimal-mods + attributes: + label: Minimal Mod List + description: >- + List ONLY the mods required to reproduce this issue. Maintainers have debugging tools that help them + locate problems quickly, but these generally don't work well in modpacks or large mod sets. + A minimal list should typically contain fewer than 10 mods. - **Hint:** If you have any screenshots, videos, or other information that you feel is necessary to - explain the issue, you can attach them here. + Reports with large mod lists will likely be closed without investigation, unless the problem is very clear. + + If you don't know which mods are causing your problem, use binary search: + + 1. Remove half your mods + + 2. Test if the issue still occurs + + 3. If yes, remove half again. If no, restore the last removed half and repeat from step 1. + + 4. Repeat until only the necessary mods remain + placeholder: "- ModernFix 5.x.x\n- SomeMod 1.2.3" + validations: + required: true - type: textarea id: description-reproduction-steps attributes: label: Reproduction Steps description: >- - Provide as much information as possible on how to reproduce this bug. Make sure your instructions are as clear and - concise as possible, because other people will need to be able to follow your guide in order to re-create the issue. - - **Hint:** A common way to fill this section out is to write a step-by-step guide. + Provide clear steps to reproduce the bug. Each step should be a single concrete action. + + Maintainers are busy and need to be able to quickly replicate your problem. Your reproduction steps should be + clear enough for someone who is unfamiliar with your mods to follow in 5 minutes or less (not counting time + to launch the game). + + Providing vague steps is likely to result in the issue being closed. + + placeholder: "1. \n2. \n3. " validations: required: true - type: textarea - id: log-file + id: diagnostic-info attributes: - label: Log File + label: Diagnostic Info description: >- - **Hint:** You can usually find the log files within the folder `.minecraft/logs`. Most often, you will want the `latest.log` - file, since that file belongs to the last played session of the game. - placeholder: >- - Drag-and-drop the log file here. + Drag and drop `latest.log` from `.minecraft/logs/` for the session where the issue occurred. + Do not paste log text inline. Issues without a valid `latest.log` will be closed. + + If a crash occurred, also attach the relevant file from `.minecraft/crash-reports/`. validations: required: true diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml index 5b621513..b22cbe7f 100644 --- a/.github/workflows/gradle.yml +++ b/.github/workflows/gradle.yml @@ -11,6 +11,8 @@ on: jobs: build: runs-on: ubuntu-22.04 + permissions: + issues: write concurrency: group: release-${{ github.ref }} cancel-in-progress: true @@ -52,6 +54,81 @@ jobs: env: CURSEFORGE_TOKEN: ${{ secrets.CURSEFORGE_TOKEN }} MODRINTH_TOKEN: ${{ secrets.MODRINTH_TOKEN }} + - name: Capture mod version + if: steps.check_branch.outputs.is_release == 'true' + run: | + echo "MOD_VERSION=$(./gradlew properties -q | grep '^version:' | awk '{print $2}')" >> $GITHUB_ENV + echo "MC_VERSION=$(grep '^minecraft_version=' gradle.properties | cut -d= -f2)" >> $GITHUB_ENV + - name: Comment on fixed issues + if: steps.check_branch.outputs.is_release == 'true' + uses: actions/github-script@v7 + with: + script: | + const { execSync } = require('child_process'); + + const branch = context.ref.replace('refs/heads/', ''); + const { data: runs } = await github.rest.actions.listWorkflowRuns({ + owner: context.repo.owner, + repo: context.repo.repo, + workflow_id: 'gradle.yml', + branch, + status: 'success', + per_page: 1 + }); + + const logArgs = runs.workflow_runs.length > 0 + ? `${runs.workflow_runs[0].head_sha}..${context.sha}` + : `-1 ${context.sha}`; + const log = execSync(`git log ${logArgs} --format=%s%n%b`, { encoding: 'utf8' }); + + const issueNumbers = new Set(); + const pattern = /(?:fix(?:es|ed)?|close[sd]?|resolve[sd]?)\s+#(\d+)/gi; + let match; + while ((match = pattern.exec(log)) !== null) { + issueNumbers.add(parseInt(match[1])); + } + + if (issueNumbers.size === 0) { + console.log('No fixed issues found in commits'); + return; + } + + const MARKER = ''; + const modVersion = process.env.MOD_VERSION; + const mcVersion = process.env.MC_VERSION; + const newLine = `- ${modVersion} for Minecraft ${mcVersion}`; + + for (const issueNumber of issueNumbers) { + try { + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issueNumber, + per_page: 100 + }); + + const existing = comments.find(c => c.body.includes(MARKER)); + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body: existing.body + `\n${newLine}` + }); + console.log(`Updated comment on issue #${issueNumber}`); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issueNumber, + body: `${MARKER}\nThe fix for this issue has been released in the following versions of ModernFix:\n${newLine}` + }); + console.log(`Created comment on issue #${issueNumber}`); + } + } catch (e) { + console.log(`Could not comment on #${issueNumber}: ${e.message}`); + } + } - name: Upload Artifacts to GitHub uses: actions/upload-artifact@v4 with: diff --git a/src/main/java/org/embeddedt/modernfix/ModernFix.java b/src/main/java/org/embeddedt/modernfix/ModernFix.java index a1fdf9b6..6f305035 100644 --- a/src/main/java/org/embeddedt/modernfix/ModernFix.java +++ b/src/main/java/org/embeddedt/modernfix/ModernFix.java @@ -2,6 +2,7 @@ package org.embeddedt.modernfix; import net.minecraft.SharedConstants; import net.minecraft.TracingExecutor; +import net.minecraft.client.Minecraft; import net.minecraft.util.Util; import net.minecraft.server.MinecraftServer; import net.minecraft.server.level.ChunkMap; @@ -51,6 +52,8 @@ public class ModernFix { if (auditAndExit || Boolean.getBoolean("modernfix.auditMixinsAtStart")) { MixinEnvironment.getCurrentEnvironment().audit(); if (auditAndExit) { + // Prevents Crash Assistant from treating mixin audit as a crash + Minecraft.getInstance().stop(); System.exit(0); } } diff --git a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/attribute_supplier_dedup/AttributeSupplierMixin.java b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/attribute_supplier_dedup/AttributeSupplierMixin.java index c214cba0..e805db0d 100644 --- a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/attribute_supplier_dedup/AttributeSupplierMixin.java +++ b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/attribute_supplier_dedup/AttributeSupplierMixin.java @@ -1,5 +1,6 @@ package org.embeddedt.modernfix.common.mixin.perf.attribute_supplier_dedup; +import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; import net.minecraft.core.Holder; import net.minecraft.world.entity.ai.attributes.Attribute; import net.minecraft.world.entity.ai.attributes.AttributeInstance; @@ -23,11 +24,11 @@ public class AttributeSupplierMixin { /** * @author embeddedt - * @reason Java 9's Map.of() implementation is significantly more compact than ImmutableMap, and we do not + * @reason more compact than ImmutableMap due to less wrapper objects, and we do not * care about insertion order in this context */ @Inject(method = "", at = @At("RETURN")) private void useCompactJavaMap(Map, AttributeInstance> instances, CallbackInfo ci) { - this.instances = Map.copyOf(this.instances); + this.instances = new Object2ObjectOpenHashMap<>(this.instances); } -} \ No newline at end of file +} diff --git a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/optimize_surface_rules/SequenceRuleSourceMixin.java b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/optimize_surface_rules/SequenceRuleSourceMixin.java index a599aa7b..428ed584 100644 --- a/src/main/java/org/embeddedt/modernfix/common/mixin/perf/optimize_surface_rules/SequenceRuleSourceMixin.java +++ b/src/main/java/org/embeddedt/modernfix/common/mixin/perf/optimize_surface_rules/SequenceRuleSourceMixin.java @@ -3,17 +3,25 @@ package org.embeddedt.modernfix.common.mixin.perf.optimize_surface_rules; import net.minecraft.world.level.levelgen.SurfaceRules; import org.embeddedt.modernfix.world.gen.SurfaceRuleOptimizer; import org.spongepowered.asm.mixin.Mixin; +import org.spongepowered.asm.mixin.Unique; import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject; import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; @Mixin(targets = {"net/minecraft/world/level/levelgen/SurfaceRules$SequenceRuleSource"}) public class SequenceRuleSourceMixin { + @Unique + private transient SurfaceRules.RuleSource mfix$optimizedSource; + @Inject(method = "apply(Lnet/minecraft/world/level/levelgen/SurfaceRules$Context;)Lnet/minecraft/world/level/levelgen/SurfaceRules$SurfaceRule;", at = @At("HEAD"), cancellable = true) private void optimizeApply(SurfaceRules.Context context, CallbackInfoReturnable cir) { - var optimized = SurfaceRuleOptimizer.optimizeSequenceRule((SurfaceRules.SequenceRuleSource)(Object) this, context); - if (optimized != null) { - cir.setReturnValue(optimized); + var optimizedSource = mfix$optimizedSource; + if (optimizedSource == null) { + mfix$optimizedSource = optimizedSource = SurfaceRuleOptimizer.optimizeSequenceRuleSource((SurfaceRules.SequenceRuleSource)(Object) this); + } + // Must check for it not being ourselves, to avoid infinite reentrance + if (optimizedSource != (Object)this) { + cir.setReturnValue(optimizedSource.apply(context)); } } } diff --git a/src/main/java/org/embeddedt/modernfix/core/ModernFixMixinPlugin.java b/src/main/java/org/embeddedt/modernfix/core/ModernFixMixinPlugin.java index c6a52cf1..995a92a0 100644 --- a/src/main/java/org/embeddedt/modernfix/core/ModernFixMixinPlugin.java +++ b/src/main/java/org/embeddedt/modernfix/core/ModernFixMixinPlugin.java @@ -109,7 +109,6 @@ public class ModernFixMixinPlugin implements IMixinConfigPlugin { @Override public void onLoad(String mixinPackage) { - } @Override diff --git a/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java b/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java index 00563452..2c12a77e 100644 --- a/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java +++ b/src/main/java/org/embeddedt/modernfix/core/config/ModernFixEarlyConfig.java @@ -239,7 +239,7 @@ public class ModernFixEarlyConfig { disableIfModPresent("mixin.bugfix.item_cache_flag", "lithium", "canary", "radium"); // DimThread makes changes to the server chunk manager (understandably), C2ME probably does the same disableIfModPresent("mixin.bugfix.chunk_deadlock", "c2me", "dimthread"); - disableIfModPresent("mixin.perf.release_protochunks", "c2me"); + disableIfModPresent("mixin.perf.release_protochunks", "c2me", "moonrise"); disableIfModPresent("mixin.launch.class_search_cache", "optifine"); disableIfModPresent("mixin.perf.faster_texture_stitching", "optifine"); disableIfModPresent("mixin.bugfix.entity_pose_stack", "optifine"); diff --git a/src/main/java/org/embeddedt/modernfix/world/gen/SurfaceRuleOptimizer.java b/src/main/java/org/embeddedt/modernfix/world/gen/SurfaceRuleOptimizer.java index 42492fa1..40b13864 100644 --- a/src/main/java/org/embeddedt/modernfix/world/gen/SurfaceRuleOptimizer.java +++ b/src/main/java/org/embeddedt/modernfix/world/gen/SurfaceRuleOptimizer.java @@ -1,9 +1,11 @@ package org.embeddedt.modernfix.world.gen; +import it.unimi.dsi.fastutil.objects.Reference2ObjectMap; import it.unimi.dsi.fastutil.objects.Reference2ObjectMaps; import it.unimi.dsi.fastutil.objects.Reference2ObjectOpenHashMap; import net.minecraft.core.Holder; import net.minecraft.resources.ResourceKey; +import net.minecraft.util.KeyDispatchDataCodec; import net.minecraft.world.level.biome.Biome; import net.minecraft.world.level.block.state.BlockState; import net.minecraft.world.level.levelgen.SurfaceRules; @@ -15,25 +17,33 @@ import java.util.Map; import java.util.Objects; public class SurfaceRuleOptimizer { - public static @Nullable SurfaceRules.SurfaceRule optimizeSequenceRule(SurfaceRules.SequenceRuleSource source, SurfaceRules.Context context) { + public static SurfaceRules.RuleSource optimizeSequenceRuleSource(SurfaceRules.SequenceRuleSource source) { // First pass: collect which biomes appear and count biome-gated branches - Reference2ObjectOpenHashMap, List> perBiomeSources = new Reference2ObjectOpenHashMap<>(); + Reference2ObjectOpenHashMap, List> perBiomeSources = null; int biomeGatedBranches = 0; - for (var innerSource : source.sequence()) { - if (innerSource instanceof SurfaceRules.TestRuleSource testRuleSource + var sequence = source.sequence(); + //noinspection ForLoopReplaceableByForEach + for (int i = 0; i < sequence.size(); i++) { + if (sequence.get(i) instanceof SurfaceRules.TestRuleSource testRuleSource && testRuleSource.ifTrue() instanceof SurfaceRules.BiomeConditionSource biomeConditionSource) { biomeGatedBranches++; + if (perBiomeSources == null) { + perBiomeSources = new Reference2ObjectOpenHashMap<>(); + } for (var biome : biomeConditionSource.biomes) { perBiomeSources.putIfAbsent(biome, new ArrayList<>()); } } } if (biomeGatedBranches < 3) { - return null; + // Just use the source as-is, not worth optimizing + return source; } // Second pass: build per-biome source lists preserving original interleaving order List noMatchSources = new ArrayList<>(); - for (var innerSource : source.sequence()) { + //noinspection ForLoopReplaceableByForEach + for (int i = 0; i < sequence.size(); i++) { + var innerSource = sequence.get(i); if (innerSource instanceof SurfaceRules.TestRuleSource testRuleSource && testRuleSource.ifTrue() instanceof SurfaceRules.BiomeConditionSource biomeConditionSource) { // Add the inner rule (condition stripped) only to the matching biomes' lists @@ -48,23 +58,41 @@ public class SurfaceRuleOptimizer { noMatchSources.add(innerSource); } } - // Compile all source lists into rule lists - Reference2ObjectOpenHashMap, List> compiledBiomeMatch = new Reference2ObjectOpenHashMap<>(perBiomeSources.size()); - Reference2ObjectMaps.fastForEach(perBiomeSources, entry -> { - List compiled = new ArrayList<>(entry.getValue().size()); - for (var src : entry.getValue()) { - compiled.add(src.apply(context)); - } - compiledBiomeMatch.put(entry.getKey(), List.copyOf(compiled)); - }); - List compiledNoMatch = new ArrayList<>(noMatchSources.size()); - for (var src : noMatchSources) { - compiledNoMatch.add(src.apply(context)); - } - return new OptimizedBiomeLookupSequenceRule(compiledBiomeMatch, List.copyOf(compiledNoMatch), context); + return new OptimizedBiomeLookupSequenceRule(perBiomeSources, List.copyOf(noMatchSources)); } public record OptimizedBiomeLookupSequenceRule( + Reference2ObjectMap, List> sourcesForBiomeMatch, + List sourcesForNoBiomeMatch + ) implements SurfaceRules.RuleSource { + @Override + public SurfaceRules.SurfaceRule apply(SurfaceRules.Context context) { + var sourcesForBiomeMatch = this.sourcesForBiomeMatch; + Reference2ObjectOpenHashMap, List> compiledBiomeMatch = + new Reference2ObjectOpenHashMap<>(sourcesForBiomeMatch.size()); + Reference2ObjectMaps.fastForEach(sourcesForBiomeMatch, entry -> { + SurfaceRules.SurfaceRule[] compiled = new SurfaceRules.SurfaceRule[entry.getValue().size()]; + var uncompiled = entry.getValue(); + for (int i = 0; i < uncompiled.size(); i++) { + compiled[i] = uncompiled.get(i).apply(context); + } + compiledBiomeMatch.put(entry.getKey(), List.of(compiled)); + }); + var sourcesForNoBiomeMatch = this.sourcesForNoBiomeMatch; + SurfaceRules.SurfaceRule[] compiledNoMatch = new SurfaceRules.SurfaceRule[sourcesForNoBiomeMatch.size()]; + for (int i = 0; i < sourcesForNoBiomeMatch.size(); i++) { + compiledNoMatch[i] = sourcesForNoBiomeMatch.get(i).apply(context); + } + return new CompiledOptimizedBiomeLookupRule(compiledBiomeMatch, List.of(compiledNoMatch), context); + } + + @Override + public KeyDispatchDataCodec codec() { + throw new UnsupportedOperationException("Do not try to serialize OptimizedBiomeLookupSequenceRule"); + } + } + + private record CompiledOptimizedBiomeLookupRule( Map, List> rulesForBiomeMatch, List rulesForNoBiomeMatch, SurfaceRules.Context context @@ -88,7 +116,7 @@ public class SurfaceRuleOptimizer { @Override public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; - OptimizedBiomeLookupSequenceRule that = (OptimizedBiomeLookupSequenceRule) o; + CompiledOptimizedBiomeLookupRule that = (CompiledOptimizedBiomeLookupRule) o; return rulesForBiomeMatch.equals(that.rulesForBiomeMatch) && rulesForNoBiomeMatch.equals(that.rulesForNoBiomeMatch); }