ItemAffix Overhaul

[CollinHerber]

There are a few things we want to change with affixes to give more control and variation in the items dropping.

Affixes now have a tier system and no longer are based on Item Level

Affixes now have tiers that become "unlocked" with the item level of the item. This is to mimic how PoE item system works. An example of this is: Tier 1 - Min Item Level 1 = Value Roll Range 1-4 Tier 2 - Min Item Level 26 = Value Roll Range 4-7 Tier 3 - Min Item Level 51 = Value Roll Range 8-12 Tier 4 - Min Item Level 76 = Value Roll Range 13-16

If an item drop at Item Level 70 then the item can roll Tier 1 , 2 OR 3.

These tiers will also have a weight system. So if the weights are 1-25 = 100 26-50 = 100 51-75 = 50 76-100 = 50 and the item drops at item level 90 it will have a 2/6 chance to be tier 1, 2/6 chance to be tier 2, 1/6 chance to be tier 3, 1/6 chance to be tier 4.

Affixes are now data driven

Instead of defining everything OOP we want to migrate these affixes to a JSON data structure so they can be easily updated and exported for visual purposes. They can also be added to a DB for live service if we wanted to at some point as well.

This will include adding in a Registry to load and fetch the affixes.

It will end up looking something like

    internal class AddedKnockbackItemAffix : ItemAffix
    {
        public override string InternalIdentifier => "Knockback";
    }

{
    "affix": "Knockback",
    "type": "Additive",
    "equipType": "Weapon",
    "tiers": [
         {
               "itemLevel": 1,
               "value": {
                   "min": 1,
                   "max": 2,
               }
         },
         {
               "itemLevel": 20,
               "value": {
                   "min": 3,
                   "max": 4,
               }
         }
    ]
}

For now I'd like to keep the class for the affix itself so we can potentially tie additional functionality, logic, etc to the affix itself incase we need it. This will also be useful for setting a SPECIFIC affix to an item with it's tier.

For example if I want my Fire Starter to have Tier 3 of Knockback then I should be able to set that in OOP

[GabeHasWon]

Are the tiers here consistent (i.e. every affix has those listed ranges), are the weights consistent (although maybe not necessarily final), and would this be using the node tree editor that passives have but for skills? That'd be the easiest way to edit since doing it in the data or - god forbid - in the code itself would be pretty miserable.

[benj7126]

How would you use a node tree editor for this..?

[CollinHerber]

Are the tiers here consistent (i.e. every affix has those listed ranges)

No. The tier will only have a itemLevelRequirement and anything that is above the itemLevelRequirement can roll that tier. Tiers will not be consistent across affixes and some affixes may only start to appear after itemLevel 50 for example.

and would this be using the node tree editor that passives have but for skills?

We could build an editor for this if we wanted, however something like LOVE would probably be overkill and just a simple table editor written in JS or something that can be run in the web would be a lot easier. I can make that once we finish the json structure itself.

[GabeHasWon]

Honestly just forgot what issue I was on and thought this was the skill tree issue. Don't mind the node editor comment lol

I still don't think I understand the tiering system - a pool of tiers is built that then has certain affixes of certain value ranges it can roll?

[CollinHerber]

It may help to look at the existing PoeDB for how they handle affixes as this is what we are mirroring. https://poedb.tw/us/Claws#ModifiersCalc These are all of the affixes that can roll on claws specifically.

Click on any of those affixes and you will have a dialog pop up. The first number from the left is the item level requirement for that tier.

Something like (0.2–0.4)% of Physical Attack Damage Leeched as Mana doesn't start until Item Level 50. So you will NOT see that affix ever on an item until you drop a claw at item level 50 or above.

[GabeHasWon]

Alright. I think I get it then, I can do this in a bit, though I might need a bit more clarification here and there.

[CollinHerber]

Happy to answer any questions you have. It's a complex system but ends up in having some of the most diverse and unique weapon acquisitions across any games I've ever played.

[GabeHasWon]

Oh, actually - is there a reason there is an InternalIdentifier? We could just use the class's name and save on boilerplate, and it doesn't look like there is anything InternalIdentifier does apart from be a name to use.

[CollinHerber]

Oh, actually - is there a reason there is an InternalIdentifier? We could just use the class's name and save on boilerplate, and it doesn't look like there is anything InternalIdentifier does apart from be a name to use.

Other than it being a "nicer" name , nope. I can't think of any reason why we couldn't just use the class name

[GabeHasWon]

Is this only for ItemAffixes? MobAffixes could be part of this system but I'm unsure if that's necessary or how much data they'd actually share.

[CollinHerber]

This is focused on Item Affixes, I will update ticket title

[GabeHasWon]

Affixes only have the one data entry, right? I wouldn't need to define two of the same affixes? Just wanna make sure.

[CollinHerber]

I don't see why we would need two entries. So yeah 1 entry should be good

[GabeHasWon]

Most of this is good to go; check out the branch here: https://github.com/Path-of-Terraria/PathOfTerraria/tree/data-driven-affixes An individual affix looks like this:

    {
        "affixType": "ChanceToApplyOnFireGearAffix",
        "equipTypes": "Ring",
        "tierDatas": [
            {
                "minValue": 1,
                "maxValue": 4,
                "minimumLevel": 1
            },
            {
                "minValue": 5,
                "maxValue": 8,
                "minimumLevel": 21
            },
            {
                "minValue": 9,
                "maxValue": 12,
                "minimumLevel": 52
            },
            {
                "minValue": 13,
                "maxValue": 16,
                "minimumLevel": 73
            }
        ]
    }

Any thoughts? I think I got all of the necessary data down, at least for now. Loading works and you can retrieve this data from any Affix instance if desired, though it's not implemented much yet.

[CollinHerber]

I think tierDatas is a bit redudant to include datas.. Maybe just tiers for that entry

Also we will want to make sure we have a requiredInfluences as well for affixes that can only roll on certain influenced gear

[GabeHasWon]

What's the difference between tierDatas and tiers?

[CollinHerber]

What's the difference between tierDatas and tiers?

Nothing, I'm just suggesting a different name. I think including the word datas in a property name is redundant

[GabeHasWon]

Alright, renamed them to just tiers.

Now, though, I am not sure what to do. The Affix system is a bit labyrinthian, so I'm not confident in saying I could implement the next steps properly. I think the tier of an Affix is a result of the item it's rolled on, based on MinDropItemLevel, and I think this would be done in Affix.Roll, but I'm not too sure.

[CollinHerber]

What are the next steps you need to implement? Like what is missing here? I would imagine the first step is to choose the affixes that drop on the item. After the affixes are chosen, then the tier of the affix is chosen based on the available tiers for the item level and weighing system.

Once the tier is chosen then a value is chosen from the range in that tier. Then assigning that value to the affix.

[GabeHasWon]

What's next is applying affixes to items with the new system. I just can't wrap my head around the system we have so I'm not confident in modifying it for the new system. I understand the gist, but I can't tell what'd be the best place to put/move/remove stuff.

[CollinHerber]

I added some things to the branch, and theoretically I got this working? I unfortunately have to leave now just as I was about to enter Testing mode. Figured I'd just mention it though. Plan to pick this up in the morning

[GabeHasWon]

Looks good! Definitely nice having someone else do what you've done, just looked over the commits and it looks nice and readable.

[benj7126]

The whole point in this was that we would end up with the classes (i still think letting them not exist is better but) being like:

    internal class AddedKnockbackItemAffix : ItemAffix
    {
        public override string InternalIdentifier => "Knockback";
    }

And the branch dosen't seem like that...

_ Or rather the point in having more things in the .json file than just "tiers", that is.

[GabeHasWon]

I don't think the json will ever hold the functionality of the affix, since that'd be a ton to do. InternalIdentifier was also rejected because we just use the class's name, which is a pre-built identifier necessary to access it anyway. The affix's weapon types, influences and tiers are all in the json now, leaving most affixes to be

internal class SomeAffix : ItemAffix
{
    public override void ApplyAffix(Player player)
    {
        player.stat += value;
    }
}

Which is either very close to or just the minimum size that an affix can be. And per the original post, keeping them as classes also means you can apply them to stuff like uniques or implicits super easily, instead of creating a pseudocode interpreter to do the same job from json data in C#.

[benj7126]

Well... as i came up with this idea i also had an idea of how to remove the ApplyAffixes from that based on the json presented in the original post.

I was thinking that we would link up the "affix": "Knockback", with the EntityModifier, as we have reflections and the Knockback stat is just called "Knockback", but given how we began using the class name, instead of the effect, we could only get this by writing the effect, public string Effect => Knockback in the class but at that point it seems a little like too much of a stretch, ig...

But alas (it wouldn't be as cool to do either as we still want the class :/ | and they both probably work more or less well enough / about the same)

[GabeHasWon]

Yeah, that would be doable and fine but ultimately that's just a preference thing, I don't think either way is substantially better than the other - OOP means we have a lot more fine control over exactly what happens, but entirely data driven would be a little easier to make.

[benj7126]

Yeah, though my thought process was that we would need to add all affixes together in the EntityModifier no matter what; as if we wanted to give +10 damage / sec to enemies within some distance of the player, we could maby have an "Update" in the affixes, but that would be weird if you stacked them as it would trigger all the damages individually and if you somehow managed to get a lot of that type of affix it would probably look dumb (the ¨10 damage numbers every second).

But again, either works and we seem to be doing it like this so this is how it is. (this = that branch) - the only reason why i would care is because it sounds like something that would be fun to make, not because it would benefit the game at all, btw. (only slightly salty)

[GabeHasWon]

(the ¨10 damage numbers every second).

This would be easily consolidated into a helper class that reduces popup text spam, since that's fairly easy to set up.