Sollace / Unicopia

The pony powers mod to power in your pony pony pony
https://modrinth.com/mod/unicopia
56 stars 22 forks source link

Server Log Spamming #14

Closed fin600 closed 2 months ago

fin600 commented 5 years ago

Server thread/WARN [minecraft/EntityDataManager]: defineId called for: class net.minecraft.entity.monster.EntityWitch from class com.minelittlepony.unicopia.player.EntityCapabilities

Every single mob that spawns makes a message similar to this, why?

Sollace commented 5 years ago

This is forge's fault. You should create an issue with them, because there is really no other way to add tracked values for entities.

public static <T> DataParameter<T> createKey(Class <? extends Entity > clazz, DataSerializer<T> serializer)
{
    if (true || LOGGER.isDebugEnabled()) //Forge: This is very useful for mods that register keys on classes that are not their own
    {
        try
        {
            Class<?> oclass = Class.forName(Thread.currentThread().getStackTrace()[2].getClassName());

            if (!oclass.equals(clazz))
            {
                //Forge: log at warn, mods should not add to classes that they don't own, and only add stacktrace when in debug is enabled as it is mostly not needed and consumes time
                if (LOGGER.isDebugEnabled()) LOGGER.warn("defineId called for: {} from {}", clazz, oclass, new RuntimeException());
                else LOGGER.warn("defineId called for: {} from {}", clazz, oclass);
            }
        }
        catch (ClassNotFoundException var5)
        {
            ;
        }
    }
unilock commented 4 months ago

Years later, this is unfortunately still an issue, though now much more difficult to diagnose...

TrackedData fields should never be registered to an existing entity via mixin.
The reason being: if two mods that do such a thing to the same entity are loaded in a different order on the client vs. the server (which isn't uncommon on Fabric), the DataTrackers on each side will be expecting the fields to be in a different order, thus causing desyncs between the TrackedData values and types.

See here for more information, here for a mod that automatically detects this issue (and similar issues), and here for an example of how to do the same thing safely using Cardinal Components API.

Sollace commented 4 months ago

I have not see this issue in large modpacks myself, although I can see how that might become a problem in other situations. I don't really like Cardinal Components as a solution, however. I think it has its own problems.

Changing to use a separate packet should be doable here, especially since I've wanted to take another look at spell synching for a while which has been limited by the lack of the ability to register my own data types with the vanilla system (it instead piggybacks off of the nbt type)

Sollace commented 4 months ago

Linking this with https://github.com/FabricMC/fabric/pull/3584 as a possible solution (if/when it is merged)

unilock commented 4 months ago

I have not see this issue in large modpacks myself...

I've seen it happen much more frequently with Quilt rather than Fabric, for whatever reason. (Quilt is mostly just a fork of Fabric, however...?)
Also note that it only occurs when a client connects to a remote server (being either a dedicated server or another client with their world open to LAN); singleplayer worlds shouldn't have this problem.

Maybe Fabric's new Data Attachment API (https://github.com/FabricMC/fabric/pull/3476) could be of use here?

Sollace commented 2 months ago

This was fixed but not closed. bah