Skip to content

Commit

Permalink
Fix attachment sync when changing world + migrate to client gametest (#…
Browse files Browse the repository at this point in the history 
…4366)

* Rework testmod into client gametest

* Fix broken mixin

ClientPlayNetworkHandlerMixin silently broke due to targeting an instruction that changed from being unconditional to inside of a branch

* spotless

* Review feedback

---------

Co-authored-by: modmuss50 <[email protected]>
(cherry picked from commit 314f4e4)
  • Loading branch information
@Syst3ms @modmuss50
Syst3ms authored and modmuss50 committed Jan 14, 2025
1 parent cb3101d commit 97c1b82
Show file tree
Hide file tree
Showing 12 changed files with 357 additions and 478 deletions.
3 changes: 2 additions & 1 deletion fabric-data-attachment-api-v1/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ testDependencies(project, [
':fabric-lifecycle-events-v1',
':fabric-biome-api-v1',
':fabric-command-api-v2',
':fabric-rendering-v1'
':fabric-rendering-v1',
':fabric-client-gametest-api-v1'
])
14 changes: 11 additions & 3 deletions ...lient/java/net/fabricmc/fabric/mixin/attachment/client/ClientPlayNetworkHandlerMixin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
import com.llamalad7.mixinextras.sugar.Local;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Slice;

import net.minecraft.client.MinecraftClient;
import net.minecraft.client.network.ClientPlayNetworkHandler;
import net.minecraft.client.network.ClientPlayerEntity;
import net.minecraft.network.packet.s2c.play.PlayerRespawnS2CPacket;
Expand All @@ -32,13 +34,19 @@
abstract class ClientPlayNetworkHandlerMixin {
@WrapOperation(
method = "onPlayerRespawn",
at = @At(value = "INVOKE", target = "Lnet/minecraft/client/network/ClientPlayerEntity;init()V")
at = @At(
value = "FIELD",
target = "Lnet/minecraft/client/MinecraftClient;player:Lnet/minecraft/client/network/ClientPlayerEntity;"
),
slice = @Slice(
from = @At(value = "INVOKE", target = "Lnet/minecraft/client/network/ClientPlayNetworkHandler;startWorldLoading(Lnet/minecraft/client/network/ClientPlayerEntity;Lnet/minecraft/client/world/ClientWorld;Lnet/minecraft/client/gui/screen/DownloadingTerrainScreen$WorldEntryReason;)V")
)
)
private void copyAttachmentsOnClientRespawn(ClientPlayerEntity newPlayer, Operation<Void> init, PlayerRespawnS2CPacket packet, @Local(ordinal = 0) ClientPlayerEntity oldPlayer) {
private void copyAttachmentsOnClientRespawn(MinecraftClient client, ClientPlayerEntity newPlayer, Operation<Void> init, PlayerRespawnS2CPacket packet, @Local(ordinal = 0) ClientPlayerEntity oldPlayer) {
/*
* The KEEP_ATTRIBUTES flag is not set on a death respawn, and set in all other cases
*/
AttachmentTargetImpl.transfer(oldPlayer, newPlayer, !packet.hasFlag(PlayerRespawnS2CPacket.KEEP_ATTRIBUTES));
init.call(newPlayer);
init.call(client, newPlayer);
}
}
12 changes: 12 additions & 0 deletions ...achment-api-v1/src/main/java/net/fabricmc/fabric/impl/attachment/sync/AttachmentSync.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import net.minecraft.util.Identifier;

import net.fabricmc.api.ModInitializer;
import net.fabricmc.fabric.api.entity.event.v1.ServerEntityWorldChangeEvents;
import net.fabricmc.fabric.api.networking.v1.EntityTrackingEvents;
import net.fabricmc.fabric.api.networking.v1.PayloadTypeRegistry;
import net.fabricmc.fabric.api.networking.v1.ServerConfigurationConnectionEvents;
Expand Down Expand Up @@ -114,6 +115,17 @@ public void onInitialize() {
}
});

ServerEntityWorldChangeEvents.AFTER_PLAYER_CHANGE_WORLD.register((player, origin, destination) -> {
// sync new world's attachments
// no conflict with previous one because the client world is recreated every time
List<AttachmentChange> changes = new ArrayList<>();
((AttachmentTargetImpl) destination).fabric_computeInitialSyncChanges(player, changes::add);

if (!changes.isEmpty()) {
AttachmentChange.partitionAndSendPackets(changes, player);
}
});

EntityTrackingEvents.START_TRACKING.register((trackedEntity, player) -> {
List<AttachmentChange> changes = new ArrayList<>();
((AttachmentTargetImpl) trackedEntity).fabric_computeInitialSyncChanges(player, changes::add);
Expand Down
172 changes: 1 addition & 171 deletions ...chment-api-v1/src/testmod/java/net/fabricmc/fabric/test/attachment/AttachmentTestMod.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,58 +16,26 @@

package net.fabricmc.fabric.test.attachment;

import static net.minecraft.server.command.CommandManager.argument;
import static net.minecraft.server.command.CommandManager.literal;

import java.io.File;
import java.io.IOException;

import com.mojang.brigadier.builder.LiteralArgumentBuilder;
import com.mojang.brigadier.context.CommandContext;
import com.mojang.brigadier.exceptions.CommandSyntaxException;
import com.mojang.brigadier.exceptions.SimpleCommandExceptionType;
import com.mojang.serialization.Codec;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import net.minecraft.block.entity.BlockEntity;
import net.minecraft.command.argument.BlockPosArgumentType;
import net.minecraft.command.argument.ColumnPosArgumentType;
import net.minecraft.command.argument.EntityArgumentType;
import net.minecraft.entity.EquipmentSlot;
import net.minecraft.item.ItemStack;
import net.minecraft.network.codec.PacketCodecs;
import net.minecraft.registry.Registries;
import net.minecraft.registry.Registry;
import net.minecraft.registry.RegistryKey;
import net.minecraft.registry.RegistryKeys;
import net.minecraft.server.command.ServerCommandSource;
import net.minecraft.server.network.ServerPlayerEntity;
import net.minecraft.server.world.ServerWorld;
import net.minecraft.text.Text;
import net.minecraft.util.Identifier;
import net.minecraft.util.WorldSavePath;
import net.minecraft.util.math.ChunkPos;
import net.minecraft.util.math.ColumnPos;
import net.minecraft.world.chunk.Chunk;
import net.minecraft.world.chunk.ChunkStatus;
import net.minecraft.world.chunk.ProtoChunk;
import net.minecraft.world.chunk.WorldChunk;
import net.minecraft.world.chunk.WrapperProtoChunk;
import net.minecraft.world.gen.GenerationStep;
import net.minecraft.world.gen.feature.DefaultFeatureConfig;

import net.fabricmc.api.ModInitializer;
import net.fabricmc.fabric.api.attachment.v1.AttachmentRegistry;
import net.fabricmc.fabric.api.attachment.v1.AttachmentSyncPredicate;
import net.fabricmc.fabric.api.attachment.v1.AttachmentTarget;
import net.fabricmc.fabric.api.attachment.v1.AttachmentType;
import net.fabricmc.fabric.api.biome.v1.BiomeModifications;
import net.fabricmc.fabric.api.biome.v1.BiomeSelectors;
import net.fabricmc.fabric.api.command.v2.CommandRegistrationCallback;
import net.fabricmc.fabric.api.event.lifecycle.v1.ServerChunkEvents;
import net.fabricmc.fabric.api.event.lifecycle.v1.ServerEntityEvents;
import net.fabricmc.fabric.api.event.lifecycle.v1.ServerLifecycleEvents;

public class AttachmentTestMod implements ModInitializer {
public static final String MOD_ID = "fabric-data-attachment-api-v1-testmod";
Expand Down Expand Up @@ -101,7 +69,7 @@ public class AttachmentTestMod implements ModInitializer {
.syncWith(PacketCodecs.BOOLEAN, AttachmentSyncPredicate.allButTarget())
);
public static final AttachmentType<Boolean> SYNCED_CREATIVE_ONLY = AttachmentRegistry.create(
Identifier.of(MOD_ID, "synced_custom"),
Identifier.of(MOD_ID, "synced_creative"),
builder -> builder
.initializer(() -> false)
.persistent(Codec.BOOL)
Expand All @@ -114,12 +82,6 @@ public class AttachmentTestMod implements ModInitializer {
.persistent(ItemStack.CODEC)
.syncWith(ItemStack.OPTIONAL_PACKET_CODEC, AttachmentSyncPredicate.all())
);
public static final SimpleCommandExceptionType TARGET_NOT_FOUND = new SimpleCommandExceptionType(Text.literal("Target not found"));

public static final ChunkPos FAR_CHUNK_POS = new ChunkPos(300, 0);

private boolean serverStarted = false;
public static boolean featurePlaced = false;

@Override
public void onInitialize() {
Expand All @@ -130,137 +92,5 @@ public void onInitialize() {
GenerationStep.Feature.VEGETAL_DECORATION,
RegistryKey.of(RegistryKeys.PLACED_FEATURE, Identifier.of(MOD_ID, "set_attachment"))
);

ServerLifecycleEvents.SERVER_STARTED.register(server -> {
File saveRoot = server.getSavePath(WorldSavePath.ROOT).toFile();
File markerFile = new File(saveRoot, MOD_ID + "_MARKER");
boolean firstLaunch;

try {
firstLaunch = markerFile.createNewFile();
} catch (IOException e) {
throw new RuntimeException(e);
}

ServerWorld overworld = server.getOverworld();
WorldChunk chunk = overworld.getChunk(0, 0);

if (firstLaunch) {
LOGGER.info("First launch, testing attachment by feature");

if (featurePlaced) {
if (!"feature".equals(chunk.getAttached(FEATURE_ATTACHMENT))) {
throw new AssertionError("Feature did not write attachment to ProtoChunk");
}
} else {
LOGGER.warn("Feature not placed, could not test writing during worldgen");
}

LOGGER.info("setting up persistent attachments");

overworld.setAttached(PERSISTENT, "world_data");

chunk.setAttached(PERSISTENT, "chunk_data");
chunk.setAttached(SYNCED_WITH_ALL, true);

ProtoChunk protoChunk = (ProtoChunk) overworld.getChunkManager().getChunk(FAR_CHUNK_POS.x, FAR_CHUNK_POS.z, ChunkStatus.STRUCTURE_STARTS, true);
protoChunk.setAttached(PERSISTENT, "protochunk_data");
} else {
LOGGER.info("Second launch, testing persistent attachments");

if (!"world_data".equals(overworld.getAttached(PERSISTENT))) throw new AssertionError("World attachment did not persist");
if (!"chunk_data".equals(chunk.getAttached(PERSISTENT))) throw new AssertionError("WorldChunk attachment did not persist");

WrapperProtoChunk wrapperProtoChunk = (WrapperProtoChunk) overworld.getChunkManager().getChunk(0, 0, ChunkStatus.EMPTY, true);
if (!"chunk_data".equals(wrapperProtoChunk.getAttached(PERSISTENT))) throw new AssertionError("Attachment is not accessible through WrapperProtoChunk");

Chunk farChunk = overworld.getChunkManager().getChunk(FAR_CHUNK_POS.x, FAR_CHUNK_POS.z, ChunkStatus.EMPTY, true);

if (farChunk instanceof WrapperProtoChunk) {
LOGGER.warn("Far chunk already generated, can't test persistence in ProtoChunk.");
} else {
if (!"protochunk_data".equals(farChunk.getAttached(PERSISTENT))) throw new AssertionError("ProtoChunk attachment did not persist");
}
}

serverStarted = true;
});

// Testing hint: load far chunk by running /tp @s 4800 ~ 0
ServerChunkEvents.CHUNK_LOAD.register(((world, chunk) -> {
if (!chunk.getPos().equals(FAR_CHUNK_POS)) return;

if (!serverStarted) {
LOGGER.warn("Chunk {} loaded before server started, can't test transfer of attachments to WorldChunk", FAR_CHUNK_POS);
return;
}

LOGGER.info("Loaded chunk {}, testing transfer of attachments to WorldChunk", FAR_CHUNK_POS);

if (!"protochunk_data".equals(chunk.getAttached(PERSISTENT))) throw new AssertionError("ProtoChunk attachment was not transfered to WorldChunk");
}));

CommandRegistrationCallback.EVENT.register((dispatcher, registryAccess, environment) -> dispatcher.register(
literal("attachment")
.then(buildCommandForKind("all", "all", SYNCED_WITH_ALL))
.then(buildCommandForKind("self_only", "only self", SYNCED_WITH_TARGET))
.then(buildCommandForKind("others_only", "all but self", SYNCED_EXCEPT_TARGET))
.then(buildCommandForKind("creative_only", "creative players only", SYNCED_CREATIVE_ONLY))
));

ServerEntityEvents.EQUIPMENT_CHANGE.register((livingEntity, equipmentSlot, previousStack, currentStack) -> {
if (equipmentSlot == EquipmentSlot.HEAD && livingEntity instanceof ServerPlayerEntity player) {
player.setAttached(SYNCED_ITEM, currentStack);
}
});
}

private static LiteralArgumentBuilder<ServerCommandSource> buildCommandForKind(String id, String syncedWith, AttachmentType<Boolean> type) {
return literal(id).executes(context -> updateAttachmentFor(
context.getSource().getPlayerOrThrow(),
type,
context,
"Set self flag (synced with %s) to %%s".formatted(syncedWith)
)).then(
argument("target", EntityArgumentType.entity()).executes(context -> updateAttachmentFor(
EntityArgumentType.getEntity(context, "target"),
type,
context,
"Set entity flag (synced with %s) to %%s".formatted(syncedWith)
))
).then(argument("pos", BlockPosArgumentType.blockPos()).executes(context -> {
BlockEntity be = context.getSource().getWorld().getBlockEntity(BlockPosArgumentType.getBlockPos(context, "pos"));

if (be == null) {
throw TARGET_NOT_FOUND.create();
}

return updateAttachmentFor(
be,
type,
context,
"Set block entity flag (synced with %s) to %%s".formatted(syncedWith)
);
})).then(argument("chunkPos", ColumnPosArgumentType.columnPos()).executes(context -> {
ColumnPos pos = ColumnPosArgumentType.getColumnPos(context, "chunkpos");
return updateAttachmentFor(
context.getSource().getWorld().getChunk(pos.x(), pos.z(), ChunkStatus.STRUCTURE_STARTS, true),
type,
context,
"Set chunk flag (synced with %s) to %%s".formatted(syncedWith)
);
})).then(literal("world").executes(context -> updateAttachmentFor(
context.getSource().getWorld(),
type,
context,
"Set world flag (synced with %s) to %%s".formatted(syncedWith)
)));
}

private static int updateAttachmentFor(AttachmentTarget target, AttachmentType<Boolean> attachment, CommandContext<ServerCommandSource> context, String messageFormat) throws CommandSyntaxException {
boolean current = target.getAttachedOrElse(attachment, false);
target.setAttached(attachment, !current);
context.getSource().sendFeedback(() -> Text.literal(messageFormat.formatted(!current)), false);
return 1;
}
}
6 changes: 4 additions & 2 deletions ...ent-api-v1/src/testmod/java/net/fabricmc/fabric/test/attachment/SetAttachmentFeature.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import net.minecraft.world.gen.feature.util.FeatureContext;

public class SetAttachmentFeature extends Feature<DefaultFeatureConfig> {
public static boolean featurePlaced;

public SetAttachmentFeature(Codec<DefaultFeatureConfig> codec) {
super(codec);
}
Expand All @@ -36,13 +38,13 @@ public boolean generate(FeatureContext<DefaultFeatureConfig> context) {
Chunk chunk = context.getWorld().getChunk(context.getOrigin());

if (chunk.getPos().equals(new ChunkPos(0, 0))) {
AttachmentTestMod.featurePlaced = true;
featurePlaced = true;

if (!(chunk instanceof ProtoChunk) || chunk instanceof WrapperProtoChunk) {
AttachmentTestMod.LOGGER.warn("Feature not attaching to ProtoChunk");
}

chunk.setAttached(AttachmentTestMod.FEATURE_ATTACHMENT, "feature");
chunk.setAttached(AttachmentTestMod.FEATURE_ATTACHMENT, "feature_data");
}

return true;
Expand Down
13 changes: 5 additions & 8 deletions fabric-data-attachment-api-v1/src/testmod/resources/fabric.mod.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,16 @@
"main": [
"net.fabricmc.fabric.test.attachment.AttachmentTestMod"
],
"client": [
"net.fabricmc.fabric.test.attachment.client.AttachmentTestModClient"
],
"fabric-gametest": [
"net.fabricmc.fabric.test.attachment.gametest.AttachmentCopyTests",
"net.fabricmc.fabric.test.attachment.gametest.BlockEntityTests"
],
"fabric-client-gametest": [
"net.fabricmc.fabric.test.attachment.client.gametest.PersistenceGametest",
"net.fabricmc.fabric.test.attachment.client.gametest.SyncGametest"
]
},
"mixins": [
"fabric-data-attachment-api-v1-testmod.mixins.json",
{
"config": "fabric-data-attachment-api-v1-testmod.client.mixins.json",
"environment": "client"
}
"fabric-data-attachment-api-v1-testmod.mixins.json"
]
}
Loading

0 comments on commit 97c1b82

Please sign in to comment.