WrappedAbility Timestamp issue

[Hanmac]

Problem with Trigger and Timestamps:

https://github.com/Card-Forge/forge/blob/93a52a4263538910543ad222f20d056a2bcfb388/forge-game/src/main/java/forge/game/trigger/WrappedAbility.java#L503-L522

WrappedAbility does mess with the Triggered Objects, but the better way would be to have the Effects handle it

currently the way is to update the Effects with equalsGameTimestamp to check if the timestamp is still OK or use LKI instead for this, it might be better if the Triggered Objects where LKI to begin with so they can't be messed up again?

it would be better if all effects are updated, and the Part could be removed from WrappedAbility

[tool4ever]

Yea there should be no reason to keep it: Because of Defined$ Self etc. effects need their own checks anyway

For the most part only changezone triggers should be able to find the new object anyway?

• there we already need the scripter to decide if he needs Card (LKI) or NewCard
• imo ideally we could remove the *LKICopy parts = never call getCardState() (it's already inconsistent since it never gets applied to CardCollections)

[Hanmac]

@tool4ever yeah that is my end goal that we don't need "LKICopy" anymore, and have the Effect handle all the stuff

[tool4ever]

A simple example where things get messed up currently:

• Shelob, Child of Ungoliant trigger on the stack
• return the dead creature to the battlefield
• no token 💩

but if we can figure out game timestamps now porting those other changes should lead to some quick wins :)

[tool4ever]

Needs #117

[Hanmac]

@tool4ever what exactly is still missing? i thought we ported most of that MR into smaller ones?

[tool4ever]

until noTimestampCheck contains all API bugs like the one above can still happen

[Hanmac]

until noTimestampCheck contains all API bugs like the one above can still happen

oh okay that's what you mean, yeah we should make smaller MR to port all the APIs if able (and when finished, remove the noTimestampCheck)