Friends & Foes (Forge/NeoForge) (Copper Golem,Crab,Glare,Moobloom,Iceologer,Rascal,Tuff Golem,Wildfire,Illusioner)

[1.20.1] Mixin injecting SynchedEntityData into entities not owned by Friends and Foes is dangerous

TelepathicGrunt opened this issue ยท 1 comments

commented

Minecraft version information

1.20.1

Mod loader information

Forge

The issue

Entity Data Accessors should only be ever set by the creator of the entity. Never added to by other mods.

The reason for this is because Entity Data Accessors are order dependent so if multiple mods tries to add these entity data accessors to mobs and on client, they run in a different order than the server did (because mod load order is not guaranteed), the client will get forcibly disconnected from server when the entity spawns.

Instead, this mixin part needs to be removed and migrated properly to a Forge Capability because capabilities were designed specifically for this use case of attaching data to arbitrary entities. It is safer for mods as the vanilla system is designed specifically only for vanilla and unsafe for mods. Then you would use your own packet to send the data to client when needed.

private static final String WILDFIRE_UUID_NBT_NAME = "WildfireUuid";
private static final TrackedData<Optional<UUID>> WILDFIRE_UUID;
static {
WILDFIRE_UUID = DataTracker.registerData(BlazeEntityMixin.class, TrackedDataHandlerRegistry.OPTIONAL_UUID);
}
protected BlazeEntityMixin(EntityType<? extends HostileEntity> entityType, World world) {
super(entityType, world);
}
@Inject(
at = @At("TAIL"),
method = "initDataTracker"
)
public void initDataTracker(CallbackInfo ci) {
this.dataTracker.startTracking(WILDFIRE_UUID, Optional.empty());
}

private static final TrackedData<Boolean> IS_ILLUSION;
private static final TrackedData<Boolean> WAS_ATTACKED;
private static final TrackedData<Integer> TICKS_UNTIL_DESPAWN;
private static final TrackedData<Integer> TICKS_UNTIL_CAN_CREATE_ILLUSIONS;
private IllusionerEntity friendsandfoes_illusioner;
static {
IS_ILLUSION = DataTracker.registerData(IllusionerEntityMixin.class, TrackedDataHandlerRegistry.BOOLEAN);
WAS_ATTACKED = DataTracker.registerData(IllusionerEntityMixin.class, TrackedDataHandlerRegistry.BOOLEAN);
TICKS_UNTIL_DESPAWN = DataTracker.registerData(IllusionerEntityMixin.class, TrackedDataHandlerRegistry.INTEGER);
TICKS_UNTIL_CAN_CREATE_ILLUSIONS = DataTracker.registerData(IllusionerEntityMixin.class, TrackedDataHandlerRegistry.INTEGER);
}
protected IllusionerEntityMixin(
EntityType<? extends SpellcastingIllagerEntity> entityType,
World world
) {
super(entityType, world);
this.friendsandfoes_illusioner = null;
}
@Inject(
method = "initDataTracker",
at = @At("TAIL")
)
public void friendsandfoes_initDataTracker(CallbackInfo ci) {
if (FriendsAndFoes.getConfig().enableIllusioner) {
this.dataTracker.startTracking(IS_ILLUSION, false);
this.dataTracker.startTracking(WAS_ATTACKED, false);
this.dataTracker.startTracking(TICKS_UNTIL_DESPAWN, 0);
this.dataTracker.startTracking(TICKS_UNTIL_CAN_CREATE_ILLUSIONS, 0);
}
}

This is especially a problem in large modpacks where many mods are doing this. Example is TNPLimitless 7 where there's many mods injecting SynchedEntityData onto entities and are getting out of order due to mod load order. Which is causes a lot of reports of people getting disconnected from server due to:
image

commented

Should be resolved with 2.0.0 mod release.