Add InventoryEffectItem class

[OhmV-IR]

Abstracts away the ugly scheduler implementation of a simple inventory effect item. Note that base will still use the talisman class for the talismans because this one doesn't have the concept of levels to make it more generally applicable

[LordIdra]

I don't think an abstraction like this should be in core. This seems like it would be more fitting for a Talisman base class in pylon-base. Any other thoughts?

[LordIdra]

don't think this needs to be weak

[JustAHuman-xD]

Should note in doc that there is no guarantee that this current pylon item instance is up to date with the itemstack when it runs

[LordIdra]

I think a better abstraction could be made here: instead of 'effects' jst have this as on removed from inventory, and on added to inventory. will need to document that on remove is a best-effort, might be called after player leaves or smth

[Seggan]

I don't think an abstraction like this should be in core. This seems like it would be more fitting for a Talisman base class in pylon-base. Any other thoughts?

It's not just for talismans tho, its for any item that has an effect in an inventory, such as the bloom damaging you if you hold it without tongs

[LordIdra]

we discussed this in VC later hence why I made another comment contradicting that one

[Seggan]

I think a better abstraction could be made here: instead of 'effects' jst have this as on removed from inventory, and on added to inventory. will need to document that on remove is a best-effort, might be called after player leaves or smth

Such an abstraction would be unapplicable to my earlier example of the bloom

[LordIdra]

Wait I'm confused. How? It's effectively exactly the same class with the methods renamed

[Seggan]

AFAICT currently applyEffect is performed multiple times as long as the item is in the inventory, so with this I can damage the player every tick

[LordIdra]

AFAICT currently applyEffect is performed multiple times as long as the item is in the inventory, so with this I can damage the player every tick

Fair. If there's no way to make this an on inventory add/remove thing (which I don't think there is?) happy to just go with this, though I do think it should be made an interface not a class

[OhmV-IR]

AFAICT currently applyEffect is performed multiple times as long as the item is in the inventory, so with this I can damage the player every tick

what? doesn't because onAddedToInventory is only called if pdc doesn't already have the effect key. otherwise it just postpones the cancel task again. If you wanted to damage the player every x ticks they have it in their inventory, just use PylonInventoryItem.

[OhmV-IR]

I think a better abstraction could be made here: instead of 'effects' jst have this as on removed from inventory, and on added to inventory. will need to document that on remove is a best-effort, might be called after player leaves or smth

renamed the stuff, but kept the InventoryEffectName as it's still accurate and it sounds better than InventoryOnAddedOrRemovedItem

[LordIdra]

I think a better abstraction could be made here: instead of 'effects' jst have this as on removed from inventory, and on added to inventory. will need to document that on remove is a best-effort, might be called after player leaves or smth

renamed the stuff, but kept the InventoryEffectName as it's still accurate and it sounds better than InventoryOnAddedOrRemovedItem

Hold on so just to clarify, the new names are accurate and Seggan is incorrect that the added method would be called every tick?

Alternative name suggestion: InventoryTrackedItem

[OhmV-IR]

I think a better abstraction could be made here: instead of 'effects' jst have this as on removed from inventory, and on added to inventory. will need to document that on remove is a best-effort, might be called after player leaves or smth

renamed the stuff, but kept the InventoryEffectName as it's still accurate and it sounds better than InventoryOnAddedOrRemovedItem

Hold on so just to clarify, the new names are accurate and Seggan is incorrect that the added method would be called every tick?

Alternative name suggestion: InventoryTrackedItem

yes seggan is incorrect about the added method being called every tick. InventoryTrackedItem sounds okay, but it's not really better than InventoryTrackedItem so I'll just keep the current name out of laziness, but if you think it's better, I can use it

[OhmV-IR]

not sure why tests failing, they passed on my machine, restarting them in CI and we'll see if that works

[Seggan]

what? doesn't because onAddedToInventory is only called if pdc doesn't already have the effect key. otherwise it just postpones the cancel task again. If you wanted to damage the player every x ticks they have it in their inventory, just use PylonInventoryItem.

Sorry thought this was gonna replace PylonInventoryItem/whatever we are calling it now