Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ExprDurability's Changer #6154

Merged
merged 4 commits into from
Nov 1, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 54 additions & 52 deletions src/main/java/ch/njol/skript/expressions/ExprDurability.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@
import ch.njol.skript.lang.Expression;
import ch.njol.skript.lang.SkriptParser.ParseResult;
import ch.njol.util.Kleenean;
import org.bukkit.Material;
import org.bukkit.event.Event;
import org.bukkit.inventory.ItemStack;
import org.bukkit.inventory.meta.Damageable;
import org.bukkit.inventory.meta.ItemMeta;
import org.eclipse.jdt.annotation.Nullable;

import ch.njol.skript.aliases.ItemType;
import ch.njol.skript.bukkitutil.ItemUtils;
import ch.njol.skript.classes.Changer.ChangeMode;
import ch.njol.skript.doc.Description;
import ch.njol.skript.doc.Examples;
Expand All @@ -44,33 +46,28 @@
"set durability of player's held item to 0"
})
@Since("1.2, 2.7 (durability reversed)")
public class ExprDurability extends SimplePropertyExpression<Object, Long> {
public class ExprDurability extends SimplePropertyExpression<Object, Integer> {

private boolean durability;

static {
register(ExprDurability.class, Long.class, "(damage[s] [value[s]]|durability:durabilit(y|ies))", "itemtypes/slots");
register(ExprDurability.class, Integer.class, "(damage[s] [value[s]]|1:durabilit(y|ies))", "itemtypes/slots");
}

@Override
public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelayed, ParseResult parseResult) {
durability = parseResult.hasTag("durability");
durability = parseResult.mark == 1;
return super.init(exprs, matchedPattern, isDelayed, parseResult);
}

@Override
@Nullable
public Long convert(Object object) {
ItemStack itemStack = null;
if (object instanceof Slot) {
itemStack = ((Slot) object).getItem();
} else if (object instanceof ItemType) {
itemStack = ((ItemType) object).getRandom();
}
if (itemStack == null)
public Integer convert(Object object) {
ItemType itemType = asItemType(object);
if (itemType == null)
return null;
long damage = ItemUtils.getDamage(itemStack);
return durability ? itemStack.getType().getMaxDurability() - damage : damage;
ItemMeta meta = itemType.getItemMeta();
return meta instanceof Damageable ? translate(itemType.getMaterial(), ((Damageable) meta).getDamage()) : 0;
}

@Override
Expand All @@ -89,62 +86,67 @@ public Class<?>[] acceptChange(ChangeMode mode) {

@Override
public void change(Event event, @Nullable Object[] delta, ChangeMode mode) {
int i = delta == null ? 0 : ((Number) delta[0]).intValue();
Object[] objects = getExpr().getArray(event);
for (Object object : objects) {
ItemStack itemStack = null;

if (object instanceof ItemType) {
itemStack = ((ItemType) object).getRandom();
} else if (object instanceof Slot) {
itemStack = ((Slot) object).getItem();
}
if (itemStack == null)
return;

int changeValue = ItemUtils.getDamage(itemStack);
if (durability)
changeValue = itemStack.getType().getMaxDurability() - changeValue;

int change = delta == null ? 0 : ((Number) delta[0]).intValue();
if (mode == ChangeMode.REMOVE)
change = -change;
for (Object object : getExpr().getArray(event)) {
ItemType itemType = asItemType(object);
if (itemType == null)
continue;

ItemMeta meta = itemType.getItemMeta();
if (!(meta instanceof Damageable))
continue;
Damageable damageable = (Damageable) meta;

Material material = itemType.getMaterial();
switch (mode) {
case REMOVE:
i = -i;
//$FALL-THROUGH$
case ADD:
changeValue += i;
case REMOVE:
int current = translate(material, damageable.getDamage());
damageable.setDamage(translate(material, current + change));
break;
case SET:
changeValue = i;
damageable.setDamage(translate(material, change));
break;
case DELETE:
case RESET:
changeValue = 0;
break;
case REMOVE_ALL:
assert false;
damageable.setDamage(0);
}

if (durability && mode != ChangeMode.RESET && mode != ChangeMode.DELETE)
changeValue = itemStack.getType().getMaxDurability() - changeValue;

if (object instanceof ItemType) {
ItemUtils.setDamage(itemStack, changeValue);
((ItemType) object).setTo(new ItemType(itemStack));
} else {
ItemUtils.setDamage(itemStack, changeValue);
((Slot) object).setItem(itemStack);
}
itemType.setItemMeta(meta);
if (object instanceof Slot)
((Slot) object).setItem(itemType.getRandom());
}
}

private int translate(Material material, int value) {
Copy link
Member

@sovdeeth sovdeeth Oct 30, 2023

Choose a reason for hiding this comment

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

please name this better or give it some comments
it's very hard to know what it's supposed to do without reading the code inside it

if (!durability)
return value;
int maxDurability = material.getMaxDurability();
if (maxDurability == 0)
return 0;
return maxDurability - value;
Comment on lines +126 to +129
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
int maxDurability = material.getMaxDurability();
if (maxDurability == 0)
return 0;
return maxDurability - value;
return Math.max(material.getMaxDurability() - value, 0);

Would be nice :) (unless negative values are intended?)

Copy link
Member

Choose a reason for hiding this comment

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

I would rather have the ability to go negative if I so desired than to be locked out of it and resort to reflect

Copy link
Member

Choose a reason for hiding this comment

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

I am not actually sure if a negative durability value is valid. If they are, then this does not need to be done.

Copy link
Member

Choose a reason for hiding this comment

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

You can certainly have an item with negative durability, it just looks stupid in game
I tested it out when I reviewed this

}

@Override
public Class<? extends Long> getReturnType() {
return Long.class;
public Class<? extends Integer> getReturnType() {
return Integer.class;
}

@Override
public String getPropertyName() {
return durability ? "durability" : "damage";
}

@Nullable
private static ItemType asItemType(Object object) {
if (object instanceof ItemType)
return (ItemType) object;
ItemStack itemStack = ((Slot) object).getItem();
if (itemStack == null)
return null;
return new ItemType(itemStack);
}

}