Supernova v1.0 GUI

[Moderator]

21:18, 4th Apr 2016
Tank-Commander: v1.0 - There's a lot of problems to go over before the resource can be approved, please view other user feedback and my post and I hope to see an update so I can approve this for you

 

[Meatmuffin]

Hashtables are a bit oldschool for a GUI spell. People tend to use Dynamic Indexing or Linked List EDIT: oops *unit groups.

This is not really configurable for a total newbie, add a configuration trigger.

When using a function more than twice like Triggering Unit, you should store it into variable for better efficiency. You seem to do that a lot.

Is there a reason to use a unit's custom value over a consistent variable? To my knowledge, messing with that is not recommendable.

Dying Unit -> Triggering Unit.

These are the things that caught my eye in the triggers.

 

[KILLCIDE]

The effects are definitely intense. There are a lot of things I can nitpick about this spell (like storing Picked unit into a variable), buut that's just optimization. Here are some things that caught my eye though:

• You should really have a config section. Almost every aspect of your spell is hardcoded, meaning that if a user wanted to change anything, they would have to go through your triggers (ex: all the dummy types).
• Consider looking into dummy.mdx. This will reduce the overall dummy count to 1
• The Unit Group - Units of Type function has a leak that can't be removed via GUI. You should just filter them out using an If/Then/Else
• Where do you change the custom value of the units? You should really avoid this as Unit Indexers are the only resources allowed to mess with them
• Eeek Integer A? Create your own integer variable so that this doesn't clash with other people's project.

Hashtables are a bit oldschool for a GUI spell. People tend to use Dynamic Indexing or Linked List

Squiggy isn't using hashtables?

 

[TriggerHappy]

You shouldn't be setting the custom value of a unit, even if it's just for your dummies.

Unit - Move (Picked unit) instantly to (Nova_tmppt[2] offset by Nova_dummy_speed towards (Facing of (Picked unit)) degrees)

^leaks a location

 

[Chaosy]

This spell is a bit odd.
It is fairly well triggered but it still got some basic mistakes.

1. Everything that you use more than once should be in a variable (triggering unit, last created unit etc)
2. You got no config trigger, if you look at any approved spell you will see what that is.
3. you got two triggers with the same event, those should be merged
4. don't use integer A/B, I prefer to do so myself but the mods WILL request you to change so I might as well point it out to save you some time.

 

[Tank-Commander]

I went over the code a couple times and I found quite a lot of problems and I figured this would be enough to list to get started, if you have any questions feel free to PM me and I'll see if I can help

Major:
- Lacks a configuration
- You have two triggers with the same event and the same stop conditions, they could be merged
- We tend to advise against using custom values of units except in the case of standardised systems like unit indexers
- You can add dummies to the Nova_Group when they're created instead of having to re select the units every iteration, if you remove them when they die then it won't cause a leak
- Variable use to store temporary values that are used more than once should be done
- (Level of Nova for (Triggering unit)) is referenced in your periodic trigger - this will return 0 if another event is fired during the course of the ability since triggering unit will change
- Spells should avoid messing up the endgame scorescreen, to that effect we try to make dummy units owned by a neutral player
- Spell damage should mimic game mechanics: The origin of damage dealt by the spell should be the caster of the spell

Minor:
- You don't need to use an array of locations, at most at any given time you need two which could easily be separate location variables (at the least the indexes 0 and 1 could be reused, the location array assigns a lot of extra memory
- When setting model scales you can set the last two values to be 0., it does the same thing and saves a few mathematical operations
- You turn the periodic trigger on every time you cast the ability, even if it's already turned on which is pretty inefficient
- Utilising Dummy.mdl would enable you to use one dummy unit instead of many for the spell and simply attach the required model and apply the abilities to them
- Dying unit can be replaced with triggering unit

 

[Squiggy]

Aaalright, I'll get to all of the bbove mentioned points and update it.
It's been over six years since I made my last spell and all... I'm rusty.

 

[Ofel]

What a bunch of eye-candy spells six years ago.
Will rate after update.