Mana Flare v1.5 (passive hero ability)

[Footman16]

This spell seems perfect for an enemy hero in my next campaign love it!

 

[Devalut]

Nice, if you put in the time you can even replace the dummy with special effects.

 

[FUJI]

So it's like creating a unit that will mana flare itself and stays within the hero.
I liked the concept and its simplicity, though you can actually code the mana flare behavior
to be modular somehow and not relying on dummy ability. The dummy will always take
the credit for the damage done by its ability instead of the hero. Imagine using this on
a map that takes care of hero kills or in a map that tracks unit kills. This is critical.

I know this is your start, there's a lot of things to consider, and I'm not discouraging you
to continue further. In fact, these will help you out to improve:
GPAG - GUI Proper Application Guide
Memory Leaks

Yes, leaks are mandatory to be eliminated. At this state, you're leaking tons of locations.
Aside from that, I only pointed out the major ones. By just previewing triggers here. (without testing in-game)
So there could be more to this if you're only interested to update at all.

 

[hemmedo]

The dummy will always take
the credit for the damage done by its ability instead of the hero.

This is the case for many spells in here unfortunately. I hate it

 

[Uncle]

Edit: Nevermind. Misunderstanding.

 

[Bribe]

@JAKEZINC
Good point about the dummy taking the kill credit.

But I just wanted to add that with systems like Bribe's Damage Engine you can change the source of damage to your desired unit.

To do this in GUI I would use a Unit Indexer to link the Dummy to the REAL source of the spell (Your Hero in this case).

DummySource = Unit (Array)

When you create the Dummy for this ability (or any ability that want to use this with), set DummySource[custom value of the dummy] as the Hero/Unit that you want the kill credit to go to.

Then you create a trigger like this (Bribe's Damage Engine Event):

• Events:
• DamageEventAmount becomes equal to 1.00
• Conditions:
• DummySource[custom value of DamageEventSource] not equal to No Unit
• Actions:
• Set DamageEventSource = DummySource[custom value of DamageEventSource]

So this Event runs whenever a unit takes damage. Then it checks if the source of the damage (it would be the Dummy in this case) has a DummySource setup for it. If it doesn't nothing happens. But if it does, the source of the damage is changed from the original unit (Dummy) to it's DummySource (your Hero).

I would like to clarify that changing DamageEventSource does not work with DamageEngine. The only way to truly change the source is to null the original damage instance to 0 and then re-issue the damage from the correct source unit.

 

[Uncle]

Ah, yeah, but it's basically the same effect, right? The Hero deals the damage instead of the Dummy?

 

[Ricardo_Gavarni]

Thanks for your words of encouragement, everyone! Yes, this ability is a good fit for boss-type enemies.

JAKEZINC, I'll definitely check the tutorials you linked.

Do you have any tips on making importing easier? Currently, the user is required to copy two abilities, one unit, and icons (in addition to triggers). Also, rebalancing the ability seems cumbersome, since you need to update text descriptions manually after changing the numbers.

(EDIT: I think I got it, one could just define some constants, then use Ability - Set Ability Tooltip, and just do all concatenation needed)

Is the "kill-stealing" thing a big deal? For example, the leaderboard of Sunken City tracks kills by the player, not by the hero (to account for summons). It is important for things like lifesteal, I guess.

 

[Bribe]

Ah, yeah, but it's basically the same effect, right? The Hero deals the damage instead of the Dummy?

Damage Engine won't even recognize your change to the source unit in many cases (at least not the latest one), and even then you'd just be changing a variable rather than changing the actual mechanics of who dealt the damage.

 

[Uncle]

Weird, I remember fixing this issue in my map... The kills were credited to the Hero instead of the Dummy and everything. Maybe it wasn't really working and I was mistaken due to my debug Text Messages displaying the "correct" name of DamageEventSource. But like you said, I was just changing the variable and not the actual mechanics.

 

[Bribe]

Weird, I remember fixing this issue in my map... The kills were credited to the Hero instead of the Dummy and everything. Maybe it wasn't really working and I was mistaken due to my debug Text Messages displaying the "correct" name of DamageEventSource. But like you said, I was just changing the variable and not the actual mechanics.

Well it sounds like it wouldn't matter if this is just to track the "summoner". However, I don't want people to get confused. For example, if the source is owned by a different player than the one who you're trying to reroute the kill to, this will fail. Another example where it can fail is if there is a limited XP bounty range and the correct hero is too far away to get credit for the kill.

Best thing to do is to use a separate variable to track the "true" source of the damage and use something like UnitIndexer to correlate the two together. GUI UnitEvent also does this automatically with summons - SummonerOfUnit[(Custom value of Water Elemental)] tracks a Water Elemental's Archmage for example. But this automatic assignment does not account for trigger-created dummy units, so you'd have to process that on your own.

Lastly, for the completely-correct process, dummy units should never be dealing damage themselves as they are - by design - only there to look pretty or to cast buffs/debuffs or to simplify effects such as lightning. The damage should always be triggered on behalf of the source unit.

 

[Ricardo_Gavarni]

Updated to version 1.1

@JAKEZINC, I fixed the worst of location leaks, thanks for your pointers. I would be grateful for any additional feedback!

 

[FUJI]

Do you have any tips on making importing easier? Currently, the user is required to copy two abilities, one unit, and icons (in addition to triggers). Also, rebalancing the ability seems cumbersome, since you need to update text descriptions manually after changing the numbers.

It's a convention. A user is always required to import objects and triggers in order for a coded/triggered spell to work. ^^
Although, there's a way to cut objects importing via 'ObjectMerger' script (back in times where JNGP is still an era of modding)

For now, if you worry that much about making importing easier,
I could suggest to code the dummy ability instead, custom icon is not to worry about as it's up to the user's preference.
A map-maker that relies on custom spells could even have a global dummy unit, that's why configuration for a dummy is a must.

In this way, only 1 ability object is required. (It's up to you how you'll code it as you chose that type of complexity and it's trivial)
I don't even know how you'll possibly detect a unit that is casting an ability nearby without sorting to a Unit - Starts Ability event.

Anyway, I can't test the map on my editor as you saved it on reforged so I'll just make a use of preview triggers:

• I'm expecting to at least see a 'Configuration' trigger.

^ This is the thing you should worry about: make configuring your spell easier for a user.

• I don't see any valid reason why 'Variables' trigger is created?

^ Is 'Automatic paste trigger data variable when importing' doesn't work anymore?

• Separated trigger for dummy creation is unnecessary and a bit expensive.

• Why you didn't remove:
  • Game - Display to (All players) the text: (Name of the current trigger)

• You're still leaking position of ManaFlare_Host:
  • Set VariableSet ManaFlare_Location = ((Position of ManaFlare_Host) offset by (ManaFlare_OFFSET, ManaFlare_OFFSET))
• You stored unit variables but didn't used them here:
  • Save Handle Of(Triggering unit) as 0 of ManaFlare_UnitsNum in ManaFlare_Table.

^ There are lots of room where you can just use the stored variable, not only this part.

• Use a user-defined integer variable for loops as
  • For each (Integer A)
  bj is notorious for its known bugs.

• • Unit - Move ManaFlare_Dummy instantly to ManaFlare_Location
  is expensive than SetUnitX/Y.

^ This means learn to work with coordinates instead of points when it comes to periodical unit-movements.

• You're looping through 0 to N where it should be 1 to N as the ManaFlare_UnitsNum starts with index of 1 and not 0.

• This spell is only designed for heroes and not for units that can't learn ability.

Is the "kill-stealing" thing a big deal? For example, the leaderboard of Sunken City tracks kills by the player, not by the hero (to account for summons). It is important for things like lifesteal, I guess.

Of course, if you meant that this spell is only usable for those maps then you can ignore it. But It'll be considered as a limitation.
This is only my review through script and not in-game though. I hope you understand that there are things to consider in-game like what happens if unit is this and that or what more if unit is invisible considering that this is a passive ability which is always active.

 

[Ricardo_Gavarni]

Anyway, I can't test the map on my editor as you saved it on reforged so I'll just make a use of preview triggers:

Could you elaborate here? My WC3 is on 1.31.1, which I thought was the last version before Reforged (1.32).

 

[FUJI]

Could you elaborate here? My WC3 is on 1.31.1, which I thought was the last version before Reforged (1.32).

Oh I just meant that 1.30.4 editor can't open 1.31.1+ maps. (including reforged)
If you're staying on legacy version and not upgrading to reforged then 1.30.4 is the right spot obviously.
It's not mandatory though but just a compatibility tip so that everyone can test your resource.

 

[MyPad]

From the looks of it, there's not a lot of things that can be configured (trigger-only) with this spell.
It may be useful for some who would like to create a passive version of Mana Flare, but it still
needs a lot of optimization for approval.

From inspecting the triggers:

1. In trigger Learn Mana Flare, there are a lot of unnecessary actions, which lead to a decrease in efficiency.
   
   
   Spoiler: trigger

   • Learn Mana Flare
     • Events
       • Unit - A unit Learns a skill
     • Conditions
       • (Learned Hero Skill) Equal to Mana Flare (permanent hero skill)
     • Actions
       • -------- Please remove the debug message --------
       • Game - Display to (All players) the text: (Name of the current trigger)
       • Set ManaFlare_Host = (Triggering unit)
       • Set ManaFlare_Level = (Level of Mana Flare (permanent hero skill) for ManaFlare_Host)
       • -------- Only run this trigger once, when you register ManaFlare_Host --------
       • Trigger - Run Create Mana Flare Dummy <gen> (ignoring conditions)
       • -------- Please remove the debug message --------
       • Game - Display to (All players) the text: (Mana Flare Level: + (String(ManaFlare_Level)))
       • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
         • If - Conditions
           • ManaFlare_Level Equal to 1
         • Then - Actions
           • Set ManaFlare_UnitsNum = (ManaFlare_UnitsNum + 1)
           • Hashtable - Save Handle Of(Triggering unit) as 0 of ManaFlare_UnitsNum in ManaFlare_Table
           • Hashtable - Save Handle Of(Last created unit) as 1 of ManaFlare_UnitsNum in ManaFlare_Table
           • -------- REVIEWER ADDED FUNCTION --------
           • Hashtable - Save ManaFlare_UnitsNum as 2 of (Key of (ManaFlare_Host)) in ManaFlare_Table -------- The action here is redundant --------
           • Set ManaFlare_Host = (Load 0 of ManaFlare_UnitsNum in ManaFlare_Table)
         • Else - Actions
           • -------- The loop is overkill at this point, O(n) --------
           • -------- You would only need to adjust the level of the dummy ability --------
           • -------- Remove the actions below, up to REVIEWER ADDED FUNCTIONS --------
           • -------- You can get the right index via (Load 2 of (Key of (ManaFlare_Host)) in ManaFlare_Table) --------
           • For each (Integer A) from 0 to ManaFlare_UnitsNum, do (Actions)
             • Loop - Actions
               • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                 • If - Conditions
                   • (Load 0 of (Integer A) in ManaFlare_Table) Equal to ManaFlare_Host
                 • Then - Actions
                   • Set ManaFlare_Dummy = (Load 1 of (Integer A) in ManaFlare_Table)
                   • Unit - Kill ManaFlare_Dummy
                   • Hashtable - Save Handle Of(Last created unit) as 1 of (Integer A) in ManaFlare_Table
                 • Else - Actions
           • -------- REVIEWER ADDED FUNCTIONS --------
           • Set <integer_var> = (Load 2 of (Key of (ManaFlare_Host)) in ManaFlare_Table)
           • Set ManaFlare_Dummy = (Load 1 of (Integer A) in ManaFlare_Table)
           • Unit - Set level of Mana Flare (permanent hero skill) for ManaFlare_Host to ManaFlare_Level
   
   
2. The passive ability still appears to activate when the user is physically dead (not dead enough for "a unit has died" event to trigger). To circumvent this, check if the user is alive and order the dummy to start channeling mana flare if so, or order the dummy to stop channeling.

JAKEZINC has already elaborated on some key points that have been omitted from this review.

Setting status to Awaiting Update

 

[Ricardo_Gavarni]

I think I fixed everything I could. Big thanks to JAKEZINC and MyPad. I need to discuss some things before taking a final pass.

Things I changed despite nobody asking me for it:

1) I backported the map to 1.30.04 for better compatibility.

2) I updated ShadeDeath.wav (shameless self-plug for my [Resource Release][WIP] Expanded Voidwalker / Obsidian Destroyer soundset)

3) I changed the map somewhat

Things I hadn't changed:

1) The passive Mana Flare remains a hero-only ability. I could change it if there's a strong enough interest, but so far I think it fits heroes better.

EDIT: I created a non-heroic version, check Satyr Soulstealer at the Mercenary camp!

2) I left the trigger for dummy creation separated. It is used in two places (maybe in three, if I come around to creating a non-hero version), so it makes sense to re-use it. I think it should ideally be a JASS function, but I'm not proficient enough for it yet.

3) There's still very minimal configuration.

I tried to change tooltips of hero ability automatically (using this trick: Dynamically Retrieve Object Editor Fields), and it works for normal tooltips, but apparently it is bugged for "learn - extended" tooltips (at least in 1.30.04 version). I left variables and script responsible for it in a desperate hope that somebody would suggest a way to fix them, but I'm going to remove them later.

I thought I can configure the hero ability (say, somebody wants to link Mana Flare effect to Bash or Engineering Update instead of my custom hero ability) with triggers by specifying

• Set MF_HERO_ABILITY = Mana Flare (permanent hero skill)

at the initialization and using this variable everywhere. However, I have no idea how to replace this condition in the Learn Mana Flare trigger:

• (Learned Hero Skill) Equal to Mana Flare (permanent hero skill)

since it does not accept any variables.

EDIT: I figured this out, thanks to Uncle!

4) I move dummy with

• Unit - Move MF_Dummy instantly to MF_temp_loc2

instead of using SetUnitX/Y. The later appears to be glitchy: the unit is moving (the lightning comes from the correct place), but the model stays in the old place.

EDIT: I figured it out, I need the dummy to have movement speed higher than 0. This way, SetUnitX/Y works.

 

[Uncle]

For learning the custom ability:

• Example
  • Events
    • Unit - A unit Learns a skill
  • Conditions
  • Actions
    • Custom script: if GetLearnedSkill() == udg_MF_HERO_ABILITY then
    • Game - Display to (All players) for 1.00 seconds the text: Learned Mana Flare
    • Custom script: endif

Edit: I don't think I realized this existed at the time (or maybe it doesn't work for custom abilities/variables?):

• (Learned Hero Skill) Equal to Human Archmage - Blizzard

 

[Ricardo_Gavarni]

Thanks! It works. Implemented a non-hero variant of the same ability, also added "creating dummies on the map initialization" stuff (to support creeps placed by mapmaker and heroes with "pre-learned" skill)

 

[Royalpimp]

This looks brilliant. Maybe try to have the effect of the spell on the unit look purple like the 'mana flare' itself? Right now it looks blue. In any case I can't wait to see your Kael'thas pack.

 

[Ricardo_Gavarni]

Maybe try to have the effect of the spell on the unit look purple like the 'mana flare' itself? Right now it looks blue.

That's a low-quality GIF issue. In reality, both the "lightning" and the "wisp" look green. Their colors do not quite match, but it's the best I could do without creating a custom model for either.

I guess I can change the projectile model to a bolt of basic lightning (not green one) and the wisp model to a lightning missile? I think it would look confusing.

(and yes, I tried the effect of Mana Burn and it looks bad)

EDIT: additional thoughts on wisp model.

Currently, I use Orb of Slow <Missile> (which is located at Abilities\Weapons\ProcMissile\ProcMissile.mdl).

It being a missile means that it has no attachment points (so you cannot run "create a special effect on Unit". You can do "create a special Effect on Location(this_unit_pos)", but the height is wrong). It kinda makes it difficult to add more splashy special effects

Also, the model has an ambient sound (looping). Surprisingly, but the name of that sound is "Abilities\Spells\Human\ManaFlare\ManaFlareLoop.wav". I'm not joking, but this is a hell of a coincidence. Maybe the Mana Flare ability was planned for Spell Breakers initially?

In any case I can't wait to see your Kael'thas pack.

I think you confused me with JAKEZINC and, honestly, I'm flattered by that

 

[Royalpimp]

Opps, sorry. Either way, I'm looking forward to your work .

 

[HandsOfficial]

wow nice

 

[Rheiko]

The code is now pretty much optimized. Good job.

One minor issue I would like to mention is, this does not work properly with reincarnation ability. MF_Dummy will be killed as soon as the unit it is attached to dies. So when the unit finishes reincarnating, MF_Dummy is already gone. I think there is no way of detecting reincarnation aside from using GUI Unit Event (now with a state-of-the-art Lua version) , so you might want to look into it.

If you choose not to implement it, you can create a notice both in your spell description and in your in-game documentation that this does not work with reincarnation. Other than that, there is no reason to keep this resource away from approval. Due to the nature of its design simplicity, but sheer usefulness,

This resource is
Approved