Modernise enchantments by bluelhf · Pull Request #8668 · SkriptLang/Skript

bluelhf · 2026-05-28T20:43:25Z

Problem

The current enchantment syntaxes are not yet migrated to the new module-based registration system, and their code is in many places out of date both in style and in logic.

Solution

This PR adds a new module for syntaxes related to enchantments, moves the existing syntaxes to the new module, improves their code style and flow and implements three new syntaxes:

Minimum enchantment level: the minimum starting level for an enchantment in vanilla Minecraft. This is 1 for all currently existing enchantments (but I still made it use the Bukkit API instead of just returning 1!)
Maximum enchantment level: the minimum's more useful sibling, e.g. 5 for Efficiency.
Stored enchantments: the enchantments stored in an enchanted book (or anything else with the data component!)

Testing Completed

There is a new unit test for stored enchantments. I ran quickTest to ensure that my changes don't break any of the existing tests.

Supporting Information

I would appreciate if people with scripts that use these syntaxes extensively could test to make sure they still work identically to how they were before.

Completes: none
Related: none
AI assistance: Gemini 3.1 Pro was used to evaluate human-made changes

… expressions

…xprEnchantmentOffer

…e applicable

@SirSmurfy2

thanks @SirSmurfy2! Co-authored-by: SirSmurfy2 <82696841+theabsolutionism@users.noreply.github.com>

…tments Co-authored-by: SirSmurfy2 <82696841+Absolutionism@users.noreply.github.com>

…edEnchantments" This reverts commit decd611, which was an incorrectly applied suggestion in GitHub UI.

APickledWalrus

Great work so far! Might be worth including https://jd.papermc.io/paper/26.1.2/org/bukkit/event/enchantment/EnchantItemEvent.html#getEnchantmentHint() which I don't think is covered yet.

APickledWalrus · 2026-06-11T15:56:17Z

+		registry.register(SyntaxRegistry.CONDITION, PropertyCondition
+			.infoBuilder(CondItemEnchantmentGlint.class, PropertyType.HAVE, "enchantment glint (override:overrid(den|e))", "itemtypes")
+			.addPatterns(getPatterns(PropertyType.BE, "forced to [:not] glint", "itemtypes"))
+			.supplier(CondItemEnchantmentGlint::new).build());


build calls should go on a new line 🙂

APickledWalrus · 2026-06-11T15:57:19Z

 	""")
-@RequiredPlugins("Spigot 1.20.5+")
 @Since("2.10")
 public class CondItemEnchantmentGlint extends PropertyCondition<ItemType> {


Missing override of getPropertyType

APickledWalrus · 2026-06-11T15:59:41Z

-		glint = !parseResult.hasTag("not");
-		return super.init(expressions, matchedPattern, isDelayed, parseResult);
+		if (!super.init(expressions, matchedPattern, isDelayed, parseResult)) return false;
+		override = parseResult.hasTag("override");


I think the parse tags is kind of confusing behavior. Since this is a single syntax info now, you should be able to just check that matchedPattern is 0 or 1

APickledWalrus · 2026-06-11T16:30:38Z

 		if (mode == ChangeMode.REMOVE || mode == ChangeMode.REMOVE_ALL || mode == ChangeMode.RESET)
 			return null;
-		return CollectionUtils.array(Number.class, Experience.class);


Why the removal of Experience? Is this functionality preserved in a different way?

Yes, via the converter

I see a Comparator but not a Converter. Is it located outside of the module?

Yeah, it's at https://github.com/SkriptLang/Skript/blob/master/src%2Fmain%2Fjava%2Fch%2Fnjol%2Fskript%2Fclasses%2Fdata%2FDefaultConverters.java#L84

I saw some other classes still had their converters registered here rather than in the relevant module, should I move the registration to the module?

Since it isn't exclusively related to enchanting, you can keep it where it is for now.

APickledWalrus · 2026-06-11T16:35:08Z

+		set the level of sharpness of the player's tool to {_maximum}
+	""")
+@Since("INSERT VERSION")
+public class ExprMaximumEnchantmentLevel extends SimplePropertyExpression<Enchantment, Integer> {


I might combine this with ExprMinimumEnchantmentLevel

I thought about that, but wasn't sure what to call it... ExprExtremumEnchantmentLevel? ExprMinMaxEnchantmentLevel? It feels like if I were looking for the class later for minimum or maximum enchantment level, two separate classes would be easier to find

I think ExprMinMaxEnchantmentLevel is fine and fits with other cases. Can always use the docs to find the relevant implementation class too 😉

APickledWalrus · 2026-06-11T16:44:37Z

 			new TagModule(this),
-			new TextModule(this)
+			new TextModule(this),
+			new EnchantmentModule(this)


Should be ordered alphabetically in the list

… Number

…and ExprEnchantingExpCost

…antments

…ions

…dItemEnchantmentGlint

bluelhf added 4 commits May 28, 2026 23:09

refactor(syntax): migrate enchantments to module-based system

13d7af5

feat(enchantments): add min level, max level, and stored enchantments…

d021d94

… expressions

fix(codestyle): remove duplicate code and assert non-nullability in E…

0f8b9ca

…xprEnchantmentOffer

style(enchantment): address code style issues and use converters wher…

6fbe3ff

…e applicable

bluelhf requested a review from a team as a code owner May 28, 2026 20:43

bluelhf requested review from Pesekjak and TheMug06 and removed request for a team May 28, 2026 20:43

Absolutionism requested changes May 28, 2026

View reviewed changes

bluelhf and others added 4 commits May 29, 2026 00:56

style: apply suggestions from code review

a8e34ff

thanks @SirSmurfy2! Co-authored-by: SirSmurfy2 <82696841+theabsolutionism@users.noreply.github.com>

fix(enchantment): fix enchantment glint override logic

555fb4c

style(enchantments): newline before closing brace in ExprStoredEnchan…

decd611

…tments Co-authored-by: SirSmurfy2 <82696841+Absolutionism@users.noreply.github.com>

Revert "style(enchantments): newline before closing brace in ExprStor…

23c9750

…edEnchantments" This reverts commit decd611, which was an incorrectly applied suggestion in GitHub UI.

skriptlang-automation Bot added the needs reviews A PR that needs additional reviews label May 29, 2026

Absolutionism requested changes May 30, 2026

View reviewed changes

bluelhf added 4 commits June 3, 2026 00:02

chore(enchantments): add explicit syntax instance suppliers

68cb97a

style: add blank lines and fix annotations

3fa0fc4

fix(classes): allow 'enchantmenttype' user input

faba4ad

style(tests): replace spaces with tabs

28026d3

bluelhf requested a review from Absolutionism June 2, 2026 21:07

APickledWalrus requested changes Jun 11, 2026

View reviewed changes

skriptlang-automation Bot added needs reviews A PR that needs additional reviews and removed needs reviews A PR that needs additional reviews labels Jun 11, 2026

bluelhf added 7 commits June 12, 2026 22:14

chore(math): add more saturating arithmetic tools to math2

7138a8b

chore(test): move enchantment tests to enchantments module

bb0caeb

refactor(enchantments): merge min and max enchantment level expressions

2c0efd1

style(enchantments): apply code style to registrations

3d80def

fix(enchantment-offer): add bounds checks and use Integer rather than…

5745638

… Number

fix(enchantment-offer): correct pattern

363b06c

fix(enchantment-offer): compute enchantment cost correctly

cd4d41a

bluelhf added 7 commits June 12, 2026 22:23

fix(enchantments): use saturating arithmetic in ExprEnchantmentLevel …

d452bb2

…and ExprEnchantingExpCost

fix(enchantments): fix typo in enchant item

88d8429

fix(enchantments): improve legacy handling of ExprEnchantItem changer

b852974

fix(enchantments): add article to ExprAppliedEnchantments#toString

54ecd1f

fix(enchantments): use pattern matching instanceof in ExprAppliedEnch…

5ee0deb

…antments

feat(enchantments): add ExprEnchantmentHint and alphabetise registrat…

33e6859

…ions

fix(enchantments): use index-based detection for override type in Con…

150ffbe

…dItemEnchantmentGlint

bluelhf requested a review from APickledWalrus June 12, 2026 19:45

Merge branch 'dev/feature' into feature/enchantments-module

b7ccc61

Uh oh!

Conversation

bluelhf commented May 28, 2026

Problem

Solution

Testing Completed

Supporting Information

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

APickledWalrus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants