Small improvements to packet reading/writing improvements

- Prefer {read,write}Nullable when possible.

 - Use SoundEvent.{writeTo,readFrom}Network, instead of sending the
   registry entries. This allows playing discs which don't register
   their SoundEvent on the server.

 - Add a couple of tests for round-tripping these packets.
This commit is contained in:
Jonathan Coates 2023-08-23 09:49:49 +01:00
parent 5b2fdec6ca
commit c1628d077a
No known key found for this signature in database
GPG Key ID: B9E431FF07C98D06
5 changed files with 146 additions and 20 deletions

View File

@ -6,13 +6,11 @@
import dan200.computercraft.shared.network.NetworkMessage;
import dan200.computercraft.shared.peripheral.diskdrive.DiskDriveBlockEntity;
import dan200.computercraft.shared.platform.RegistryWrappers;
import net.minecraft.core.BlockPos;
import net.minecraft.network.FriendlyByteBuf;
import net.minecraft.sounds.SoundEvent;
import javax.annotation.Nullable;
import java.util.function.BiConsumer;
/**
* Starts or stops a record on the client, depending on if {@link #soundEvent} is {@code null}.
@ -40,28 +38,19 @@ public PlayRecordClientMessage(BlockPos pos) {
public PlayRecordClientMessage(FriendlyByteBuf buf) {
pos = buf.readBlockPos();
soundEvent = buf.readBoolean() ? RegistryWrappers.readKey(buf, RegistryWrappers.SOUND_EVENTS) : null;
name = buf.readBoolean() ? buf.readUtf(Short.MAX_VALUE) : null;
soundEvent = buf.readNullable(SoundEvent::readFromNetwork);
name = buf.readNullable(FriendlyByteBuf::readUtf);
}
@Override
public void toBytes(FriendlyByteBuf buf) {
buf.writeBlockPos(pos);
writeOptional(buf, soundEvent, (b, e) -> RegistryWrappers.writeKey(b, RegistryWrappers.SOUND_EVENTS, e));
writeOptional(buf, name, FriendlyByteBuf::writeUtf);
buf.writeNullable(soundEvent, (b, e) -> e.writeToNetwork(b));
buf.writeNullable(name, FriendlyByteBuf::writeUtf);
}
@Override
public void handle(ClientNetworkContext context) {
context.handlePlayRecord(pos, soundEvent, name);
}
private static <T> void writeOptional(FriendlyByteBuf out, @Nullable T object, BiConsumer<FriendlyByteBuf, T> write) {
if (object == null) {
out.writeBoolean(false);
} else {
out.writeBoolean(true);
write.accept(out, object);
}
}
}

View File

@ -10,7 +10,6 @@
import net.minecraft.core.registries.Registries;
import net.minecraft.network.FriendlyByteBuf;
import net.minecraft.resources.ResourceLocation;
import net.minecraft.sounds.SoundEvent;
import net.minecraft.world.inventory.MenuType;
import net.minecraft.world.item.Item;
import net.minecraft.world.item.crafting.RecipeSerializer;
@ -33,7 +32,6 @@ public final class RegistryWrappers {
public static final RegistryWrapper<Fluid> FLUIDS = PlatformHelper.get().wrap(Registries.FLUID);
public static final RegistryWrapper<Enchantment> ENCHANTMENTS = PlatformHelper.get().wrap(Registries.ENCHANTMENT);
public static final RegistryWrapper<ArgumentTypeInfo<?, ?>> COMMAND_ARGUMENT_TYPES = PlatformHelper.get().wrap(Registries.COMMAND_ARGUMENT_TYPE);
public static final RegistryWrapper<SoundEvent> SOUND_EVENTS = PlatformHelper.get().wrap(Registries.SOUND_EVENT);
public static final RegistryWrapper<RecipeSerializer<?>> RECIPE_SERIALIZERS = PlatformHelper.get().wrap(Registries.RECIPE_SERIALIZER);
public static final RegistryWrapper<MenuType<?>> MENU = PlatformHelper.get().wrap(Registries.MENU);

View File

@ -55,7 +55,7 @@ public TurtleTool fromNetwork(ResourceLocation id, FriendlyByteBuf buffer) {
var allowsEnchantments = buffer.readBoolean();
var consumesDurability = buffer.readEnum(TurtleToolDurability.class);
var breakable = buffer.readBoolean() ? TagKey.create(Registries.BLOCK, buffer.readResourceLocation()) : null;
var breakable = buffer.readNullable(b -> TagKey.create(Registries.BLOCK, b.readResourceLocation()));
return new TurtleTool(id, adjective, craftingItem, toolItem, damageMultiplier, allowsEnchantments, consumesDurability, breakable);
}
@ -67,7 +67,6 @@ public void toNetwork(FriendlyByteBuf buffer, TurtleTool upgrade) {
buffer.writeFloat(upgrade.damageMulitiplier);
buffer.writeBoolean(upgrade.allowEnchantments);
buffer.writeEnum(upgrade.consumeDurability);
buffer.writeBoolean(upgrade.breakable != null);
if (upgrade.breakable != null) buffer.writeResourceLocation(upgrade.breakable.location());
buffer.writeNullable(upgrade.breakable, (b, x) -> b.writeResourceLocation(x.location()));
}
}

View File

@ -0,0 +1,58 @@
// SPDX-FileCopyrightText: 2023 The CC: Tweaked Developers
//
// SPDX-License-Identifier: MPL-2.0
package dan200.computercraft.shared.network.client;
import dan200.computercraft.test.core.StructuralEquality;
import dan200.computercraft.test.shared.MinecraftArbitraries;
import dan200.computercraft.test.shared.WithMinecraft;
import io.netty.buffer.Unpooled;
import net.jqwik.api.*;
import net.minecraft.network.FriendlyByteBuf;
import net.minecraft.sounds.SoundEvent;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
@WithMinecraft
class PlayRecordClientMessageTest {
static {
WithMinecraft.Setup.bootstrap(); // @Property doesn't run test lifecycle methods.
}
/**
* Sends packets on a roundtrip, ensuring that their contents are reassembled on the other end.
*
* @param message The message to send.
*/
@Property
public void testRoundTrip(@ForAll("message") PlayRecordClientMessage message) {
var buffer = new FriendlyByteBuf(Unpooled.directBuffer());
message.toBytes(buffer);
var converted = new PlayRecordClientMessage(buffer);
assertEquals(buffer.readableBytes(), 0, "Whole packet was read");
assertThat("Messages are equal", converted, equality.asMatcher(PlayRecordClientMessage.class, message));
}
@Provide
Arbitrary<PlayRecordClientMessage> message() {
return Combinators.combine(
MinecraftArbitraries.blockPos(),
MinecraftArbitraries.soundEvent().injectNull(0.3),
Arbitraries.strings().ofMaxLength(1000).injectNull(0.3)
).as(PlayRecordClientMessage::new);
}
private static final StructuralEquality<PlayRecordClientMessage> equality = StructuralEquality.all(
StructuralEquality.field(PlayRecordClientMessage.class, "pos"),
StructuralEquality.field(PlayRecordClientMessage.class, "name"),
StructuralEquality.field(PlayRecordClientMessage.class, "soundEvent", StructuralEquality.all(
StructuralEquality.at("location", SoundEvent::getLocation),
StructuralEquality.field(SoundEvent.class, "range"),
StructuralEquality.field(SoundEvent.class, "newSystem")
))
);
}

View File

@ -0,0 +1,82 @@
// SPDX-FileCopyrightText: 2023 The CC: Tweaked Developers
//
// SPDX-License-Identifier: MPL-2.0
package dan200.computercraft.shared.turtle.upgrades;
import dan200.computercraft.api.turtle.ITurtleUpgrade;
import dan200.computercraft.api.turtle.TurtleToolDurability;
import dan200.computercraft.test.core.StructuralEquality;
import dan200.computercraft.test.shared.MinecraftArbitraries;
import dan200.computercraft.test.shared.WithMinecraft;
import io.netty.buffer.Unpooled;
import net.jqwik.api.*;
import net.minecraft.core.registries.Registries;
import net.minecraft.network.FriendlyByteBuf;
import net.minecraft.world.item.ItemStack;
import org.hamcrest.Description;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
@WithMinecraft
class TurtleToolSerialiserTest {
static {
WithMinecraft.Setup.bootstrap(); // @Property doesn't run test lifecycle methods.
}
/**
* Sends turtle tools on a roundtrip, ensuring that their contents are reassembled on the other end.
*
* @param tool The message to send.
*/
@Property
public void testRoundTrip(@ForAll("tool") TurtleTool tool) {
var buffer = new FriendlyByteBuf(Unpooled.directBuffer());
TurtleToolSerialiser.INSTANCE.toNetwork(buffer, tool);
var converted = TurtleToolSerialiser.INSTANCE.fromNetwork(tool.getUpgradeID(), buffer);
assertEquals(buffer.readableBytes(), 0, "Whole packet was read");
if (!equality.equals(tool, converted)) {
System.out.println("Break");
}
assertThat("Messages are equal", converted, equality.asMatcher(TurtleTool.class, tool));
}
@Provide
Arbitrary<TurtleTool> tool() {
return Combinators.combine(
MinecraftArbitraries.resourceLocation(),
Arbitraries.strings().ofMaxLength(100),
MinecraftArbitraries.item(),
MinecraftArbitraries.itemStack(),
Arbitraries.floats(),
Arbitraries.of(true, false),
Arbitraries.of(TurtleToolDurability.values()),
MinecraftArbitraries.tagKey(Registries.BLOCK)
).as(TurtleTool::new);
}
private static final StructuralEquality<ItemStack> stackEquality = new StructuralEquality<>() {
@Override
public boolean equals(ItemStack left, ItemStack right) {
return ItemStack.isSameItemSameTags(left, right) && left.getCount() == right.getCount();
}
@Override
public void describe(Description description, ItemStack object) {
description.appendValue(object).appendValue(object.getTag());
}
};
private static final StructuralEquality<TurtleTool> equality = StructuralEquality.all(
StructuralEquality.at("id", ITurtleUpgrade::getUpgradeID),
StructuralEquality.at("craftingItem", ITurtleUpgrade::getCraftingItem, stackEquality),
StructuralEquality.at("tool", x -> x.item, stackEquality),
StructuralEquality.at("damageMulitiplier", x -> x.damageMulitiplier),
StructuralEquality.at("allowEnchantments", x -> x.allowEnchantments),
StructuralEquality.at("consumeDurability", x -> x.consumeDurability),
StructuralEquality.at("breakable", x -> x.breakable)
);
}