ElenaiDev / ElenaiDodge2.0

Other
9 stars 9 forks source link

Feather Regeneration Attribute #17

Closed Jackiecrazy closed 3 years ago

Jackiecrazy commented 3 years ago

The formula should be correct.

ElenaiDev commented 3 years ago

Thanks so much for the PR! I really appreciate you taking the time to do this. I do have two quick questions before merging as I am not (too) experienced with Mixins or Attributes.

Thank you so much again!

Jackiecrazy commented 3 years ago
  1. Gah! I forgot to change that bit! It should be elenaidodge2.refmap.json. Apparently there's a way to register attributes to players without resorting to mixins, but I haven't heard of it yet. Forge itself basically coremods swim speed and reach distance in...
  2. Indeed, and don't forget to actually call getAttributeValue when needed; I always forget. I made some silly presumptions in the implementation which will cause it to crash on load, pushing a commit right now to make it fully working. There may be a way to optimize the lookup of attribute values, but it's 1AM and whatever it may be currently eludes me; getAttributeValue shouldn't be particularly expensive, judging by how it's called a few times per render tick in the client HUD. Another thing is that adding attribute modifiers to itemstacks will cause them to be written in the tooltips. It can be suppressed, but may be annoying if you're looking to shift steel feathers entirely to attributes. In any case, it seems like ItemAttributeModifierEvent is where you would adjust attributes for other items; looks like I won't have to resort to extreme ends to implement absorption, deflection and shatter on existing items, which is a relief. Despite the weird requirement of mixing in custom attributes, it's now a surprisingly flexible system.
ElenaiDev commented 3 years ago

Thanks so much for explaining this! This seems to be a much better method of implementation than creating a bunch of Capabilities like I usually do.

I'll do some extensive testing tomorrow and then I'll merge! Thank you so much for contributing I really appreciate it, if there's anything I can do to help/thank you please let me know :)

Jackiecrazy commented 3 years ago

I found the new forge way of doing it. Listening to EntityAttributeModificationEvent will allow you to add to the attribute lists of any entity. It's a fairly recent addition, so it might not be backwards compatible, but it provides an "official" way of going around things. Caelus still does mixins, for one.

ElenaiDev commented 3 years ago

Thanks for finding that! I'll have a little play around with it and see which I prefer.

I've always been a little cautious with using Mixins, mainly because I haven't learned how to best use them yet but also because they may cause unpredictable issues with Forge updates. EDIT: Currently Researching this properly, it seems pretty reliable in Elenai Dodge 2's case as it doesn't do anything too risky with Mixins.

Your implementation of attributes is very nice though so I think I'll use it, thanks so much again for that.