[π]: [1.20.1] Mixin injecting SynchedEntityData into entities not owned by Supplementaries is dangerous
TelepathicGrunt opened this issue Β· 6 comments
Version-Loader
1.20.1-forge
Issue Detail
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.
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:
yeah was aware of this...
tho never had reports of issues happening
Keep telling that forge should make this safe with some extra syncing. Anyways guess ill do some networking
So in your case, you need the data on client. Do what I do for my horse owner uuid syncing workaround. Mixin to client. When entity spawns on client, send packet to server asking for data. Server then gets the needed data and sends it back to client. Client then holds the data
Only 1 mixin and whatever system you want for attaching that data to the entity. Whether that be cap or to the entityβs nbt directly or to a normal field mixin added to entity that is not an EntityDataAccessor
and synced entity data isnt even used here to store data here, that is done with the save methdods which are arguably way easier to use than capabilities. Capabilities still dont offer any automatic syncing behaviors either for entities.
Reason synced data is so useful is that it automatically gets sent in the spawn packet. Here i'm setting some data after an entitty has spawned in finalize spawn. by there the entity isnt even available on client so i cant even send a packet there. I'm also pretty sure the same would apply tothe spawn entity event and if thats the case i dont really know how to replace this other than some huge mixin mess
im checking them now. Data is just visial so not critical so such a delay introduced by client asking for data shouldnt be a problem
I have not checked out the entity spawn events but if those fire on client, it could replace the mixin I suggested