-
Notifications
You must be signed in to change notification settings - Fork 273
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
Implement block states for commands and /testforblock command #579
Changes from 8 commits
b6fce97
b4ef01b
33b4d95
1930008
bb6bf23
ef79641
9db1767
39f75c2
46c7bc5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package net.glowstone.block.state; | ||
|
||
import java.util.HashMap; | ||
|
||
public class BlockStateData extends HashMap<String, String> { | ||
@Override | ||
public String toString() { | ||
StringBuilder builder = new StringBuilder(); | ||
for (Entry<String, String> entry : entrySet()) { | ||
builder.append(",").append(entry.getKey()).append("=").append(entry.getValue()); | ||
} | ||
return builder.length() == 0 ? builder.toString() : builder.substring(1); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package net.glowstone.block.state; | ||
|
||
import org.bukkit.Material; | ||
import org.bukkit.material.MaterialData; | ||
|
||
import java.util.Set; | ||
|
||
public abstract class BlockStateReader<T extends MaterialData> { | ||
|
||
public abstract Set<String> getValidStates(); | ||
|
||
public abstract T read(Material material, BlockStateData data) throws InvalidBlockStateException; | ||
|
||
public abstract boolean matches(BlockStateData state, T data) throws InvalidBlockStateException; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like sticking this on the reader is a bit disjoint with how I implemented the Entity's tag matching. I'm not opposed to sticking it on the reader, but and if you want to keep it like this you should think about changing how the CompoundTag's matching works. Otherwise, I'd say you should implement the match method on the BlockStateData, similar to how CompoundTag implements its match method. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package net.glowstone.block.state; | ||
|
||
import net.glowstone.constants.ItemIds; | ||
import org.bukkit.Material; | ||
|
||
public class InvalidBlockStateException extends Exception { | ||
public InvalidBlockStateException(Material material, String state) { | ||
super("'" + state + "' is not a state for block " + ItemIds.getName(material)); | ||
} | ||
|
||
public InvalidBlockStateException(Material material, BlockStateData state) { | ||
super("'" + state.toString() + "' is not a state for block " + ItemIds.getName(material)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extending an implementation of Map<> seems like the wrong way to go here. In my experience, this makes it harder to refactor, especially if another implementation of map is needed, and can dilute the reason for having BlockStateData vs a regular map. It's convenient, yes, but often introduces a design smell. I usually prefer introducing the map as a field and exposing only the relevant delegate methods, making it clear what is going on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good points, I will change this.