Sweenus / SimplySkills

Other
9 stars 15 forks source link

Suggestion: Make tree use instead ranged_weapon:damage instead of puffish_skills:player.ranged_damage #79

Open Partonetrain opened 2 months ago

Partonetrain commented 2 months ago

I suggest the tree use the Attribute from Ranged Weapon API. This can be done with datapacks, but considering SimplySkill's inbuilt support for RPG series mods, it would make sense to use the attributes provided by them instead. This would require an additional dependency, however.

In fact, is there a list anywhere of all attributes being used?

Sweenus commented 2 months ago

Hello 👋

The issue is that having the skill tree use the RWA attribute, means adding yet another dependency to the mod. There are already a lot, so I'd prefer to steer away from this where possible. It also ends up being restrictive for users that might not want to use RWA or the Archers mod.

If the skill trees weren't handled via json files interpreted by Pufferfish Skills, then it would be possible for me to add conditional behaviour, but alas, this is not possible.

I need to take a closer look at how Pufferfish Skills' ranged damage attribute works. It's possible that it still affects ranged damage alongside the RWA attribute - in which case, it's effectively behaving the same way, no matter which attribute is used. If they are inherently incompatible, I may need to add a hidden compat layer that converts one attribute to the other.

There is already some similar hidden logic that causes active ranged abilities, such as Rapid Fire, to use the RWA attribute when it's present, and Pufferfish Skills' attribute when it's not.

In regards to an attribute list, there isn't one. The best way to check is to look through the skill tree definition files.

Partonetrain commented 2 months ago

In my testing, they are compatible, but don't necessarily scale in the same way (further testing required).

Something strange that I noticed about the puffish_skills version is that players don't have any value in the attribute by default, making /attribute @p puffish_skills:player.ranged_damage get always return 0, even with the skills (which are always multiply base, since 0*x=0), but the attributes still seem to work These are apparently "dynamic attributes" which are intended to work differently than vanilla

In any case, if this is a non-issue, then I will just make the datapack. Cheers!