Skip to content

Fix accessor redirects, patch priority sorting, and CreativeModeTab coremod descriptor - #2297

Merged
Su5eD merged 3 commits into
Sinytra:26.1.xfrom
Spagles:26.1.x
Aug 15, 2026
Merged

Su5eD merged 3 commits into
Sinytra:26.1.xfrom
Spagles:26.1.x

Conversation

@Spagles

@Spagles Spagles commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

The Issue

This PR addresses three distinct bugs in the transformer and coremod pipeline:

  1. Broken Accessor Redirects (AccessorRedirectTransformer): In AccessorRedirectTransformer, this.methodRenames was assigned an immutable snapshot (Map.copyOf(methodRenames)) at construction time before any patches were registered or transformed. Because the transform lambda populated the local map instance after construction, this.methodRenames remained empty at all times during class processing. Consequently, call sites for redirected accessors were never renamed to their prefixed names (connector$redirect$...), resulting in runtime NoSuchMethodError crashes.
  2. Inverted Patch Priority Order (PatchRegistrarImpl): PatchRegistrarImpl.getSortedPatches() sorted patch registrations using natural (ascending) order by priority value (Comparator.comparingInt(Registration::priority)). Since PatchRegistrar defines HIGHEST_SYSTEM_PRIORITY = 1000 and LOWEST_SYSTEM_PRIORITY = -1000, highest-priority patches (including BuiltinConnectorPlugin priority patches) were being applied last instead of first.
  3. Outdated Descriptor in Coremod (ConnectorClassProcessor): The synthetic CreativeModeTab constructor injection in ConnectorClassProcessor was still using Lnet/minecraft/resources/ResourceLocation; in its GETSTATIC instruction and constructor signature descriptor instead of Lnet/minecraft/resources/Identifier;, causing NoSuchFieldError and NoSuchMethodError in 26.1+.

The Proposal

  • AccessorRedirectTransformer.java: Made methodRenames a direct ConcurrentHashMap field on the transformer and removed the premature Map.copyOf snapshot in the constructor. The transform lambda now populates this.methodRenames directly, ensuring method renames persist and are applied during process(ClassNode).
  • PatchRegistrarImpl.java: Inverted the priority comparator in getSortedPatches() using Comparator.comparingInt(Registration::priority).reversed() so higher integer priorities execute first.
  • ConnectorClassProcessor.java: Replaced all Lnet/minecraft/resources/ResourceLocation; descriptors in the CreativeModeTab constructor injection with Lnet/minecraft/resources/Identifier;.

Possible Side Effects

  • Method patches registered with higher priority values will now correctly execute ahead of lower/default priority patches.
  • Accessor redirects will now properly rewrite bytecode instructions in transformed mod classes.
  • No public APIs or external behavior's were broken.

Alternatives

  • For PatchRegistrarImpl, inverting the integer priority scale was considered, but updating the comparator preserves the existing constants (HIGHEST_SYSTEM_PRIORITY = 1000, LOWEST_SYSTEM_PRIORITY = -1000) and keeps plugin compatibility intact.
  • For AccessorRedirectTransformer, passing the rename map through an explicit post-processing step was considered, but keeping a ConcurrentHashMap directly on the processor is cleaner and safe for parallel jar transformations.

Additional Notes

None.

@Su5eD
Su5eD merged commit c84a96a into Sinytra:26.1.x Aug 15, 2026
1 check passed
@github-project-automation github-project-automation Bot moved this from 🆕 New to ✅ Done in Connector Triage Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants