Alien Massacre

[Moderator]

23:01, 2nd Jun 2009
hvo-busterkomo:
Edit: Changes made, looks good.

 

[Guishiu]

Here is where I will post new News.

Ok fixed the leaks.

But I will still be working to improve it. I still want it to spiral.

Ok, well its been awhile. And OMG guys there WAS a bug. Which is funny, because i knew it was there, but the spell got approved anyways, and out of ALL the people who reviewed and left posts there was ONE, Dimitri231 seen it! Good job guy, +rep!!! Sometimes the ships wouldnt come down. APPARENTLY, Unit Groups DO NOT cycle through right. The ships all have to be lowered and raised individually. So with that, I have REMOVED the unit groups.

Ok, sorry About the Previous Update yesterday!! I accidentally uploaded an older version and then a storm came in and fried my Modem!
But I just uploaded the right one and the Spiral is complete now.

 

[DarkFrog]

I love it. Great documentation aswell.

 

[dansaDisco]

Question

General Review [TD] : [/TD] [tr][td]The spell works?[/td][td]

[/td][/tr][tr][td]The spell does not lag?[/td][td]

[/td][/tr][tr][td]The spell is MUI?[/td][td]

[/td][/tr][tr][td]The Spell is Original?[/td][td]

[/td][/tr][tr][/tr]

Effects	The effects are quite nice, the spawning of the ship is great. But I'm not very fond on the fact that the mini-ships sends out lightning, that makes the target burn. I'd change the missile of the Phoenix Fire.
Movement	The mothership stands still, just as it should. Although, I'd want the mini-ships to have a less "strict" path. Perhaps make them change direction every 1 second or something?
Bugs Found	No bugs found when testing; It does not hit neutral units, but that's probably intentional and can be easily changed.
Other Comments	Good job on this spell. Works very well and I like the originality. Just try to fix the stuff i mentioned and it will be a lot nicer.
Usefulness	In case you don't want to use the original concept, you can easily change the models to something more "medieval". This makes this spell very useful indeed.
Overall Rating	Idea: 5/5 Execution: 4/5 Effects: 3/5 Fun: 5/5

Detailed Review [TD] : [/TD]

 

[Aspard]

Hmm... If you are interested:

1) Yes, by default all the variables are null or not exist. So I don't see any usage of the Initialization trigger. And even if I can undestand that you set all number variables to zero, what's the matter to create a point and then remove it? oO Like "Do nothing", well, you wrote, that you didn't know, now you do, I hope. Anyway, you can easily change the basic number in the variable editor (place where you create these variables)

2) Spell map could be better, I still suggest using one of the uploaded spell test templates by redscores, Nidd-Hogg or aaron

3) Well... import? I understand, that's the matter of the spell Alliens themselves, so you had to import this plate model, but I think, that as a preview you could use random round model, but at spell description tell, that the idea was created to match this model etc

4) Leaks:

• (Distance between (Position of TinyUFO1[Inter_A]) and AB_Target[Inter_A]) (<) 1000.00

Position

• (Distance between (Position of TinyUFO1[Inter_A]) and AB_Target[Inter_A]) (>) 1000.00

Same here

• (Distance between (Position of TinyUFO1[Inter_A]) and AB_Target[Inter_A]) (<) 75.00

And here. Seems to be over with the leaks.

5) Could have better animation concerning damage SFXs.

6) Doesn't have any settings ;(

7) Change the voice of hero!

Otherwise, seems to be MUI, does what it's supposed to, overall codding is okay, excluding these leak points. Idea is creative, but uses import and doesn't have any difficult actions. I think, it's 3/5 now. And, really, please, make a better map or use already created one.

 

[Guishiu]

@Aspard
Ok guy thanx, yea I wasnt sure if they were 0 initially or not, figured they were but just incase someones map they put it into had some setting changed that caused the default to be something other than 0, I added that.
And I did not know a condition could leak? So I have to make it create a point variable for tiny UFO1 every loop and then refer to it and destroy it at the end of the loop? damn. ok
And the test map might get a cliff in there... After all these alien ship spell, I dunno WHAT would happen if it was cast on a cliff..... Well since this one targets the ground and not a unit, it couldnt happen B4 now. Otherwise the map isnt changing. Thanx though.

Oh by the way it is Multi Level. You would have to change the Damage of the Laser Ability the small ships are given. That's why it does have 3 levels to it.

 

[dimitri231]

Hmm, there is a bug when i used it in the fourth time, the space ship didnt came down

 

[Aspard]

And I did not know a condition could leak? So I have to make it create a point variable for tiny UFO1 every loop and then refer to it and destroy it at the end of the loop? damn. ok

Of course, by checking condition you need to get this point anyway, right? Since you're getting this point, WE should give some bytes to this point. And then you are never going to use this point again... ;( Tragic, yeah? Funny now =)

 

[Deuterium]

Nice review there Aspard however i had to pass by and comment on something:

3) Well... import? I understand, that's the matter of the spell Als themselves, so you had to import this plate model, but I think, that as a preview you could use random round model, but at spell description tell, that the idea was created to match this model etc

I'm not a rule fanatic, but the rules comply with what I'm about to say this time:
Imports are not allowed unless - and I emphasize unless - it is a must for the spell.
So, this is a case where it's a must and thus allowable.

@ Guishiu:
I wasn't gonna check the spell as I'm taking a break from reviewing spell until school is done. But since you used to be that pain in the ass when you begin triggering, I will

EDIT:
STUPID AAAH! I MADE THIS ASS HUGE REVIEW WITH EVERYTHING AND DELETED IT BY MISTAKE!!

Anyways, briefly, you leak points, groups and regions. Special effects could be improved and same for the motion of the small alien ships and test-map.
Why copy and spam your test map with disabled and useless triggers?

Here's where you set the initial value of your variable:

Variable Editor

I'm really sorry for deleting my review, it had everything that was needed to fix it and with every leak and mistake pointed...

 

[Guishiu]

Well if that's true then wouldnt the same be said of getting the Flying Heigh in a condition? It is a point isnt it? Only of the Z coordinate instead of X and Y.

Edit* Ok sorry Msaeed's post wasnt here when I wrote that. ANyways crap Yea i forgot about the unit groups. easy fix though at the end of the action trigger. Not sure how a region is leaked... could u be more specific please? I don't use regions here at all did I? O.O
Im working on a spiral formula for the little ships so dont fret over the animation of the ships too much. Spirals are hard. For me they is.
Dunno what you were saying about the Variable Editor though.
BRB with those leak fixes. =D

Okay hell WHY doesn't this work??

• Custom script: call DestroyGroup(udg_TU_Group[Inter_A])

 

[Deuterium]

Of course, by checking condition you need to get this point anyway, right? Since you're getting this point, WE should give some bytes to this point. And then you are never going to use this point again... ;( Tragic, yeah? Funny now =)

Condition leaks are easily fixable, just set the position before the condition into a variable, then use the condition and destroy the point later. Example:

Leaking

• Bad Example
  • Events
    • Time - Every 2.00 seconds of game time
  • Conditions
    • (Distance between (Position of (Triggering unit)) and (Position of (Target unit of ability being cast))) Greater than or equal to 10.00
  • Actions
    • -------- all the actions --------

Leakless

• Good Example
  • Events
    • Time - Every 2.00 seconds of game time
  • Conditions
  • Actions
    • Set DT_Point[0] = (Position of (Triggering unit))
    • Set DT_Point[1] = (Position of (Target unit of ability being cast))
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Distance between DT_Point[0] and DT_Point[1]) Greater than or equal to 10.00
      • Then - Actions
        • -------- all the actions --------
      • Else - Actions
    • Custom script: call RemoveLocation(udg_DT_Point[0])
    • Custom script: call RemoveLocation(udg_DT_Point[1])

Problem solved ladies and gentlemen!

Well if that's true then wouldnt the same be said of getting the Flying Heigh in a condition? It is a point isnt it? Only of the Z coordinate instead of X and Y.

X,Y, and Z are all coordinates and not points. They are simply real numbers which would form a sort of "graph". Thus they only allocate 32-bits only and nothing more. And later, when reused, they do not leak.

So no it's not the same...

 

[Guishiu]

@Msaeed
Okay gotchya. I already got the locations leaks fixed, just dont know WTF I messed that group script up on..
Just wait till u see my location fix. you're gonna be proud =D

 

[Deuterium]

Well, the group leaks once you pick it so if you want to remove the units, there's an actions that's like: Unit Group - Remove all units from <variable>

That would basically "prevent" the leak and allow you to recycle the group... get my point?
Then no "DestroyGroup" would be needed since it doesn't leak but recycles instead...

EDIT:
Region leaks in the Initialization trigger you made to set the variables to zero, but since you're gonna remove the whole trigger then never mind.

EDIT:
I'm not advertising here helping instead!
For the spiral, you can check my spell Comradeship, I use a spiral for like 2 seconds through out the spell. If that isn't clear enough, you can contact me and I'll send you an example from a spell which I stopped working on whenever I check the hive...

 

[a7xpwns3]

Whoo! You finally fixed it!
Good job ;P

Looking good, looking good. Don't have time to test at the moment though. Sorry c:

 

[Aspard]

Condition leaks are easily fixable, just set the position before the condition into a variable, then use the condition and destroy the point later. Example:

Well, I knew that, why did you post it to me?

X,Y, and Z are all coordinates and not points. They are simply real numbers which would form a sort of "graph". Thus they only allocate 32-bits only and nothing more. And later, when reused, they do not leak.

Only to make it more clear, want to add, that flying height - isn't the point of the unit - the point still will be at ground, not in the air, dunno specific part of what it is: WarCraft or whole Jass, but this is so.

I didn't get the idea of a group leak, if it's removed anyway. ;(

 

[Deuterium]

I didn't get the idea of a group leak, if it's removed anyway. ;(

It's not removed anywhere in the trigger...
And what Guishiu is doing here is sort of recycling the groups, but since they're indexed, then it might create a huge number of groups that are never destroyed, but all re-used.

This method is usually used when you're using a one non-arrayed group for all instances.

So I still advise removing and creating groups instead...

To create - in case you decide to use that method:

• Custom script: set <variable> = CreateGroup()

(Anyone correct me if I'm wrong )

Congrats on your first accepted spell

 

[Aspard]

It's not removed anywhere in the trigger...

Well, I was talking about REMOVING, not about this particular trigger

 

[Deuterium]

Btw, this still leaks, example:

• Set TU_Point[T1[Inter_A]] = ((Position of TinyUFO1[Inter_A]) offset by 1050.00 towards 45.00 degrees)

• Set TU_Point[T2[Inter_A]] = ((Position of TinyUFO2[Inter_A]) offset by 1000.00 towards 135.00 degrees)

• Set TU_Point[T3[Inter_A]] = ((Position of TinyUFO3[Inter_A]) offset by 1000.00 towards 225.00 degrees)

• Set TU_Point[T4[Inter_A]] = ((Position of TinyUFO4[Inter_A]) offset by 1000.00 towards 315.00 degrees)

I know you're too excited about this getting accepted but I have to point things out...



@ Aspard:
Well, whenever you make a Unit Group Variable, look at the initial value, it says something like: Empty Unit Group.

This basically means that somewhere in the Map Initialization this is gonna happen:

• Custom script: set <variable> = CreateGroup()

This is a leak which people usually don't know about, but I for example directly destroy such groups (if not arrayed, since I still have to check about the arrayed ones) in the a Map Initialization trigger.

Now Guishiu's spell is gonna be creating a group everytime the MUI Index increase and never destroy them... let's say there's 100 casts, that's hundred leaks. Then maybe only one unit is gonna be casting per time, so the other 99 present groups are useless.

So my advice to Guishiu - to not complicate things - is to ignore the created groups at Map Initialization, and simply destroy after each cast then create at the beginning.

Get my point now Aspard?

& I also have my own way of totally removing the leaks of arrayed groups, but that's complicated so if you're interested I'll pm you my method some day (although it's theoretical and I still didn't test it yet )

 

[Aspard]

Hmm... well, I got it now, you mean, that on the initialization WarCraft creates this empty groups, right? Interesting, I didn't know.

 

[Deuterium]

Haha yes!

 

[Guishiu]

• Then - Actions
  • Set Order_Number[Inter_A] = (Order_Number[Inter_A] + 1)
  • Set TU_Point[T1[Inter_A]] = ((Position of TinyUFO1[Inter_A]) offset by 1050.00 towards 45.00 degrees)
  • Unit - Order TinyUFO1[Inter_A] to Move To TU_Point[T1[Inter_A]]
  • Unit - Add UFO Laser to TinyUFO1[Inter_A]
  • Unit - Set level of UFO Laser for TinyUFO1[Inter_A] to AB_Level[AB_Cast_Number]
  • -------- Custom script that removes location leaks. Location leaks are the area of the target, these are stored by the game cache (unless removed like this) and then dumped at the end, The reason for sometimes having a long loading screen at the end of a game. --------
  • Custom script: call RemoveLocation(udg_TU_Point[udg_T1[udg_Inter_A]])
  • Set TU_Point[T2[Inter_A]] = ((Position of TinyUFO2[Inter_A]) offset by 1000.00 towards 135.00 degrees)
  • Unit - Order TinyUFO2[Inter_A] to Move To TU_Point[T2[Inter_A]]
  • Unit - Add UFO Laser to TinyUFO2[Inter_A]
  • Unit - Set level of UFO Laser for TinyUFO2[Inter_A] to AB_Level[AB_Cast_Number]
  • Custom script: call RemoveLocation(udg_TU_Point[udg_T2[udg_Inter_A]])
  • Set TU_Point[T3[Inter_A]] = ((Position of TinyUFO3[Inter_A]) offset by 1000.00 towards 225.00 degrees)
  • Unit - Order TinyUFO3[Inter_A] to Move To TU_Point[T3[Inter_A]]
  • Unit - Add UFO Laser to TinyUFO3[Inter_A]
  • Unit - Set level of UFO Laser for TinyUFO3[Inter_A] to AB_Level[AB_Cast_Number]
  • Custom script: call RemoveLocation(udg_TU_Point[udg_T3[udg_Inter_A]])
  • Set TU_Point[T4[Inter_A]] = ((Position of TinyUFO4[Inter_A]) offset by 1000.00 towards 315.00 degrees)
  • Unit - Order TinyUFO4[Inter_A] to Move To TU_Point[T4[Inter_A]]
  • Unit - Add UFO Laser to TinyUFO4[Inter_A]
  • Unit - Set level of UFO Laser for TinyUFO4[Inter_A] to AB_Level[AB_Cast_Number]
  • Custom script: call RemoveLocation(udg_TU_Point[udg_T4[udg_Inter_A]])

How do those points leak? I have the variable setting the point, the unit being ordered to move to that point, then removing the point immediately after.

Ohhhh, just looked at it again.. It leaks off of the position of the UFO doesnt it?

Well if thats it then thats easy then. All i have to do is reference it to the Target Point instead of the UFO.


K, Done

 

[Deuterium]

Yupp that's how it leaks.. fix that

 

[Kingsting12]

even if it's a litle weak is it Very good. I like it alot!!!

 

[Deuterium]

You should have continued the spiral until they get back to mothership... I mean, they start getting farther "spiraly" then started getting closer "spiraly"...

Because the spiral made it really look nice

Leaks a point:

• Set TinyPoint1[AB_Cast_Number] = (Position of TinyUFO1[AB_Cast_Number])

^{Abduction Spell Cast}

Also, there's something wrong with removing locations, in-game it gives the message that a null point is being destroyed.

This mean you placed the custom scripts in the wrong place, where points get destroyed without being set at the very end of the cast...

And you use a do nothing at the end of the main trigger...

 

[Guishiu]

I just updated it with the correct version, and a completed spiral. I don't see this message you speak of when I test it.?? Though if its a message being displayed during the spell then I may have left a Game Text Message in the trigger. HAHA. I use those when testing it if there is a bug, so i can see where the bug is at.

 

[Deuterium]

Ummm yes my warcraft shows such stuff although others dont... maybe it's an option in v1.21 :s

And I'll check 2moro good night!

 

[Aspard]

The damage is very low, isn't it? oO

 

[Guishiu]

LOL, dude the damage is configurable. It is configured in the actual spell. And the fleet ships spell is leveled based on the level of the caster's spell.

 

[Dimetrixdemon]

well hm looking nice but still cant download why =(?