Hurl Meteor v1.2

[Moderator]

12th Dec 2015
IcemanBo: Too long as NeedsFix. Rejected.

00:51, 25th Oct 2012
Magtheridon96:

Comments

• The modification of custom values is totally bad. Custom values are global data for a unit, meaning that if a user implements this system into his map, he can never use custom values for anything, or use any other resource that uses custom values. This is why people use UnitIndexers. UnitIndexers assign unique indexes for units and store them as the custom value so you can use that index for data storage in arrays. Check out Bribe's GUI UnitIndexer. It will set the custom values of units to unique ids so that you can attach data to a unit. Unit Indexers are the ONLY resources allowed to modify the custom value of a unit.
• Simply increasing an index by 1 in order to assign a unique id isn't good because of the array index limit. In order to pull of good indexing, you should probably take a look at Hanky's Dynamic Indexing Template.
• You can get rid of this:
  Custom script: set udg_KB_Picked_Unit = FirstOfGroup(udg_HM_KBPick)
  And just use (Picked unit). Apparently, you were initially getting a random unit from a group that you were iterating over. If you're iterating over a group, you're going to run code for every single unit in this group. If you want to do this for only one unit in the group, you shouldn't be picking, you should just be setting the unit variable using the line of code I gave you and then using it as a random unit from the group.
  Actually, I see your mistake now. Get rid of the "Pick every unit in ()" line, and just set the KB_Picked_Unit variable to FirstOfGroup(udg_HM_KBPick), then do the actions you were doing inside the group loop to this unit. Currently, it appears that your code is looping over the group and doing actions for the same unit depending on how many units are in the HM_KBPick group.

Note: I have updated the review.

 

[Tank-Commander]

Replace all possible unit selectors with (triggering unit) when applicable, i.e. (dying unit)

Avoid the use of custom values, shouldn't be too significant of an issue but it's still bad practice and -could - on occasion cause issues

you only need one loop and it should run every 0.03 seconds, not 0.02

• Unit Group - Pick every unit in (Random 1 units from HM_KBPick) and do (Actions)

Why use a random unit and not just all when they're in a close enough proximity? will almost always give only 1 unit return anyway. Also you need to put this in a variable and clear it or use bjWantDestroyGroup()

• Set HM_Damage[1] = 150

If you're going to use constant values - this should be in the configuration trigger or "hashtable" as you call it, although that name is Ambiguous and shouldn't really be used for a trigger

Using co-ordinates rather than locations would also heavily improve this

 

[doomhammer99]

Put this Script

• Custom Script: set bjwantDestroyGroup = true

In here.

• Unit Group - Pick every unit in HM_Dummies and do (Actions)

Like This

• Custom script: set bjwantDestroyGroup = true
• Unit Group - Pick every unit in HM_Dummies and do (Actions)

 

[Almia]

@doomhammer
the group doesnt leak
If you saw my spells,you will understand.


The spell doesnt have any configuration.
A group leak mentioned by Tank-Commander
Your loop is very fast,make it 0.02

 

[doomhammer99]

Ooops Sorry

 

[Magtheridon96]

Using 0.03 as a loop period is more optimal though.
33FPS is double human retinal persistence.
50FPS is pushing it.