Don't destroy TP/Identification scroll unless it is properly used

[MerleMoqueur]

Current behavior:

• Right click on Identification/Town Portal scroll
• Scroll is destroyed
• Targeting cursor appears
• Maybe really use the scroll (if the action cancelled for any reason, the scroll is lost)

Better behavior (as seen in Diablo 2):

• Right click on scroll
• Targeting cursor appears
• Use the scroll
• Scroll is destroyed

Both are consistent, but the first one is frustrating and isn't useful to the game.

[julealgon]

Slightly related:

• https://github.com/diasurgical/devilutionX/issues/792
• https://github.com/diasurgical/devilutionX/issues/2844

As far as I recall, when the hit-frame for the animation happens, the information about which item caused the effect is not present. We'd need to provide the item as a parameter so that it can be tracked down for destruction at that point in time instead of when originally issuing the command.

(TL;DR I don't think this is very trivial to do)

[kphoenix137]

Current behavior:

* Right click on Identification/Town Portal scroll

* Scroll is destroyed

* Targeting cursor appears

* Maybe really use the scroll (if the action cancelled for any reason, the scroll is lost)

Better behavior (as seen in Diablo 2):

* Right click on scroll

* Targeting cursor appears

* Use the scroll

* Scroll is destroyed

Both are consistent, but the first one is frustrating and isn't useful to the game.

Fixed, check linked PR

[kphoenix137]

Current behavior:

* Right click on Identification/Town Portal scroll

* Scroll is destroyed

* Targeting cursor appears

* Maybe really use the scroll (if the action cancelled for any reason, the scroll is lost)

Better behavior (as seen in Diablo 2):

* Right click on scroll

* Targeting cursor appears

* Use the scroll

* Scroll is destroyed

Both are consistent, but the first one is frustrating and isn't useful to the game.

Fixed, check linked PR

My analysis was completely incorrect. It appears I completely misunderstood the game mechanics before attempting to fix this issue. It appears that there are only 2 scrolls that will be consumed immediately and give you a cast animation before giving you a spell cursor: Resurrect and Identify. The rest of the scrolls will give you a cursor first, and then consume the scroll. As far as spells go, only Heal Other and Telekinesis give you a spell cursor, and they first initiate the cast animation and consume mana, and then give you a spell cursor. Considering spells have a chance to fail/not be used, regardless of if they are scrolls, staves or spells, to me it would make more sense to streamline how all spells that give targeting cursors work and give the targeting cursor first, and then enter a cast animation and consume resources. I suppose this would be up for debate though.