Relic_data does not update after a transform

[foulman]

Any item that uses relic_data to grant powers or attributes or even item based attack activations will not change after a transform is done on the item to change it.

Steps To Reproduce

1. Download file, it contains a 3 item transform. One is a strength boost, one is a dex boost, one is no relic data at all.
2. debug in one of the rings, note the item id you want is xring_.
3. Once you get the ring, using the transform will not change the function of the ring, it will be stuck giving the bonuses of the item when you first picked it up. This also applies if you drop the item and pick it up again in a different state, drop it and save/reload, etc.
   This also seems to apply to items with on-hit powers, presumably the entire range of relic abilities are affected.

file: relic-ring-test.zip

As relic_data development continues, as it currently is, this will become a bigger problem, especially when recharging (ARTC_TIME and the like) comes into play.

Expected behavior

Items that undergo a transform should update themselves and behave as indicated in the JSON, as old artifact_data items do.

Screenshots

Versions and configuration

Additional context

I am aware of #38262 but am unsure if that would address this issue, or was intended to address this type of issue, therefore I'm posting this anyway so that at least a report has been made.

[KorGgenT]

the linked pr was specifically to change how data's stored; i don't think it will affect this particular issue at all.

[KorGgenT]

So I've been thinking about this for a while and I've still got a conundrum:

My current plan for artifacts (it's obviously not done yet, but it shouldn't be too far behind nested containers) is to make "artifacts" able to just be regular items. This would mean that for example, we could have an item with a transform. So, if the item transforms, we don't want it to lose the artifact-ness that the game randomly assigned, instead of was from the itype of the item, but I definitely see that there is some amount of benefit for modding for transforming artifacts.

[foulman]

So I've been thinking about this for a while and I've still got a conundrum:

My current plan for artifacts (it's obviously not done yet, but it shouldn't be too far behind nested containers) is to make "artifacts" able to just be regular items. This would mean that for example, we could have an item with a transform. So, if the item transforms, we don't want it to lose the artifact-ness that the game randomly assigned, instead of was from the itype of the item, but I definitely see that there is some amount of benefit for modding for transforming artifacts.

We didn't see any problem with that being inherited, I think that's expected in fact, much like a lot of other values are retained, but we also tried specifying 'empty' relic_data and alternative relic_data and neither of those worked. That would, I believe, be the best way for it to work, have it inherit (stay the same) after a transform unless it's specified that it should change or be stopped entirely.

Or as stated by another interested party which I agree with:

I'd say that there should ideally be both the ability to transform a relic's properties entirely AND to inherit it via transform, both would be useful. Perhaps make it so that it:

1. Double-checks relic_data when it transforms.
2. Interprets a deliberately empty but present relic_data block as "clear the relic data"
3. Interprets a different block of relic data as "change properties to this relic setup instead"
4. Interprets complete absence of a relic data block as "inherit last relic state defined"

That said, updating relic data after a transform, and assigning relic data to both states of the item if inheritance is desired, would be workable too. Transforming generated artifacts will have to explicitly be build in someway anyway, so having to tell it outright to use the same relic data for both states could be baked into that.

[KorGgenT]

So interpreting the relic_data blocks in json as something implicit is right out. A flag could be added i suppose, that tells the item to overwrite the artifact data? It's really not my favorite idea. I'll have to think on the possibilities of comparing the enchantments with the enchantments from the itype and inherit that way

[foulman]

There's just a lot of functionality lost that currently exists with artifacts if there's no way to switch things on and off or between different "sets". That would be a loss to functionality in not only magiclysm but also to things aftershock would probably want available, so some sort of override flag would be good.

[KorGgenT]

I'll have to think on the possibilities of comparing the enchantments with the enchantments from the itype and inherit that way

was this unclear? an override flag is one way to do it, yes, but i think there might be a more elegant solution by comparing the enchantment on the item to its itype when it transforms, and inherit the part that is not on the itype.

[foulman]

Yes, it was a little unclear as you dropped into cataclysm c++ speak for some unknown reason.
How elegant it ultimately is is entirely up to you, I was merely restating the need for it, I beg your forgiveness.

[KorGgenT]

you dropped into cataclysm c++ speak for some unknown reason

To be more clear, an itype is the item as it is in json. It is always the same, and it is unchangeable. There's no extra data to it other than the stuff that's in json. An item, at least in terms of cdda, has lots of other data attached to it such as contents and the rot timer. I wanted to be especially clear so i used the term itype to mean that the enchantment attached to the json is there always, as opposed to an enchantment attached to an item from some other way (i.e. the procgen that i want to do eventually)

[foulman]

you dropped into cataclysm c++ speak for some unknown reason

To be more clear, an itype is the item as it is in json. It is always the same, and it is unchangeable. There's no extra data to it other than the stuff that's in json. An item, at least in terms of cdda, has lots of other data attached to it such as contents and the rot timer. I wanted to be especially clear so i used the term itype to mean that the enchantment attached to the json is there always, as opposed to an enchantment attached to an item from some other way (i.e. the procgen that i want to do eventually)

Yes that is much more clear, thank you.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not 'bump' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

[Kenan2000]

Kinda hoped to see if anyone's found any likely cause in the code of this. In addition to the above concerns with the future use of this feature in Magiclysm, the impact it will have on conversion of artifacts, and in the OP's own projects, I recently noticed that Cataclysm++ actively makes use of transforming artifacts as well

[KorGgenT]

i haven't picked the artifact project back up as i've been busy with nested containers.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not 'bump' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

[Kenan2000]

I and other modders that have to face this issue would be really thankful if you pin this for the 0.F milestone so that it won't keep being marked as stale, can you do this, @KorGgenT ? This is a long-term issue that will need to be fixed once you get to work on the relic overhaul, so it likely shouldn't be flagged as stale before you've had the opportunity to work on that feature 😄

[stale]

This issue has been automatically closed due to lack of activity. This does not mean that we do not value the issue. Feel free to request that it be re-opened if you are going to actively work on it