Lemonszz / Biome-Makeover

MIT License
28 stars 34 forks source link

Several unsafe practices in HorseMixin #252

Open TropheusJ opened 10 months ago

TropheusJ commented 10 months ago

HorseMixin

I was originally just reporting the tracked data handler, but it's always good to improve compatibility and safety.

  1. adds a new tracked data handler This is extremely unsafe and prone to load-order dependent issues that are impossible to debug. You should instead add a regular field to the mixin, and sync it manually with a custom packet.

  2. Overrides removeWhenFarAway Method overrides in mixins are worse than an @Overwrite, because it's the same, but silent. You should instead inject to the parent class and check instanceof AbstractHorse.

  3. cowboySpawned not marked as @Unique This means your field may conflict with and overwrite/be overwritten by another mod's. It also makes your mod harder to track down if it ends up in an error. Unique fields will be prefixed with the mod ID automatically.

  4. methods in HorseHat are not prefixed These method overrides may conflict with methods from another mod. These can't be marked Unique because they need to match the names in the interface. This is usually resolved my prefixing injected interface method names with your mod ID. ex. biome_makeover$hasHat

2, 3, and 4 likely also apply to other mixins, but HorseMixin is all I've looked at.