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.

Sign up for GitHub

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

Data Values Enhancement #2750

Closed
wants to merge 6 commits into from
Closed

Data Values Enhancement #2750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f6138af
Merge pull request #13 from SkriptLang/master
Whimsyturtle Jan 8, 2020
610dacf
Update BlockValues.java
Whimsyturtle Jan 8, 2020
38b577e
Update MagicBlockCompat.java
Whimsyturtle Jan 8, 2020
7a0da0d
Update NewBlockCompat.java
Whimsyturtle Jan 8, 2020
00293a5
Update ExprDurability.java
Whimsyturtle Jan 8, 2020
2169262
Format Fixes
Whimsyturtle Jan 9, 2020
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
4 changes: 3 additions & 1 deletion src/main/java/ch/njol/skript/bukkitutil/block/BlockValues.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,6 @@ public abstract class BlockValues {
@Override
public abstract int hashCode();

}
public abstract Object getData();

}
7 changes: 6 additions & 1 deletion src/main/java/ch/njol/skript/bukkitutil/block/MagicBlockCompat.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public class MagicBlockCompat implements BlockCompat {
private class MagicBlockValues extends BlockValues {

private Material id;
short data;
private short data;
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this cause synthetic accessors to be generated by compiler? They're somewhat problematic before Java 11 (nestmates JVM feature).

Copy link
Member Author

Choose a reason for hiding this comment

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

The itemFlags variable below is also a private primitive... is that also an issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not related, or specific to primitives. It's a global thing for nested classes and private members.

If you are using Eclipse, it will show a warning when a synthetic accessor is needed to access a field. You can also enable a similar warning in the IntelliJ, from File -> Settings -> Inspections, search for "synthetic"

All fields should be package-private (no additional visibility prefix, directly type, name and obviously the semicolon) or have getters/setters and accessed by getters/setters in nested (sub) classes to not generate a synthetic accessor method.

More on synthetic access: https://stackoverflow.com/a/16226833/12308576

private int itemFlags;

@SuppressWarnings("null")
Expand Down Expand Up @@ -133,6 +133,11 @@ public MatchQuality match(BlockValues other) {
return MatchQuality.DIFFERENT;
}
}

@Override
public Object getData() {
return data;
}
}

private static class MagicBlockSetter implements BlockSetter {
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/ch/njol/skript/bukkitutil/block/NewBlockCompat.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ public MatchQuality match(BlockValues other) {
}
}

@Override
public Object getData() {
return data;
}

}

private static class NewBlockSetter implements BlockSetter {
Expand Down
115 changes: 71 additions & 44 deletions src/main/java/ch/njol/skript/expressions/ExprDurability.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,20 @@
*/
package ch.njol.skript.expressions;

import org.bukkit.block.data.BlockData;
import org.bukkit.event.Event;
import org.bukkit.inventory.ItemStack;
import org.eclipse.jdt.annotation.Nullable;

import ch.njol.skript.Skript;
import ch.njol.skript.aliases.ItemType;
import ch.njol.skript.bukkitutil.block.BlockValues;
import ch.njol.skript.classes.Changer.ChangeMode;
import ch.njol.skript.classes.Changer.ChangerUtils;
import ch.njol.skript.doc.Description;
import ch.njol.skript.doc.Examples;
import ch.njol.skript.doc.Name;
import ch.njol.skript.doc.Since;
import ch.njol.skript.expressions.base.SimplePropertyExpression;
import ch.njol.skript.util.slot.Slot;
import ch.njol.util.coll.CollectionUtils;

/**
Expand All @@ -43,21 +44,25 @@
"but this expression can e.g. be used to \"add 1 to data of <item>\", e.g. for cycling through all wool colours."})
@Examples({"add 1 to the data value of the clicked block"})
@Since("1.2")
public class ExprDurability extends SimplePropertyExpression<Object, Short> {
public class ExprDurability extends SimplePropertyExpression<ItemType, Object> {

static {
register(ExprDurability.class, Short.class, "((data|damage)[s] [value[s]]|durabilit(y|ies))", "itemstacks/slots");
register(ExprDurability.class, Object.class, "((data|damage)[s] [value[s]]|durabilit(y|ies))", "itemtypes");
}

private final static boolean USING_NEW_BLOCK_COMPAT = Skript.isRunningMinecraft(1, 13);

@SuppressWarnings("deprecation")
@Override
@Nullable
public Short convert(final Object o) {
if (o instanceof Slot) {
final ItemStack i = ((Slot) o).getItem();
return i == null ? null : i.getDurability();
} else {
return ((ItemStack) o).getDurability();
public Object convert(final ItemType i) {
if (i.hasBlock()) {
BlockValues bv = i.getTypes().get(0).getBlockValues();
if (bv == null) return null;
return USING_NEW_BLOCK_COMPAT ? ((BlockData) bv.getData()).getAsString() : bv.getData();
}
ItemStack stack = i.getRandom();
return stack == null ? null : stack.getDurability();
}

@Override
Expand All @@ -66,53 +71,75 @@ public String getPropertyName() {
}

@Override
public Class<Short> getReturnType() {
return Short.class;
public Class<Object> getReturnType() {
Copy link
Member

Choose a reason for hiding this comment

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

making an expression return an object isn't as simple as this sadly, take a look at the ternary expression

Copy link
Member

Choose a reason for hiding this comment

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

you aren't actually accepting Object.class, you're accepting a Number or a String

return Object.class;
}

@Override
@Nullable
public Class<?>[] acceptChange(final ChangeMode mode) {
if (mode == ChangeMode.REMOVE_ALL)
return null;
if (Slot.class.isAssignableFrom(getExpr().getReturnType()) || getExpr().isSingle() && ChangerUtils.acceptsChange(getExpr(), ChangeMode.SET, ItemStack.class, ItemType.class))
return CollectionUtils.array(Number.class);
return null;
return CollectionUtils.array(Object.class);
}

@SuppressWarnings("deprecation")
@Override
public void change(final Event e, final @Nullable Object[] delta, final ChangeMode mode) {
Copy link
Member

Choose a reason for hiding this comment

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

i feel like this whole method is really confusingly written (particularly how the recursion is involved)

int a = delta == null ? 0 : ((Number) delta[0]).intValue();
final Object[] os = getExpr().getArray(e);
for (final Object o : os) {
final ItemStack i = o instanceof Slot ? ((Slot) o).getItem() : (ItemStack) o;
if (i == null)
continue;
switch (mode) {
case REMOVE:
a = -a;
//$FALL-THROUGH$
case ADD:
i.setDurability((short) (i.getDurability() + a));
break;
case SET:
i.setDurability((short) a);
break;
case DELETE:
case RESET:
a = 0;
i.setDurability((short) 0);
break;
case REMOVE_ALL:
assert false;
if (delta == null) return;
Object[] arr = getExpr().getArray(e);
ItemType[] newarr = new ItemType[arr.length];
if (delta[0] instanceof String && USING_NEW_BLOCK_COMPAT) {
String a = (String) delta[0];
assert a != null;
for (int i = 0; i < arr.length; i++) {
ItemType item = (ItemType) arr[i];
ItemType newitem;
switch (mode) {
case SET:
newitem = new ItemType(item.getMaterial(), a);
newitem.setAmount(item.getAmount());
break;
case DELETE:
case RESET:
newitem = new ItemType(item.getMaterial());
newitem.setAmount(item.getAmount());
break;
case REMOVE_ALL:
assert false;
default:
newitem = item;
}
newarr[i] = newitem;
}
if (o instanceof Slot)
((Slot) o).setItem(i);
else if (ChangerUtils.acceptsChange(getExpr(), ChangeMode.SET, ItemStack.class))
getExpr().change(e, new ItemStack[] {i}, ChangeMode.SET);
else
getExpr().change(e, new ItemType[] {new ItemType(i)}, ChangeMode.SET);
} else if (delta[0] instanceof Number) {
int a = ((Number) delta[0]).intValue();
for (int i = 0; i < arr.length; i++) {
ItemStack stack = ((ItemType) arr[i]).getRandom();
assert stack != null;
switch (mode) {
case REMOVE:
stack.setDurability((short) (stack.getDurability() - a));
break;
case ADD:
stack.setDurability((short) (stack.getDurability() + a));
break;
case SET:
stack.setDurability((short) a);
break;
case DELETE:
case RESET:
stack.setDurability((short) 0);
break;
case REMOVE_ALL:
assert false;
}
newarr[i] = new ItemType(stack);
}
} else {
return;
}
change(e, newarr, ChangeMode.SET);
}

}