Glacial Dragon v1.4 [GUI]

[KILLCIDE]

Hmmm... I actually kind of like the idea you have here. A GUI spell that just has one trigger It is a little hard to read though without the indentation and colored syntax.

• Did you copy-paste this from WEU? If so, could you update your description to have the vanilla World Editor functions?
• I hate magic numbers in code
  
  • Set GlaciarDragon__TempLoc2 = (GlaciarDragon__TempLoc offset by 0.50 towards (Facing of GlaciarDragon__Caster[GlaciarDragon__TempInt2]) degrees)
  Make that 0.50 configurable and or explain its purpose!
• Creating dummy units for the owner of the caster messes up the score board at the end of the game (which some people get anal over). It would be better to use any of the neutral players, unless of course you want the dragon to give vision.
• Instead of always adding + removing Crow Form from the dummy unit, wouldn't it just be better to add it to the unit via Object Editor?
• Between function GlaciarDragon_Damage and GlaciarDragon_InitDamage, there is a random Custom script line with nothing
• Given the size of allocation + deallocation, why not just use linked list to save on performance?

I didn't take a look at the onPeriodic function because... well its sort of a pain to read. I will look at it more closely on the next update.


Awaiting Update

 

[Kazeon]

I don't see any necessary change in your review.

I hate magic numbers in code

It's to make the unit keep its facing angle. Larger numbers may causing unit to face other angles because of path finding. Therefore, it should be kept away from user reach.

Creating dummy units for the owner of the caster messes up the score board at the end of the game (which some people get anal over). It would be better to use any of the neutral players, unless of course you want the dragon to give vision.

The dummy units have 0 points value, it won't cause anything to the score. And yes the purpose is to allow player to have vision over it.

Between function GlaciarDragon_Damage and GlaciarDragon_InitDamage, there is a random Custom script line with nothing

It's spacing between functions and there are more like it in other functions too.

Given the size of allocation + deallocation, why not just use linked list to save on performance?

Dynamic indexing is more commonly used in GUI. But if you are being nitpicky, maybe I can manage to update it one day.

 

[KILLCIDE]

I don't see any necessary change in your review.

I see three.

Did you copy-paste this from WEU? If so, could you update your description to have the vanilla World Editor functions?

Make that 0.50 configurable and or explain its purpose!

Instead of always adding + removing Crow Form from the dummy unit, wouldn't it just be better to add it to the unit via Object Editor?

Dynamic indexing is more commonly used in GUI. But if you are being nitpicky, maybe I can manage to update in one day.

The linked list was just a suggestion. It would really help with the readability, as most of what I said was.

 

[Kazeon]

Nah, I'm not judging you but.. I thought moderator should have understood the approval criteria well. Nothing about those three is that required to change. Those can be improved but not a must.

 

[KILLCIDE]

Why are you in a rush to get this approved? I found enough time in my day to do a quick review of 2 submissions and to mark the resources I wanted to review later in the future, which is why I said this:

I will look at it more closely on the next update.

I didn't reject your spell. I merely put it on "Awaiting Update" so I can come back to it when I can. I saw a few things that COULD be updated, especially the three I quoted on my 2nd post.

 

[Kazeon]

I'm in no rush at all. Maybe I said a thing about approval criteria but I didn't mean the spell to be approved right now. Your review just isn't critical enough to tag the resource as needing fix.

 

[KILLCIDE]

Did you read my last sentence?

Your review just isn't critical enough to tag the resource as needing fix.

Needs Fixed is not a thing anymore.

 

[Kazeon]

needs fix != rejected
needs fix = awaiting update

go figure

 

[KILLCIDE]

If you have an issue with my review, feel free to make a post in Admin or Staff contact. It's going to stuck on Awaiting Update when I find free time again. Anyway, if you think your submission isn't breaking any rules, take a look again.

needs fix != rejected
needs fix = awaiting update

go figure

I don't understand the need for sarcasm.

 

[Kazeon]

I have no problem with the awaiting update status. But rather the reasoning behind it. If you need me to elaborate, fine. "awaiting update" or formerly "needs fix" only given to resources with required critical update (contains leaks, etc.).

 

[KILLCIDE]

I told you exactly why I set it to awaiting update.

and to mark the resources I wanted to review later in the future

I merely put it on "Awaiting Update" so I can come back to it when I can. I saw a few things that COULD be updated, especially the three I quoted on my 2nd post.

 

[Kazeon]

I pointed out the issue under impression you are setting this to needs fix upon minor issues. But if it's indeed somehow helping you in keeping track of reviewed resources, then okay I can live with that. But as far as I remember moderators don't usually do it.

Anyway, if you think your submission isn't breaking any rules, take a look again.

I failed to see which part of rule this spell does not meet.

 

[KILLCIDE]

I couldn't find anything game breaking, so everything looks good. I think it's pretty cool that you kept a GUI spell in one trigger, but if you do plan on doing it again in the future, add more spaces between the functions. It became a pain looking for when a function started and ended.

The spell is Approved, but my suggestions in the first post are still available for you if you ever feel like looking at it. Also, please make sure to update the triggers to the VANILLA one. I'm not entirely sure which ones you have posted right now.

 

[zv27]

This spell requires imports.The question is, why it is not mentioned in the description.I think that would be very useful for beginners.
This spell requires explanation of how to install.

 

[KILLCIDE]

This spell requires imports.

No it doesn't. The spell functions perfectly fine without the imports.

This spell requires explanation of how to install.

Good point, not sure how I missed that one. @Quilnez, could you add How to Install instructions in the map + spell description.

 

[zv27]

No it doesn't. The spell functions perfectly fine without the imports.


Good point, not sure how I missed that one. @Quilnez, could you add How to Install instructions in the map + spell description.