Approved
Moderator
M
Moderator
12th Dec 2015
IcemanBo: Too long as NeedsFix. Rejected.
22nd July 2012
Magtheridon96:
In addition to Pharaoh_'s comments:
This indeed needs heavy modifications.
IcemanBo: Too long as NeedsFix. Rejected.
22nd July 2012
Magtheridon96:
In addition to Pharaoh_'s comments:
-
-
Set TossPlayer = (Player group((Owner of TossU[TossCount])))
-
Cinematic - Clear the screen of text messages for TossPlayer
-
Game - Display to TossPlayer for 8.00 seconds the text: ...
-
Player Group - Pick every player in TossPlayer and do (Actions)
-
Loop - Actions
-
Custom script: if GetLocalPlayer() == GetEnumPlayer() then
-
Sound - Play Error <gen>
-
Custom script: endif
-
Custom script: call DestroyForce (udg_TossPlayer)
-
-
Custom script: set udg_ErrorText = "INSERT_YOUR_TEXT_HERE"
-
Custom script: set udg_TossPlayer = GetOwningPlayer(udg_TossU[udg_TossCount])
-
Custom script: if GetLocalPlayer() == udg_TossPlayer then
-
Custom script: call ClearTextMessages()
-
Custom script: call DisplayTimedTextToPlayer(udg_TossPlayer, 0, 0, 8, udg_ErrorText)
-
Sound - Play Error <gen>
-
Custom script: endif
-
- Since you're already setting the triggering unit in a variable, why not use that variable rather than repeating the call.
- The above point applies to all the other trigger actions. (It's better to cache
things like (Last created unit), (Triggering unit), (Triggering player), etc... into variables rather than repeating the calls.) - There should be some form of configuration here. Right now, all the spells are hard-coded.
- When you're picking destructables, you're leaking a region.
- You don't need to null global variables. Like at all.
- Currently, your spell can destroy bridges. You need to make a dummy peasant unit that is ordered to harvest the
destructables. After ordering him to do so, if his current order is "harvest", then the destructable is in fact a tree. - Your "Clear" trigger could totally bug up a game. You shouldn't be using Peasants as dynamic dummy casters in the first place.
- Check out this. Your Hard Skin ability should use this
damage detection system's Damage event rather than using the attack event. - When you set the size of a unit, the last two values could be 0 and it wouldn't matter, because you can't stretch a unit.
- The Grow trigger modifies the units movement speed which can conflict with a lot of systems the user might be using :/
-
-
Unit - Create 1 Peasant for (Owner of (Attacked unit)) at (Position of (Attacked unit)) facing Default building facing degrees
-
- You shouldn't add and remove the crow form ability, use this method instead.
This indeed needs heavy modifications.
05:14, 22nd May 2012
Pharaoh_: The triggers could be shorter, by using calculations and not a bunch of if/then/else. Use custom integers and not Integer A. Do not use waits. There is no configurability whatsoever. Unit is attacked is an abuseable event. Triggering unit > Dying unit. Certain location variables don't need an array, use separate ones instead. Store values in a variable, before calling them repeatedly, such as (Picked unit).
Do not reupload the spellpack, update it instead. I will delete your previous one. If you still want my review from that one, make sure to leave me a visitor message.
Needs heavy modifications. Rejected.
Pharaoh_: The triggers could be shorter, by using calculations and not a bunch of if/then/else. Use custom integers and not Integer A. Do not use waits. There is no configurability whatsoever. Unit is attacked is an abuseable event. Triggering unit > Dying unit. Certain location variables don't need an array, use separate ones instead. Store values in a variable, before calling them repeatedly, such as (Picked unit).
Do not reupload the spellpack, update it instead. I will delete your previous one. If you still want my review from that one, make sure to leave me a visitor message.
Needs heavy modifications. Rejected.
