1.18.2 conflict with polymer/universal graves

[Monster-Zer0]

https://aofpaste.com/woqomijeji.properties

looks like Data Attributes makes a cast without checking

causing universal graves to crash.

[CleverNucleus]

Hi there, I appreciate the report - it is Polymer making the unchecked cast. Polymer adds a lot to server -> client packets to do what it does, so I have a feeling that it is one of these that is conflicting with Data Attributes. If so, it will likely be on Polymer to patch it - regardless, I'll see if I can fix it my end first. Thanks.

[Patbox]

Hi there, I appreciate the report - it is Polymer making the unchecked cast. Polymer adds a lot to server -> client packets to do what it does, so I have a feeling that it is one of these that is conflicting with Data Attributes. If so, it will likely be on Polymer to patch it - regardless, I'll see if I can fix it my end first. Thanks.

Polymer uses most "basic" interface to implement the mocking (MutableWorldProperties). Your mod presumes that every instance of it has your own interface, which isn't the case

[Patbox]

See: https://github.com/CleverNucleus/Data-Attributes/blob/b4a5c09727fbb80cf6ea709637653c50bc6f1996/src/main/java/com/github/clevernucleus/dataattributes/mixin/LivingEntityMixin.java#L33

[CleverNucleus]

Hi @Patbox You're mostly correct, I see that upon first inspection I've not identified the cause correctly. In fairness, I couldn't have expected a new World implementation, and for that implementation to not return either of the standard client/server WorldProperties 😄.

1. Cause

The cause of this crash is two-fold: this was getting passed into the LivingEntity constructor, and my mod assumes that World#getLevelProperties returns either client-side ClientWorld$Properties or server-side ServerWorldProperties. I'd argue that this is actually a reasonable and valid assumption, allow me to explain below:

2. Solution

The solution is mostly obvious and easy for me to implement - I just mixin to the parent MutableWorldProperties such that it implements my MutableIntFlag. However, I'm concerned that this will cause secondary issues:

WorldProperties [Interface]
┗MutableWorldProperties [Interface]
        ┣ClientWorld$Properties [Class]
        ┣FakeWorldProperties [Class]
        ┗ServerWorldProperties [Interface]
                ┣UnmodifiableLevelProperties [Class]
                ┗LevelProperties [Class]

An instance of ServerWorldProperties is always passed into the LivingEntity constructor on the server - this is necessary for the level's NBT data to be read/written (things like weather clear time, difficulty etc.). In addition to this standard NBT data, I add an integer to be read/written. I won't go into details because it is out-of-scope, but when a LivingEntity is created on the server it must have access to the level data and this integer in order for its attributes to be updated and synced properly.

So the question I have is what happens when the server-side LivingEntity constructor no longer receives access to its level (NBT) data? I.e., by not passing an instance of ServerWorldProperties, and instead an instance of the parent MutableWorldProperties, the level data is no longer accessible.

3. Conclusion

Correct me if I am wrong, but you have already fixed this issue here? If so, then what I have discussed above can be considered resolved. I'll still output a release where MutableWorldProperties implements MutableIntFlag, just in case.

Hope that was constructive, and I just want to be clear that I have total respect for you and was not intending any offense. Thanks.

[Patbox]

Only resolved on 1.18.2 (got asked by aof team, my solution isn't perfect either, as it passes nulls and haven't checked if it would still work in newer versions), but keep in mind other mods constructing fake world instances might have the same "issue". Generally speaking custom world implementations not extending ServerWorld or ClientWorld are used for mocking or rendering outside of world. In my case I need it for data filtering to make sure invalid stuff won't get synced to clients. And for solution, quick instanceOf with sane default would work just fine

[CleverNucleus]

Hopefully fixed with latest releases on 1.18.2 and 1.19.2.