Frost Field v1.0.2

[Daffa]

Your productivity makes me looks bad

Took a quick look at the code and I think it's great, but I'm sure I miss a lot of things.

 

[Rheiko]

I still have some other spells in progress, expect them uploaded soon. =P

 

[Ofel]

Cool Hi there.

 

[Rheiko]

Cool Hi there.

Yo! long time no see!

Edit:
I made a small update. I noticed I overwrote FRF_Loc[1] at config trigger twice, lol. Clumsy me. Glad you didn't notice, @Daffa. =P
I also replaced Kill FRF_TempUnit to call SetUnitWidget because I heard the former has a slightly lower performance.
I tried to compare them and it does feel like SetUnitWidget is better but it could be just a placebo effect. I don't know.
I wish I could do a benchmark but I don't know where to start. So, I'll just leave it that way for now.

 

[Ofel]

Yo! long time no see!

Edit:
I made a small update. I noticed I overwrote FRF_Loc[1] at config trigger twice, lol. Clumsy me. Glad you didn't notice, @Daffa. =P
I also replaced Kill FRF_TempUnit to call SetUnitWidget because I heard the former has a slightly lower performance.
I tried to compare them and it does feel like SetUnitWidget is better but it could be just a placebo effect. I don't know.
I wish I could do a benchmark but I don't know where to start. So, I'll just leave it that way for now.

Looks like you are having good time making spells now.
I forgot how to review a submission hehe.

Maybe you could try writing this in Jass since you are using a lot of custom scripts.
It's somehow easy after you try learning a couple of lines, and it also boosts performance a bit.

 

[Rheiko]

Looks like you are having good time making spells now.
I forgot how to review a submission hehe.

Haha, yes! I got some spell ideas after coming back. Also trying to revive the spell section while I'm working on them.
No worries, I'm sure they'll be coming back to you in an instant!

Maybe you could try writing this in Jass since you are using a lot of custom scripts.
It's somehow easy after you try learning a couple of lines, and it also boosts performance a bit.

I'm planning to do so in the near future. For the time being, I prefer to work with something I know better which is GUI.
But I have a JASS spell coming up, although the progress is rather slow (because I suck), you can look forward to it. =)

 

[lucci102]

thx bro ,cool one

 

[tlstkwjr]

For my needs, I set the skill radius to 10000, and it crashes. What could be the cause?

 

[Rheiko]

Ah probably because it goes out of bounds. You can turn on the pathing check to prevent this. Although it will kill the projectile early if it collides with doodads and stuffs. Are you sure you really need 10000?

I'll add a check for map bound in the next version. Thanks for the feedback.

Edit:
I made a quick update to v1.0.1 to fix the crashing issue. You can try downloading the new version, sir @tlstkwjr.
You don't have to turn on the pathing check. It should work now. Hopefully.
I will try to further optimize the code down the line, the more I look at it, it looks like a complete mess from my perspective, lol.

 

[Antares]

You're removing the location FRF_Loc[3] twice during the pathing checks. When you assign CP_Point to that location, you're not cloning the location, you're just assigning a new variable pointing to the same one, so destroying CP_Point is not necessary.

The fact that you copied this giant block of code just for the reverse direction is a bit wasteful. Surely there's a simple way to do that with one such block.

If you want to increase the performance of the spell, you could do the following:

• Move FRF_Loc[1] = (Position of FRF_Caster[FRF_Spell_ID] outside of the loop over dummy units, as it's always the same location.
• Reduce the number of array lookups by presaving certain values that are looked up often, such as FRF_Aoe[FRF_Level[FRF_Spell_ID]], to non-array globals. This will also make your triggers more readable.
• Move the (FRF_PickedUnit is alive) Equal to True condition to the bottom, because it's the least likely to return false.

Certainly not an extensive list.

It's a shame that you have to use dummy units for this.


But I guess that makes it viable for 1.26 users. If you're a 1.31 or Reforged user, consider replacing the dummy units with special effects to improve performance.

Apart from all that, the config is well-done, the spell looks great, and there are no glaring issues.

Approved

 

[Rheiko]

You're removing the location FRF_Loc[3] twice during the pathing checks. When you assign CP_Point to that location, you're not cloning the location, you're just assigning a new variable pointing to the same one, so destroying CP_Point is not necessary.

Really? I didn't know that. I thought it would leak. So for this case, removing one of them should be sufficient?

The fact that you copied this giant block of code just for the reverse direction is a bit wasteful. Surely there's a simple way to do that with one such block.

If you want to increase the performance of the spell, you could do the following:

• Move FRF_Loc[1] = (Position of FRF_Caster[FRF_Spell_ID] outside of the loop over dummy units, as it's always the same location.
• Reduce the number of array lookups by presaving certain values that are looked up often, such as FRF_Aoe[FRF_Level[FRF_Spell_ID]], to non-array globals. This will also make your triggers more readable.
• Move the (FRF_PickedUnit is alive) Equal to True condition to the bottom, because it's the least likely to return false.

Certainly not an extensive list.

I completely agree with you, thanks for letting me know!
and Thank you so much for the approval!

 

[Sindobato]

I also thought it would leak but the "not cloning" was a revelation. My whole life was a lie

 

[Antares]

Really? I didn't know that. I thought it would leak. So for this case, removing one of them should be sufficient?

There are primitive types and reference types. Primitive types (strings ,integers, reals, and booleans) store a value, so changing one does not change a copy.

set A = 5
set B = A
set A = 3
//B is still 5.

All other variable types are reference types. They do not store a value, but are a pointer to an object somewhere behind the curtains, so if you're changing the underlying object, it will affect all variables referencing it.

set Location1 = Location(0, 0)
set Location2 = Location1
call MoveLocation(Location1, 1000, 1000)
//Location2 is now at (1000, 1000)

 

[Rheiko]

Updated to v1.0.2 - Code Optimization

I reduced the block of code so now it only does the movement once. If the user wants the projectiles to return, only the offset will change.
I can feel the wonderful performance increase! (I have a potato machine) Felt good.
I also did what Antares suggested. Though I'm still not sure what's the best way to do this:

Reduce the number of array lookups by presaving certain values that are looked up often, such as FRF_Aoe[FRF_Level[FRF_Spell_ID]], to non-array globals. This will also make your triggers more readable.

@Antares, if you don't mind, would you please review the updated version? =)

 

[Antares]

Looks fine to me.

 

[Daffa]

Updated to v1.0.2 - Code Optimization

I reduced the block of code so now it only does the movement once. If the user wants the projectiles to return, only the offset will change.
I can feel the wonderful performance increase! (I have a potato machine) Felt good.
I also did what Antares suggested. Though I'm still not sure what's the best way to do this:


@Antares, if you don't mind, would you please review the updated version? =)

You can create another variable as non-array variant and set it at the start of cast or loop, depending when the value is needed.

Set FRF_SpellAOE = FRF_Aoe[FRF_Level[FRF_Spell_ID]]

Then use the FRF_SpellAOE in place of the array version for that trigger portion

 

[Rheiko]

Maybe I should put it inside the loop, yeah?
I'm afraid the value will change when a multi-cast happens if I put it in the cast trigger.

 

[Daffa]

Maybe I should put it inside the loop, yeah?
I'm afraid the value will change when a multi-cast happens if I put it in the cast trigger.

Easiest way: place it next to whenever the value for the arrayed version is altered.

Say:
(Variable value change)
Set variable to non-array here

 

[popeye_336]

very good spell and super customizable too

 

[LightMidnight]

Very cool ability. Is there a way to add a pause before returning the projectiles to their original location?
I am trying to insert a third stage (in between the first and the second) to make projectiles stop for a bit at the outward position.

Edit: I added a simple condition at the ~end of the first stage yet somehow it doesnt feel right in the game.

Spoiler: Trigger condition

 

[Uncle]

Very cool ability. Is there a way to add a pause before returning the projectiles to their original location?
I am trying to insert a third stage (in between the first and the second) to make projectiles stop for a bit at the outward position.

If you dig through the Frost Field Loop trigger you can find this comment (a note from the author) and the Action that handles the returning movement:

• Else - Actions
  • -------- Still far away; Decrease the relative offset --------
  • Set FRF_RelativeOffset[FRF_Spell_ID] = (FRF_RelativeOffset[FRF_Spell_ID] - FRF_Velocity[FRF_Spell_ID])

You could add a delay here by introducing a new Real array variable that will keep track of elapsed time.

This should work:

• Else - Actions
  • -------- Still far away; Decrease the relative offset --------
  • Set FRF_Return_Delay[FRF_Spell_ID] = (FRF_Return_Delay[FRF_Spell_ID] - FRF_Interval)
  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    • If - Conditions
      • FRF_Return_Delay[FRF_Spell_ID] Less than or equal to 0.01
    • Then - Actions
      • Set FRF_RelativeOffset[FRF_Spell_ID] = (FRF_RelativeOffset[FRF_Spell_ID] - FRF_Velocity[FRF_Spell_ID])
    • Else - Actions

FRF_Return_Delay[] is the new variable.

You also need to Set this variable inside of the Frost Field Cast trigger. You can add it after the other configurable variables:

• ...
• Set FRF_IsOutward[FRF_Spell_ID] = True
• Set FRF_Return_Delay[FRF_Spell_ID] = 1.50

Now the projectiles will pause for 1.50 seconds, adjust this to whatever value you want.

 

[LightMidnight]

If you dig through the Frost Field Loop trigger you can find this comment (a note from the author) and the Action that handles the returning movement:

• Else - Actions
  • -------- Still far away; Decrease the relative offset --------
  • Set FRF_RelativeOffset[FRF_Spell_ID] = (FRF_RelativeOffset[FRF_Spell_ID] - FRF_Velocity[FRF_Spell_ID])

You could add a delay here by introducing a new Real array variable that will keep track of elapsed time.

This should work:

• Else - Actions
  • -------- Still far away; Decrease the relative offset --------
  • Set FRF_Return_Delay[FRF_Spell_ID] = (FRF_Return_Delay[FRF_Spell_ID] - FRF_Interval)
  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    • If - Conditions
      • FRF_Return_Delay[FRF_Spell_ID] Less than or equal to 0.01
    • Then - Actions
      • Set FRF_RelativeOffset[FRF_Spell_ID] = (FRF_RelativeOffset[FRF_Spell_ID] - FRF_Velocity[FRF_Spell_ID])
    • Else - Actions

FRF_Return_Delay[] is the new variable.

You also need to Set this variable inside of the Frost Field Cast trigger. You can add it after the other configurable variables:

• ...
• Set FRF_IsOutward[FRF_Spell_ID] = True
• Set FRF_Return_Delay[FRF_Spell_ID] = 1.50

Now the projectiles will pause for 1.50 seconds, adjust this to whatever value you want.

It works well, thanks.

Now a different problem occurred.
The projectiles move with the caster while "paused". I would like them to stay in place and then move towards the caster when the FRF_Return_Delay runs out. Right now they teleport to a position around the caster and then start moving. I tried to work with the variables a bit to no avail.
I presume this is where i need to make certain changes...

Spoiler: Trigger Code

 

[Pyrogasm]

Making such a change requires fundamentally altering how this spell calculates the position of all of the frost novas. Each nova's position is stored as a "distance offset" relative to the caster rather than absolute XY coordinates or even distance/angle, so to place them temporarily on the ground requires decoupling the projectiles from the caster. After pausing the novas, the math to get them to return to the caster from their now-decoupled position is not exceedingly complex but does need to be worked out because otherwise they will either teleport or not all arrive at the caster's position at the same time (and the direction/angle they all have to move in the meantime is unique for each nova).

The code just isn't set up to do that easily, though it could be modified with a lot of effort. I don't think it's worth it. You could try to fake it by using a dummy caster to cast this spell instead of letting the actual caster use it directly. Then you can manipulate the position of the 'true' dummy caster to place the novas where you want them to be (lock dummy in place for N seconds, then make another periodic section to quickly move the dummy from that position to the caster's current position).