-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add block/meta and item/meta pair classes #98
base: master
Are you sure you want to change the base?
Conversation
src/main/java/com/gtnewhorizon/gtnhlib/eventbus/AutoEventBus.java
Outdated
Show resolved
Hide resolved
src/main/java/com/gtnewhorizon/gtnhlib/util/data/BlockMeta.java
Outdated
Show resolved
Hide resolved
/** | ||
* A mutable implementation of {@link ImmutableBlockMeta}. If your API should return a mutable pair, return this | ||
* instead. Must follow the same contracts as the immutable version if this is ever upcast to a | ||
* {@link ImmutableBlockMeta} in your API. If this type is exposed instead of the immutable interface, assume that the | ||
* contained values can change. | ||
*/ | ||
public class BlockMeta implements ImmutableBlockMeta { |
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.
Could you offer an example use case where we'd want to have a BlockMeta instance which can be mutated outside the control of the code that receives it? Worse, there's nothing stopping anyone from returning a mutable BlockMeta
when a ImmutableBlockMeta
is required so you can actually never guarantee the immutability of the data you receive. That sounds like a recipe for bugs.
It seems to me we'd be much better of and with a cleaner API with only immutable implementations.
If you must have both, they probably shouldn't share an interface.
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.
Ideally there'd be a way to statically check the mutability, but since we're writing java and not rust I had to rely on this system. I can add an immutable implementation if that'd be better, but I didn't think it was necessary. The only benefit that would provide is to prevent casting down to BlockMeta, which people shouldn't do unless they have a very good reason (in which case, they should be able to mutate it).
Mutability is useful for a pattern similar to pointers. It's mainly a way of reducing allocations in hot-paths. 99% of the time you'll want to return or accept an immutable pair unless you have a good reason not to, since that's the more generic type.
ImmutableBlockMeta getSurfaceBlock(World world, int x, int z) {
BlockMeta pooled = new BlockMeta(Blocks.air, 0);
for (int y = 255; y >= 0; y--) {
getBlock(pooled, world, x, y, z);
if (pooled.getBlock().getMaterial() = Materials.rock || pooled.getBlock().getMaterial() == Materials.ground) {
pooled.setBlockMeta(world.getBlockMetadata(x, y, z));
return pooled;
}
}
return null;
}
void getBlock(BlockMeta bm, World world, int x, int y, int z) {
// as an example, this would need to be much more complex
bm.setBlock(world.getBlock(x, y, z));
}
Warning: 2 uncommitted changes |
This PR adds a few util classes that I've needed several times in different mods. I've also seen similar classes replicated a good amount of times, so I figure these should be formalized properly.
If the mutability contract isn't clear, let me know. I tried to make it as robust as possible.