🚀 Enchantment Improvements #4366

AyhamAl-Ali · 2021-10-15T16:57:55Z

Description

This PR does the following:

Added support for stored enchantments for enchanted book to CondIsEnchanted, ExprEnchantments and EffEnchant classes
Added new expression %itemtypes% (with|of) [stored] [enchant[ment[s]]] %enchantmenttypes%
Added disenchanting specific enchantments
Also fixes (dis)enchanting multiple itemtypes causes severe error due to not handling it at all
Added related methods to ItemType class
Added has conflicting enchantments condition
Changed behavior of ItemType#hasEnchantments level check to != instead of < (this should be strict and level expression should be the way to check with math) - (this caused this check if player's tool is enchanted with unbreaking 1 to be true when player's tool has unbreaking 2`) Let me know if you want this to be reverted

NOTE: I couldn't find a good syntax for the new changes so if anyone have a better syntax let me know.

Target Minecraft Versions: All
Requirements: None
Related Issues:

- Support added to CondIsEnchanted, ExprEnchantments and EffEnchant classes - Added new expr to return itemtypes with stored enchantments - Also fixes (dis)enchanting multiple itemtypes causes severe error due to not handling it at all - Added related methods to ItemType class - Changed behavior of "ItemType#hasEnchantments" level check to !- instead of < (this should be strict and level expression should be the way to check with math)

AyhamAl-Ali · 2021-10-15T17:12:38Z

Might be good to add #4245 and #1234 as well

This should return a single item so it can work with other item related expressions or upgrade other expressions to support %itemtypes% but that's not really needed

APickledWalrus

Some things I noticed on my first pass

Good feature PR :D

src/main/java/ch/njol/skript/aliases/ItemType.java

src/main/java/ch/njol/skript/conditions/CondHasConflictingEnchantments.java

src/main/java/ch/njol/skript/conditions/CondIsEnchanted.java

src/main/java/ch/njol/skript/expressions/ExprEnchantments.java

src/main/java/ch/njol/skript/expressions/ExprItemWithEnchantments.java

Co-authored-by: APickledWalrus <apickledwalrus@gmail.com>

…antments.java Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>

TheLimeGlass

Can tests be added to this? There were multiple issues relating to this. Some regression tests are needed.

src/main/java/ch/njol/skript/aliases/ItemType.java

src/main/java/ch/njol/skript/effects/EffEnchant.java

src/main/java/ch/njol/skript/expressions/ExprItemWithEnchantments.java

Co-authored-by: LimeGlass <16087552+TheLimeGlass@users.noreply.github.com>

…/stored-ench

APickledWalrus

looking forward to this feature 😎

APickledWalrus · 2024-05-29T18:16:12Z

src/main/java/ch/njol/skript/conditions/CondHasConflictingEnchantments.java

+public class CondHasConflictingEnchantments extends Condition {
+
+	static {
+		PropertyCondition.register(CondHasConflictingEnchantments.class, PropertyType.HAVE,


What do you think about a "can be enchanted with" condition? Do you think that reflects the property well?

APickledWalrus · 2024-05-29T18:17:42Z

src/main/java/ch/njol/skript/aliases/ItemType.java

+	 */
+	public EnchantmentType @Nullable [] getStoredEnchantmentTypes() {
+		EnchantmentStorageMeta meta = getEnchantmentStorageMeta();
+		if (meta == null)


should this also check !meta.hasStoredEnchants()?

APickledWalrus · 2024-05-29T18:17:56Z

src/main/java/ch/njol/skript/aliases/ItemType.java

+		if (meta == null)
+			return false;
+		if (!meta.hasStoredEnchants())
+			return false;


Suggested change

if (meta == null)

return false;

if (!meta.hasStoredEnchants())

return false;

if (meta == null || !meta.hasStoredEnchants())

return false;

APickledWalrus · 2024-05-29T18:19:37Z

src/main/java/ch/njol/skript/aliases/ItemType.java

+			assert type != null; // Bukkit working different from what we expect
+			if (!meta.hasStoredEnchant(type))
+				return false;
+			if (enchantment.getInternalLevel() != -1 && meta.getStoredEnchantLevel(type) != enchantment.getLevel())


-1 represents an enchantment type without a specific level (e.g. it has any level of X stored) right?

Maybe we should move this into a public field on EnchantmentType with docs. Or maybe better, a method like representsAnyLevel (we could come up with a better name lol)

APickledWalrus · 2024-05-29T18:47:19Z

src/main/java/ch/njol/skript/conditions/CondIsEnchanted.java

 public class CondIsEnchanted extends Condition {

 	static {
-		PropertyCondition.register(CondIsEnchanted.class, "enchanted [with %-enchantmenttype%]", "itemtypes");
+		Skript.registerCondition(CondIsEnchanted.class,


I wonder if we should support item has enchantment <enchantment>

APickledWalrus · 2024-05-29T19:14:33Z

src/main/java/ch/njol/skript/expressions/ExprItemWithEnchantments.java

+
+	static {
+		Skript.registerExpression(ExprItemWithEnchantments.class, ItemType.class, ExpressionType.COMBINED,
+				"%itemtype% (with|of) [:stored] [enchant[ment[s]]] %enchantmenttypes%"); // Added support for non-stored enchantments, check https://github.com/SkriptLang/Skript/issues/1836


Suggested change

"%itemtype% (with|of) [:stored] [enchant[ment[s]]] %enchantmenttypes%"); // Added support for non-stored enchantments, check https://github.com/SkriptLang/Skript/issues/1836

"%itemtype% (with|of) [:stored] [enchant[ment[s]]] %enchantmenttypes%");

I don't think it needs linked in this case

APickledWalrus · 2024-05-29T19:15:39Z

src/main/java/ch/njol/skript/expressions/ExprItemWithEnchantments.java

+			if (item.getEnchantmentStorageMeta() != null)
+				item.addStoredEnchantments(enchantments);


Suggested change

if (item.getEnchantmentStorageMeta() != null)

item.addStoredEnchantments(enchantments);

item.addStoredEnchantments(enchantments);

looks like the ItemType method should handle this internally

APickledWalrus · 2024-05-29T19:16:26Z

src/test/skript/tests/syntaxes/effects/EffEnchant.sk

+	assert enchantments of {_item} is not set with "enchanting effect failed test ##1"
+
+	enchant {_item} with sharpness 3
+	assert enchantment level of sharpnees on {_item} is 3 with "enchanting effect failed test ##2"


Suggested change

assert enchantment level of sharpnees on {_item} is 3 with "enchanting effect failed test ##2"

assert enchantment level of sharpness on {_item} is 3 with "enchanting effect failed test ##2"

APickledWalrus · 2024-05-29T19:16:38Z

src/test/skript/tests/syntaxes/effects/EffEnchant.sk

+	assert enchantment level of sharpnees on {_item} is 3 with "enchanting effect failed test ##2"
+
+	enchant {_item} with sharpness 2
+	assert enchantment level of sharpnees on {_item} is 2 with "enchanting effect failed test ##2"


Suggested change

assert enchantment level of sharpnees on {_item} is 2 with "enchanting effect failed test ##2"

assert enchantment level of sharpness on {_item} is 2 with "enchanting effect failed test ##2"

APickledWalrus · 2024-05-29T19:24:39Z

src/main/java/ch/njol/skript/expressions/ExprItemWithEnchantments.java

+
+	static {
+		Skript.registerExpression(ExprItemWithEnchantments.class, ItemType.class, ExpressionType.COMBINED,
+				"%itemtype% (with|of) [:stored] [enchant[ment[s]]] %enchantmenttypes%"); // Added support for non-stored enchantments, check https://github.com/SkriptLang/Skript/issues/1836


thoughts on a pattern instead like %itemtype% (with|of) [enchant[ment[s]]] %enchantmenttypes% [:stored] as in dirt with sharpness stored

TheLimeGlass · 2024-09-19T04:17:33Z

src/main/java/ch/njol/skript/conditions/CondHasConflictingEnchantments.java

+			if ((isStored && meta instanceof EnchantmentStorageMeta)) {
+				return ((EnchantmentStorageMeta) meta).hasConflictingStoredEnchant(ench);


Suggested change

if ((isStored && meta instanceof EnchantmentStorageMeta)) {

return ((EnchantmentStorageMeta) meta).hasConflictingStoredEnchant(ench);

if (isStored && meta instanceof EnchantmentStorageMeta enchantmentStorageMeta) {

return enchantmentStorageMeta.hasConflictingStoredEnchant(ench);

TheLimeGlass · 2024-09-19T04:18:16Z

src/main/java/ch/njol/skript/conditions/CondHasConflictingEnchantments.java

+	@Override
+	public String toString(@Nullable Event event, boolean debug) {
+		return PropertyCondition.toString(this, PropertyType.HAVE, event, debug, items,
+			"conflicting " + (isStored ? "stored " : "") + "enchantments with " + ench.toString(event, debug));


Suggested change

"conflicting " + (isStored ? "stored " : "") + "enchantments with " + ench.toString(event, debug));

"conflicting " + (isStored ? "stored " : "") + "enchantments with " + ench.toString(event, debug));

TheLimeGlass · 2024-09-19T04:20:13Z

src/main/java/ch/njol/skript/expressions/ExprEnchantments.java

-	public String toString(@Nullable Event e, boolean debug) {
-		return "the enchantments of " + items.toString(e, debug);
+	public String toString(@Nullable Event event, boolean debug) {
+		return "the " + (isStored ? "stored " : "") + "enchantments of " + items.toString(event, debug);


Suggested change

return "the " + (isStored ? "stored " : "") + "enchantments of " + items.toString(event, debug);

return (isStored ? "stored " : "") + "enchantments of " + items.toString(event, debug);

sovdeeth · 2024-12-15T03:34:07Z

Closing due to inactivity. Re-open if you're interested in taking it over, anyone can!

AyhamAl-Ali added 2 commits October 15, 2021 19:45

Fix SkriptLang#1836

bc52165

Add has conflicting enchantments condition (SkriptLang#4245)

4564525

TPGamesNL added enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature Pull request adding a new feature. labels Oct 16, 2021

AyhamAl-Ali added 5 commits October 19, 2021 14:28

Added null checker

782ce15

Fix return type of ItemWithEnchantments Expr

e8b83b3

This should return a single item so it can work with other item related expressions or upgrade other expressions to support %itemtypes% but that's not really needed

Merge branch 'master' into ench/stored-ench

9189377

Merge branch 'master' into ench/stored-ench

029720e

Add disenchant specific enchantments

11e0e8e

AyhamAl-Ali mentioned this pull request Jan 27, 2022

is enchanted condition docs update for level #4517

Open

1 task

Merge branch 'master' into ench/stored-ench

c6b7cf9

APickledWalrus requested changes Mar 5, 2022

View reviewed changes

AyhamAl-Ali and others added 2 commits March 5, 2022 16:12

Apply suggestions from code review

de7e9dd

Co-authored-by: APickledWalrus <apickledwalrus@gmail.com>

Merge branch 'master' into ench/stored-ench

e616818

AyhamAl-Ali changed the title ~~🚀 Support stored enchantments~~ 🚀 Enchantment Improvements Mar 12, 2022

Update src/main/java/ch/njol/skript/conditions/CondHasConflictingEnch…

eaf7f57

…antments.java Co-authored-by: Patrick Miller <apickledwalrus@gmail.com>

Fusezion mentioned this pull request Mar 17, 2023

[BUG] Error when enchanting multiple entities' tools #5530

Closed

1 task

Requested changes - Improvements

cfe81d2

AyhamAl-Ali requested a review from APickledWalrus March 17, 2023 12:34

TheLimeGlass added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Mar 17, 2023

EffEnchant pattern improvement

429a6df

TheLimeGlass suggested changes Mar 17, 2023

View reviewed changes

AyhamAl-Ali and others added 5 commits March 17, 2023 16:18

Apply suggestions from code review

8643058

Co-authored-by: LimeGlass <16087552+TheLimeGlass@users.noreply.github.com>

Update src/main/java/ch/njol/skript/aliases/ItemType.java

48f75ba

Co-authored-by: LimeGlass <16087552+TheLimeGlass@users.noreply.github.com>

Requested changes

2398bde

Merge remote-tracking branch 'AyhamAl-Ali/ench/stored-ench' into ench…

eddd8b6

…/stored-ench

Indentations

7078f60

AyhamAl-Ali marked this pull request as draft April 22, 2023 00:04

AyhamAl-Ali mentioned this pull request Aug 20, 2023

⚒ Fix enchanting multiple items #5927

Merged

Moderocky force-pushed the master branch 2 times, most recently from bd134d0 to 3f08853 Compare September 16, 2023 16:59

AyhamAl-Ali mentioned this pull request Apr 2, 2024

ItemType with enchantments expression #6522

Closed

1 task

AyhamAl-Ali changed the base branch from master to dev/feature April 5, 2024 14:50

AyhamAl-Ali added 2 commits April 5, 2024 18:46

Fix EffEnchant pattern

a139e22

Update toString

1cc0840

AyhamAl-Ali marked this pull request as ready for review April 5, 2024 16:01

Merge remote-tracking branch 'origin/dev/feature' into ench/stored-ench

d0e6451

AyhamAl-Ali requested a review from TheLimeGlass April 5, 2024 17:20

AyhamAl-Ali added 4 commits April 5, 2024 20:23

add useful comments

fd4c257

fix building

e52049e

Updated submodule skript-aliases

0560dc1

Add tests

581813e

APickledWalrus requested changes May 29, 2024

View reviewed changes

Moderocky approved these changes Aug 17, 2024

View reviewed changes

TheLimeGlass approved these changes Sep 19, 2024

View reviewed changes

sovdeeth closed this Dec 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🚀 Enchantment Improvements #4366

🚀 Enchantment Improvements #4366

AyhamAl-Ali commented Oct 15, 2021 •

edited

Loading

AyhamAl-Ali commented Oct 15, 2021

APickledWalrus left a comment

TheLimeGlass left a comment

APickledWalrus left a comment

APickledWalrus May 29, 2024

APickledWalrus May 29, 2024

APickledWalrus May 29, 2024

APickledWalrus May 29, 2024

APickledWalrus May 29, 2024

APickledWalrus May 29, 2024

APickledWalrus May 29, 2024

APickledWalrus May 29, 2024

APickledWalrus May 29, 2024

APickledWalrus May 29, 2024

TheLimeGlass Sep 19, 2024

TheLimeGlass Sep 19, 2024

TheLimeGlass Sep 19, 2024

sovdeeth commented Dec 15, 2024

	"%itemtype% (with\|of) [:stored] [enchant[ment[s]]] %enchantmenttypes%"); // Added support for non-stored enchantments, check https://github.com/SkriptLang/Skript/issues/1836
	"%itemtype% (with\|of) [:stored] [enchant[ment[s]]] %enchantmenttypes%");

		if (item.getEnchantmentStorageMeta() != null)
		item.addStoredEnchantments(enchantments);

	assert enchantment level of sharpnees on {_item} is 3 with "enchanting effect failed test ##2"
	assert enchantment level of sharpness on {_item} is 3 with "enchanting effect failed test ##2"

		if ((isStored && meta instanceof EnchantmentStorageMeta)) {
		return ((EnchantmentStorageMeta) meta).hasConflictingStoredEnchant(ench);

	"conflicting " + (isStored ? "stored " : "") + "enchantments with " + ench.toString(event, debug));
	"conflicting " + (isStored ? "stored " : "") + "enchantments with " + ench.toString(event, debug));

	return "the " + (isStored ? "stored " : "") + "enchantments of " + items.toString(event, debug);
	return (isStored ? "stored " : "") + "enchantments of " + items.toString(event, debug);

🚀 Enchantment Improvements #4366

🚀 Enchantment Improvements #4366

Conversation

AyhamAl-Ali commented Oct 15, 2021 • edited Loading

Description

AyhamAl-Ali commented Oct 15, 2021

APickledWalrus left a comment

Choose a reason for hiding this comment

TheLimeGlass left a comment

Choose a reason for hiding this comment

APickledWalrus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sovdeeth commented Dec 15, 2024

AyhamAl-Ali commented Oct 15, 2021 •

edited

Loading