Reincarnation

[tdauth]

Hi again, I am trying to implement Reincarnation now as hero ability as well as item ability. My basic idea is to hide and pause the hero and make the hero invulnerable until he/she is revived. To prevent actualy death I immediately set life above 0 and restore mana if necessary.

I have the following issues:

• Added onBeforeDeath since I do not know when to handle it before the unit dies.
• How to keep a target spell effect and remove it when the reincarnation actually happens.
• Not sure about the order ID.
• Should I check if it is actually a hero? What happens in original Warcraft?
• I immediately hide and pause the hero. I think in original Warcraft it shows some death animation but I think you cannot click on the hero icon anymore?

This whole pull request is unfinished but maybe you can give me some hints/feedback, so maybe in the future I can implement more abilities which are missing.

[tdauth]

Ah I think I have to rebase again. There are some changes in CUnit about the bounty etc. which have nothing to do with Reincarnation. Actually, I only added the beforeDeath calls to CUnit.

[Retera]

Seems cool! If memory serves, units like the Ancient Wendigo that have Reincarnation as a unit will actually do a hero dissipate on death before reincarnating, rather than their normal decay animation. We could try to add that behavior if we want, but it's an interesting case of something being a little hacky. It almost seems like it might be the result of some copy pasted code back in 2002 moreso than an intentional feature. Maybe along with the "death type" options (raise/can't raise, or decay/doesn't decay) we could put an engine-level "dissipate" option for the death type as a field on the unit type or something. Then again, the way that I store abilities to the unit and not the type (to allow unit instance behaviors) might make what I'm suggesting less of a good idea in the current code.

As a suggestion from looking at your code the first time through, I noticed that you were confused by my ability API, which is quite understandable. Originally I had this idea of making everything performant by putting it in a Java class at every level of caching -- AbilityType for the UnitType, and Ability for the Unit. And then I wanted a third one that defined all AbilityTypes who were copied from the same thing in world editor, so those were called AbilityTypeDefinition.

But in practice it became too complex so more recently I had created a type "CAbilitySpell" that had a "populateData" command and skipped all of that stuff, opting to have easier-to-write code instead of performance. On a high level, if you make your Reincarnation ability extend "CAbilitySpell" (or in your case the NoTarget abstract subclass with some default settings) then because you implemented the "populateData(...)" method for how to process gaining or leveling up the skill, it is no longer necessary to have an AbilityType nor an AbilityTypeDefinition for your class. It looks like maybe you saw this, that you can instantiate an CAbilityTypeDefinitionSpellBase for your Reincarnation skill instead of needing to create an instance of your own AbilityTypeDefinitionReincarnation.

So, with that in mind, it's probably safe to delete your AbilityTypeDefinitionReincarnation class, unless you were planning to change to that "possibly more performant but waste of time to develop" API that it is using.

[Retera]

https://www.youtube.com/watch?v=U9IJDkda7ig

Maybe this YouTube video from last year might help? It's possible that I could make a better one in the future, but this was what I recorded for a guy last time I tried to help with how Warsmash abilities work. (Back then we didn't have CAbilitySpell to try to make things easier, though.)

[Retera]

Reincarnation was included as a user-replaceable (i.e. modifiable in custom maps in the future) behavior defined in JSON by the ability builder pull request merged from @Glasislundr here: https://github.com/Retera/WarsmashModEngine/blob/4298a9010119dcb1c19fe087fcf79a19884da12e/core/assets/abilityBehaviors/generalPassives.json#L476

Accordingly, I am going to close this pull request because (1) we do not need a built-in ability solution for Reincarnation if we have a user replaceable definition external to the engine, and (2) even if we wanted a built-in solution, the one provided here was using an active ability base class instead of a passive ability base class. As I recall, the JSON definition of Reincarnation is able to show its cooldown while also inheriting passive functionalities, including an unclickable icon.

Feel free to reach out if you have more questions.