Skip to content

Commit

Permalink
Extend Data Attachment API to ProtoChunk (FabricMC#3548)
Browse files Browse the repository at this point in the history
* allow data-attachment on ProtoChunks

- moved interfaceInjection from WorldChunk to Chunk
- dataAttachment saving on ProtoChunks in ChunkSerializer
- copy attachment from ProtoChunk to WorldChunk on creation.
- make WrapperProtoChunk wrap attachment calls to WorldChunk

* add test for data-attachment on ProtoChunks, and extend testmod.

* code style and license headers

* fix typos in javadoc

* extend testmod to test setting attachment during worldgen.

* code formatting

* fix testmod: don't crash when feature isn't placed (i.e. on GameTest server)

* add warning when adding persistent attachment to chunk with status EMPTY.

* update javadoc

* update javadoc to reference ServerLivingEntityEvents#MOB_CONVERSION
  • Loading branch information
jacobsjo authored Feb 9, 2024
1 parent d74054c commit 32782cf
Show file tree
Hide file tree
Showing 17 changed files with 312 additions and 43 deletions.
3 changes: 2 additions & 1 deletion fabric-data-attachment-api-v1/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ moduleDependencies(project, [
])

testDependencies(project, [
':fabric-lifecycle-events-v1'
':fabric-lifecycle-events-v1',
':fabric-biome-api-v1'
])
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ private void copyAttachmentsOnClientRespawn(ClientPlayerEntity newPlayer, Operat
/*
* The KEEP_ATTRIBUTES flag is not set on a death respawn, and set in all other cases
*/
AttachmentTargetImpl.copyOnRespawn(oldPlayer, newPlayer, !packet.hasFlag(PlayerRespawnS2CPacket.KEEP_ATTRIBUTES));
AttachmentTargetImpl.transfer(oldPlayer, newPlayer, !packet.hasFlag(PlayerRespawnS2CPacket.KEEP_ATTRIBUTES));
init.call(newPlayer);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@
import net.minecraft.entity.Entity;
import net.minecraft.server.world.ServerWorld;
import net.minecraft.world.chunk.Chunk;
import net.minecraft.world.chunk.WorldChunk;
import net.minecraft.world.chunk.ChunkStatus;

/**
* Marks all objects on which data can be attached using {@link AttachmentType}s.
*
* <p>Fabric implements this on {@link Entity}, {@link BlockEntity}, {@link ServerWorld} and {@link WorldChunk} via mixin.</p>
* <p>Fabric implements this on {@link Entity}, {@link BlockEntity}, {@link ServerWorld} and {@link Chunk} via mixin.</p>
*
* <p>Note about {@link BlockEntity} and {@link WorldChunk} targets: these objects need to be notified of changes to their
* <p>Note about {@link BlockEntity} and {@link Chunk} targets: these objects need to be notified of changes to their
* state (using {@link BlockEntity#markDirty()} and {@link Chunk#setNeedsSaving(boolean)} respectively), otherwise the modifications will not take effect properly.
* The {@link #setAttached(AttachmentType, Object)} method handles this automatically, but this needs to be done manually
* when attached data is mutable, for example:
Expand All @@ -55,6 +55,12 @@
* which takes care of all vanilla types. However, modded block entities may be coded differently, so be wary of this
* when attaching data to modded block entities.
* </p>
*
* <p>
* Note about {@link Chunk} targets with {@link ChunkStatus#EMPTY}: These chunks are not saved unless the generation
* progresses to at least {@link ChunkStatus#STRUCTURE_STARTS}. Therefore, persistent attachments to those chunks may not
* be saved. The {@link #setAttached(AttachmentType, Object)} method will log a warning when this is attempted.
* </p>
*/
@ApiStatus.Experimental
@ApiStatus.NonExtendable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,14 @@
import org.jetbrains.annotations.Nullable;

import net.minecraft.entity.Entity;
import net.minecraft.entity.EntityType;
import net.minecraft.entity.mob.MobEntity;
import net.minecraft.server.world.ServerWorld;
import net.minecraft.util.Identifier;
import net.minecraft.world.chunk.Chunk;
import net.minecraft.world.chunk.ProtoChunk;
import net.minecraft.world.chunk.WorldChunk;

import net.fabricmc.fabric.api.entity.event.v1.ServerEntityWorldChangeEvents;
import net.fabricmc.fabric.api.entity.event.v1.ServerLivingEntityEvents;
import net.fabricmc.fabric.api.entity.event.v1.ServerPlayerEvents;

/**
Expand All @@ -39,13 +42,15 @@
* immutable types. More generally, different attachments <i>must not</i> share mutable state, and it is <i>strongly advised</i>
* for attachments not to hold internal references to their target. See the following note on entity targets.</p>
*
* <p>Note on {@link Entity} targets: in several instances, the name needs to copy data from one {@link Entity} to another.
* These are player respawning, mob conversion, return from the End and cross-world entity teleportation. By default,
* attachments are simply copied wholesale, up to {@link #copyOnDeath()}. Since one entity instance is discarded,
* an attachment that keeps a reference to an {@link Entity} instance can and will break unexpectedly. If,
* for whatever reason, keeping to reference to the target entity is absolutely necessary, be sure to use
* {@link ServerPlayerEvents#COPY_FROM}, {@link ServerEntityWorldChangeEvents#AFTER_ENTITY_CHANGE_WORLD}
* and a mixin into {@link MobEntity#convertTo(EntityType, boolean)} to implement custom copying logic.</p>
* <p>Note on {@link Entity} and {@link Chunk} targets: in several instances, the game needs to copy data from one instance to another.
* These are player respawning, mob conversion, return from the End, cross-world entity teleportation, and conversion of a {@link ProtoChunk} to
* {@link WorldChunk}. By default, attachments are simply copied wholesale, up to {@link #copyOnDeath()}. Since one instance is discarded,
* an attachment that keeps a reference to an {@link Entity} or {@link ProtoChunk} instance can and will break unexpectedly. If,
* for whatever reason, keeping a reference to the target is absolutely necessary, be sure to implement custom copying logic.
* For {@link Entity} targets, use {@link ServerPlayerEvents#COPY_FROM}, {@link ServerEntityWorldChangeEvents#AFTER_ENTITY_CHANGE_WORLD},
* and {@link ServerLivingEntityEvents#MOB_CONVERSION}. For {@link Chunk} targets, mixin into
* {@link WorldChunk#WorldChunk(ServerWorld, ProtoChunk, WorldChunk.EntityLoader)}.
* </p>
*
* @param <A> type of the attached data. It is encouraged for this to be an immutable type.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,28 @@

package net.fabricmc.fabric.impl.attachment;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import net.fabricmc.api.ModInitializer;
import net.fabricmc.fabric.api.entity.event.v1.ServerEntityWorldChangeEvents;
import net.fabricmc.fabric.api.entity.event.v1.ServerLivingEntityEvents;
import net.fabricmc.fabric.api.entity.event.v1.ServerPlayerEvents;

public class AttachmentEntrypoint implements ModInitializer {
public static final Logger LOGGER = LoggerFactory.getLogger("fabric-data-attachment-api-v1");

@Override
public void onInitialize() {
ServerPlayerEvents.COPY_FROM.register((oldPlayer, newPlayer, alive) ->
AttachmentTargetImpl.copyOnRespawn(oldPlayer, newPlayer, !alive)
AttachmentTargetImpl.transfer(oldPlayer, newPlayer, !alive)
);
ServerEntityWorldChangeEvents.AFTER_ENTITY_CHANGE_WORLD.register(((originalEntity, newEntity, origin, destination) ->
AttachmentTargetImpl.copyOnRespawn(originalEntity, newEntity, false))
AttachmentTargetImpl.transfer(originalEntity, newEntity, false))
);
// using the corresponding player event is unnecessary as no new instance is created
ServerLivingEntityEvents.MOB_CONVERSION.register((previous, converted, keepEquipment) ->
AttachmentTargetImpl.copyOnRespawn(previous, converted, true)
AttachmentTargetImpl.transfer(previous, converted, true)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@

public interface AttachmentTargetImpl extends AttachmentTarget {
/**
* Copies entity attachments when it is respawned and a new instance is created.
* Is triggered on player respawn, entity conversion, return from the End or cross-world entity teleportation.
* Copies attachments from the original to the target. This is used when a ProtoChunk is converted to a
* WorldChunk, and when an entity is respawned and a new instance is created. For entity respawns, it is
* triggered on player respawn, entity conversion, return from the End, or cross-world entity teleportation.
* In the first two cases, only the attachments with {@link AttachmentType#copyOnDeath()} will be transferred.
*/
*/
@SuppressWarnings("unchecked")
static void copyOnRespawn(AttachmentTarget original, AttachmentTarget target, boolean isDeath) {
static void transfer(AttachmentTarget original, AttachmentTarget target, boolean isDeath) {
Map<AttachmentType<?>, ?> attachments = ((AttachmentTargetImpl) original).fabric_getAttachments();

if (attachments == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,14 @@
import net.minecraft.nbt.NbtCompound;
import net.minecraft.world.World;
import net.minecraft.world.chunk.Chunk;
import net.minecraft.world.chunk.WorldChunk;
import net.minecraft.world.chunk.ChunkStatus;

import net.fabricmc.fabric.api.attachment.v1.AttachmentType;
import net.fabricmc.fabric.impl.attachment.AttachmentEntrypoint;
import net.fabricmc.fabric.impl.attachment.AttachmentSerializingImpl;
import net.fabricmc.fabric.impl.attachment.AttachmentTargetImpl;

@Mixin({BlockEntity.class, Entity.class, World.class, WorldChunk.class})
@Mixin({BlockEntity.class, Entity.class, World.class, Chunk.class})
abstract class AttachmentTargetsMixin implements AttachmentTargetImpl {
@Nullable
private IdentityHashMap<AttachmentType<?>, Object> fabric_dataAttachments = null;
Expand All @@ -54,8 +55,12 @@ public <T> T setAttached(AttachmentType<T> type, @Nullable T value) {

if (thisObject instanceof BlockEntity) {
((BlockEntity) thisObject).markDirty();
} else if (thisObject instanceof WorldChunk) {
} else if (thisObject instanceof Chunk) {
((Chunk) thisObject).setNeedsSaving(true);

if (type.isPersistent() && ((Chunk) thisObject).getStatus().equals(ChunkStatus.EMPTY)) {
AttachmentEntrypoint.LOGGER.warn("Attaching persistent attachment {} to chunk with chunk status EMPTY. Attachment might be discarded.", type.identifier());
}
}

if (value == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import net.minecraft.util.math.ChunkPos;
import net.minecraft.world.ChunkSerializer;
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.poi.PointOfInterestStorage;

Expand All @@ -42,7 +42,19 @@ abstract class ChunkSerializerMixin {
),
method = "deserialize"
)
private static WorldChunk readChunkAttachments(WorldChunk chunk, ServerWorld world, PointOfInterestStorage poiStorage, ChunkPos chunkPos, NbtCompound nbt) {
private static WorldChunk readWorldChunkAttachments(WorldChunk chunk, ServerWorld world, PointOfInterestStorage poiStorage, ChunkPos chunkPos, NbtCompound nbt) {
((AttachmentTargetImpl) chunk).fabric_readAttachmentsFromNbt(nbt);
return chunk;
}

@ModifyExpressionValue(
at = @At(
value = "NEW",
target = "net/minecraft/world/chunk/ProtoChunk"
),
method = "deserialize"
)
private static ProtoChunk readProtoChunkAttachments(ProtoChunk chunk, ServerWorld world, PointOfInterestStorage poiStorage, ChunkPos chunkPos, NbtCompound nbt) {
((AttachmentTargetImpl) chunk).fabric_readAttachmentsFromNbt(nbt);
return chunk;
}
Expand All @@ -52,8 +64,6 @@ private static WorldChunk readChunkAttachments(WorldChunk chunk, ServerWorld wor
method = "serialize"
)
private static void writeChunkAttachments(ServerWorld world, Chunk chunk, CallbackInfoReturnable<NbtCompound> cir) {
if (chunk.getStatus().getChunkType() == ChunkStatus.ChunkType.LEVELCHUNK) {
((AttachmentTargetImpl) chunk).fabric_writeAttachmentsToNbt(cir.getReturnValue());
}
((AttachmentTargetImpl) chunk).fabric_writeAttachmentsToNbt(cir.getReturnValue());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright (c) 2016, 2017, 2018, 2019 FabricMC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package net.fabricmc.fabric.mixin.attachment;

import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;

import net.minecraft.server.world.ServerWorld;
import net.minecraft.world.chunk.ProtoChunk;
import net.minecraft.world.chunk.WorldChunk;

import net.fabricmc.fabric.api.attachment.v1.AttachmentTarget;
import net.fabricmc.fabric.impl.attachment.AttachmentTargetImpl;

@Mixin(WorldChunk.class)
public class WorldChunkMixin {
@Inject(
method = "<init>(Lnet/minecraft/server/world/ServerWorld;Lnet/minecraft/world/chunk/ProtoChunk;Lnet/minecraft/world/chunk/WorldChunk$EntityLoader;)V",
at = @At("TAIL")
)
public void transferProtoChunkAttachement(ServerWorld world, ProtoChunk protoChunk, WorldChunk.EntityLoader entityLoader, CallbackInfo ci) {
AttachmentTargetImpl.transfer(protoChunk, (AttachmentTarget) this, false);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* Copyright (c) 2016, 2017, 2018, 2019 FabricMC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package net.fabricmc.fabric.mixin.attachment;

import java.util.Map;

import org.jetbrains.annotations.Nullable;
import org.spongepowered.asm.mixin.Final;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Shadow;

import net.minecraft.nbt.NbtCompound;
import net.minecraft.world.chunk.WorldChunk;
import net.minecraft.world.chunk.WrapperProtoChunk;

import net.fabricmc.fabric.api.attachment.v1.AttachmentType;
import net.fabricmc.fabric.impl.attachment.AttachmentTargetImpl;

@Mixin(WrapperProtoChunk.class)
public class WrapperProtoChunkMixin implements AttachmentTargetImpl {
@Shadow
@Final
private WorldChunk wrapped;

@Override
@Nullable
public <T> T getAttached(AttachmentType<T> type) {
return this.wrapped.getAttached(type);
}

@Override
@Nullable
public <T> T setAttached(AttachmentType<T> type, @Nullable T value) {
return this.wrapped.setAttached(type, value);
}

@Override
public boolean hasAttached(AttachmentType<?> type) {
return this.wrapped.hasAttached(type);
}

@Override
public void fabric_writeAttachmentsToNbt(NbtCompound nbt) {
((AttachmentTargetImpl) this.wrapped).fabric_writeAttachmentsToNbt(nbt);
}

@Override
public void fabric_readAttachmentsFromNbt(NbtCompound nbt) {
((AttachmentTargetImpl) this.wrapped).fabric_readAttachmentsFromNbt(nbt);
}

@Override
public boolean fabric_hasPersistentAttachments() {
return ((AttachmentTargetImpl) this.wrapped).fabric_hasPersistentAttachments();
}

@Override
public Map<AttachmentType<?>, ?> fabric_getAttachments() {
return ((AttachmentTargetImpl) this.wrapped).fabric_getAttachments();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
"BlockEntityUpdateS2CPacketMixin",
"ChunkSerializerMixin",
"EntityMixin",
"ServerWorldMixin"
"ServerWorldMixin",
"WorldChunkMixin",
"WrapperProtoChunkMixin"
],
"injectors": {
"defaultRequire": 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"fabric-api:module-lifecycle": "experimental",
"loom:injected_interfaces": {
"net/minecraft/class_2586": ["net/fabricmc/fabric/api/attachment/v1/AttachmentTarget"],
"net/minecraft/class_2818": ["net/fabricmc/fabric/api/attachment/v1/AttachmentTarget"],
"net/minecraft/class_2791": ["net/fabricmc/fabric/api/attachment/v1/AttachmentTarget"],
"net/minecraft/class_1297": ["net/fabricmc/fabric/api/attachment/v1/AttachmentTarget"],
"net/minecraft/class_3218": ["net/fabricmc/fabric/api/attachment/v1/AttachmentTarget"]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import net.minecraft.server.world.ServerWorld;
import net.minecraft.util.Identifier;
import net.minecraft.util.math.BlockPos;
import net.minecraft.world.chunk.ProtoChunk;
import net.minecraft.world.chunk.WorldChunk;

import net.fabricmc.fabric.api.attachment.v1.AttachmentRegistry;
Expand Down Expand Up @@ -82,8 +83,9 @@ void testTargets() {
Entity entity = mock(Entity.class, CALLS_REAL_METHODS);
BlockEntity blockEntity = mock(BlockEntity.class, CALLS_REAL_METHODS);
WorldChunk worldChunk = mock(WorldChunk.class, CALLS_REAL_METHODS);
ProtoChunk protoChunk = mock(ProtoChunk.class, CALLS_REAL_METHODS);

for (AttachmentTarget target : new AttachmentTarget[]{serverWorld, entity, blockEntity, worldChunk}) {
for (AttachmentTarget target : new AttachmentTarget[]{serverWorld, entity, blockEntity, worldChunk, protoChunk}) {
testForTarget(target, basic);
}
}
Expand Down Expand Up @@ -161,8 +163,8 @@ void testEntityCopy() {
Entity respawnTarget = mock(Entity.class, CALLS_REAL_METHODS);
Entity nonRespawnTarget = mock(Entity.class, CALLS_REAL_METHODS);

AttachmentTargetImpl.copyOnRespawn(original, respawnTarget, true);
AttachmentTargetImpl.copyOnRespawn(original, nonRespawnTarget, false);
AttachmentTargetImpl.transfer(original, respawnTarget, true);
AttachmentTargetImpl.transfer(original, nonRespawnTarget, false);
assertTrue(respawnTarget.hasAttached(copiedOnRespawn));
assertFalse(respawnTarget.hasAttached(notCopiedOnRespawn));
assertTrue(nonRespawnTarget.hasAttached(copiedOnRespawn));
Expand Down
Loading

0 comments on commit 32782cf

Please sign in to comment.