Skip to content

Refactor: Fix stale enchant interactions and optimize enchant hot paths#459

Open
Chloemlla wants to merge 12 commits into
Auxilor:masterfrom
Chloemlla:master
Open

Refactor: Fix stale enchant interactions and optimize enchant hot paths#459
Chloemlla wants to merge 12 commits into
Auxilor:masterfrom
Chloemlla:master

Conversation

@Chloemlla

@Chloemlla Chloemlla commented May 31, 2026

Copy link
Copy Markdown

Summary

This PR contains a set of compatibility fixes and hot-path optimizations for EcoEnchants.

The main goals are:

  • prevent stale held-item enchant effects from firing after the relevant item interaction state changes
  • respect third-party plugins that intentionally block an anvil result
  • avoid unnecessary lore rewrites when no enchantment conversion is needed
  • reduce repeated allocation and repeated config/registry scans in common enchantment paths
  • make CI build artifacts easier to verify from pull requests and fork builds

This branch is based on upstream Auxilor/EcoEnchants@e6aa7c84 and contains 7 commits, touching 29 files.

Behavior Fixes

Held interaction refresh

Adds HeldInteractionRefreshSupport, which refreshes libreforge holders before main-hand interaction triggers when
the player already has a cached main-hand enchant using alt_click or click_block.

This is intentionally narrow:

  • ignores off-hand events
  • ignores physical actions
  • only refreshes when a cached main-hand interaction enchant exists

This addresses stale held enchant behavior where effects could remain active after switching, dropping, or otherwise
changing the held item state.

Related reports: #327, #360, #420.

Anvil result compatibility

PrepareAnvilEvent now respects an already-cleared result before EcoEnchants asynchronously replaces the preview.

If another plugin sets the prepared result to null, EcoEnchants now preserves that blocked state instead of
restoring a result afterward.

Related report: #429.

The anvil merge path also now:

  • uses a precomputed enchant limit
  • stops adding new enchants once the limit is reached
  • checks applicability against the evolving enchant set
  • precomputes repair-unit lookup data instead of scanning the repair map repeatedly

Lore conversion safety

Lore conversion now exits early when:

  • the item has no lore
  • no lore lines match known enchantments

This avoids rewriting item metadata/lore when EcoEnchants has nothing to convert, reducing the chance of interfering
with lore owned by other plugins.

Vanilla enchant conflict handling

ModifiedVanillaCraftEnchantment now caches vanilla enchant data from the key and avoids recursive conflict checks
between modified vanilla enchant wrappers.

Related report: #390.

Performance / Allocation Improvements

This PR moves several repeated scans and allocations out of hot paths.

Cached obtainable enchantment sources

Adds EnchantmentSourceCache for enchantments obtainable through:

  • enchanting table
  • villager trading
  • discovery / loot

These lists are rebuilt on reload and then reused by enchanting, villager, and loot logic.

The previous EcoEnchants.values().shuffled() pattern was replaced with randomizedIteration(), which still visits
candidates in randomized order but avoids copying and shuffling the full enchant list every time.

Display sorting

Display sorting now builds a comparator on reload instead of recursively applying sorter objects on every display
pass.

This keeps the configured rarity/type/length ordering behavior while reducing repeated wrapping, filtering, and
sorting overhead.

Enchant display

EnchantDisplay now:

  • returns early when the item has no enchants
  • reads display config values once per display application
  • reuses the player dispatcher
  • evaluates not-met holder/effect conditions directly and formats placeholders once

Command completions

/enchant and /enchantinfo completions are now cached and refreshed on plugin reload instead of recomputing
enchantment names and level ranges on every tab-complete call.

GUI and target checks

The enchant GUI now caches sorted applicable enchantments by hashed item and invalidates that cache on reload.

EnchantmentTargets.applicableEnchantments now computes the current enchant set once and checks applicability with
the same enchant-limit-aware path used elsewhere.

Other hot-path cleanups

  • Hardcoded enchant config files are indexed on reload instead of walking the enchant folder per lookup.
  • Essentials integration caches reflective map lookups.
  • Repairing gets active items once per player tick and computes excluded equipped/held items once.
  • Replenish checks ignored crop types before doing active-enchant work.
  • Grindstone XP orb cleanup uses getNearbyEntitiesByType.

Build / CI Changes

The GitHub Actions build workflow now also runs for relevant YAML, Gradle wrapper, and workflow changes, not only
Kotlin source changes.

The workflow now uploads the built jar as an artifact, excluding source and javadoc jars, so reviewers can inspect the
exact artifact produced by CI.

Packaging

The root shadow jar now includes the matching libreforge shadow artifact:

embeddedLibreforge("com.willfp:libreforge:${libreforgeVersion!!}:shadow@jar")

This makes the generated build artifact more self-contained and easier to validate from CI.

Compatibility Notes

  • No user-facing config migration is required.

  • Caches introduced in this PR are refreshed during the existing reload flow.

  • Random enchant selection still visits candidates in randomized order; it just avoids allocating a shuffled copy each time.

  • The anvil changes are conservative and preserve blocked results from other plugins instead of overriding them.

Verification

Verified through GitHub Actions on the fork:

The successful build run used current head:

a58bf7e

The build artifact was uploaded as:

EcoEnchants-a58bf7e8a5bad5f3059fedd03601c91b95cfc180

@CallumJohnson CallumJohnson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is lots of changes in this PR each seemingly semi-disconnected.

Can we break this PR into separate entries so we can review each specific change in isolation please?

Comment thread .github/workflows/build.yml Outdated
Comment thread .github/workflows/build.yml Outdated
@Chloemlla

Chloemlla commented May 31, 2026

Copy link
Copy Markdown
Author

There is lots of changes in this PR each seemingly semi-disconnected.

Can we break this PR into separate entries so we can review each specific change in isolation please?
Understood, thank you for your feedback.

I agree that this PR covers too much ground; breaking it down into several smaller PRs would make the review process easier. I'm a Minecraft enthusiast and genuinely want to contribute something useful to this project. I'm currently preparing for the Chinese National College Entrance Examination (Gaokao) from June 13th to 15th, so I won't be able to properly revise it until June 16th.

Can I come back after my exam and submit a smaller, more specific PR for each change?
@CallumJohnson

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.

2 participants