add Codecs for FluidVariant and ResourceAmount<FluidVariant>
#3419
Conversation
|
I thought IDEA had automatically added imports needed when I copy-pasted these lines from a mod of mine, but apparently it didn't |
Please add some unit tests for this, make sure that the change works as expected. |
…d so tests `FluidVariant.CODEC`
| @@ -27,4 +30,9 @@ | |||
| */ | |||
| @ApiStatus.Experimental | |||
| public record ResourceAmount<T> (T resource, long amount) { | |||
| public static final Codec<ResourceAmount<FluidVariant>> FLUID_VARIANT_CODEC = RecordCodecBuilder.create(instance -> | |||
There was a problem hiding this comment.
This has no business being here, fluid-specific code should go in the fluid subpackage of the transfer API. This could probably go in FluidVariant.
| @@ -43,6 +50,12 @@ | |||
| @ApiStatus.Experimental | |||
| @ApiStatus.NonExtendable | |||
| public interface FluidVariant extends TransferVariant<Fluid> { | |||
| public static final Codec<FluidVariant> CODEC = RecordCodecBuilder.create(instance -> | |||
There was a problem hiding this comment.
This should probably a MapCodec in case some people want to use it inline with other fields?
| public static final Codec<FluidVariant> CODEC = RecordCodecBuilder.create(instance -> | ||
| instance.group( | ||
| Registries.FLUID.getCodec().fieldOf("fluid").forGetter(FluidVariant::getFluid), | ||
| NbtCompound.CODEC.optionalFieldOf("nbt").forGetter(variant -> Optional.ofNullable(variant.getNbt())) |
There was a problem hiding this comment.
If I remember correctly, the NBT parsing code might read small numbers as a different datatype than int, that should really be checked.
|
Moved the resource amount codec. This was enough for my use case and I'm not interested in putting more work into it. I'm also not familiar with data fixers beyond the common uses, so you'll have to find someone else for any codec shenanigans If the 81000 denominator for fluids is not user-friendly, why was it used in the first place? Is there a non-leaking abstraction for it now? |
It is used internally, but should not necessarily be exposed to the user. I think being able to input as floating point buckets or millibuckets is more user-friendly, and should be offered as an option. Not sure about the exact format, here are a few options: {
"amount": {"unit": "bucket", "amount": 0.1 }, // a bit verbose
"amount": "0.1 B", // parsing is a bit tricky but probably reasonable
"amount": "100 mB", // could also support mB with the same format
"amount": "8100 d", // maybe also droplets
}Personally I like the string with a mandatory unit suffix. I can add the code to your PR if you want. |
|
If doing fancy codecs, why not have one of "amount", "millibuckets" or "buckets". Basically – with variant also "flattened" – like this: {
"fluid": "minecraft:water",
"amount": 40500
},
{
"fluid": "minecraft:water",
"millibuckets": 500
},
{
"fluid": "minecraft:water",
"buckets": 0.5
}If hiding that fluids use denominator of 81000, why not hide the fact that there are some "fluid variants", which users might never have heard of? |
|
Yeah we should definitely hide the "variant" too, IMO. I think |
|
If there are codecs for |
|
we can start with the fluid codecs only IMO, that makes things simpler |
Adds
Codecfields for these two types, which might be useful for the new recipe serializers, which now useRecipeSerializer#codecfor reading from json, instead of receivingJsonObjectand reading it imperatively like it was before.