[Trigger] A couple of noob GUI questions

[Blood Raven]

Hey everyone!
I'm making a really simple "Village chatter" system for RPGs that basically does the following:
When you select a friendly unit of a designated type, they face your hero, say one of their generic "Hello there!" speeches both with a sound file and floating text, then face wherever they were facing before. The responses are randomized.

Trigger

• Villager Male
  • Events
    • Player - Player 1 (Red) Selects a unit
  • Conditions
    • (Unit-type of (Triggering unit)) Equal to Villager (Male)
    • (Owner of (Triggering unit)) Equal to Player 5 (Yellow)
  • Actions
    • Set Distance = (Distance between (Position of Admiral Proudmoore 0007 <gen>) and (Position of (Triggering unit)))
    • If (Distance Greater than or equal to 500.00) then do (Skip remaining actions) else do (Do nothing)
    • Trigger - Turn off (This trigger)
    • Set RNG = (Random integer number between 1 and 3)
    • Set VillagerMale_Sound[1] = VillagerM1 <gen>
    • Set VillagerMale_Sound[2] = VillagerM2 <gen>
    • Set VillagerMale_Sound[3] = VillagerM3 <gen>
    • Set VillagerMale_String[1] = Greetings, friend.
    • Set VillagerMale_String[2] = Welcome to our village.
    • Set VillagerMale_String[3] = Hello there!
    • Animation - Lock (Triggering unit)'s Head to face Admiral Proudmoore 0007 <gen>, offset by (0.00, 0.00, 90.00)
    • Set Facing = (Facing of (Triggering unit))
    • Unit - Make (Triggering unit) face Admiral Proudmoore 0007 <gen> over 0.50 seconds
    • Sound - Play VillagerMale_Sound[RNG] at 100.00% volume, attached to (Triggering unit)
    • Floating Text - Create floating text that reads VillagerMale_String[RNG] above (Triggering unit) with Z offset 0.00, using font size 10.00, color (100.00%, 100.00%, 100.00%), and 0.00% transparency
    • Floating Text - Change (Last created floating text): Disable permanence
    • Floating Text - Set the velocity of (Last created floating text) to 25.00 towards 90.00 degrees
    • Floating Text - Change the lifespan of (Last created floating text) to 1.00 seconds
    • Floating Text - Change the fading age of (Last created floating text) to 0.50 seconds
    • Wait 1.00 seconds
    • Unit - Make (Triggering unit) face Facing over 0.00 seconds
    • Animation - Reset (Triggering unit)'s body-part facing
    • Floating Text - Destroy (Last created floating text)
    • Wait 0.50 seconds
    • Trigger - Turn on (This trigger)

Now, before you say it, I know there are tons of leaks here. However, I don't know the right commands to null all of my different variables to avoid said leaks. All tutorials say that I need to use custom scripts, but then only give scripts for one of two types of variables.
I currently use two real variables, one integer, and two arrays of sound and string variables. My first question would be, what are all the custom scripts required to null them so they don't leak.

My second question is about the section where I set all the sound and string variable arrays, should I put them into a separate trigger that only runs once on initialization or is it okay to set them each time the trigger runs, and would they leak without nulling, even though the values stay the same?

And my third question is, I've selected the player-controlled hero just by specifying unit, and I'm 99% sure that is the wrong way to go (since he might die/revive/get replaced by rpg triggers and whatnot). Is that okay or should I put him in his own variable?

My final question is, would such a simple system be useful if I posted it in the spells section and would it get approved if I fixed all the leaks and other bugs there might be?

Thanks for your time!
Sincerely, a complete GUI noob.

 

[Jampion]

1. If you use global variables you don't have to null them. You must however destroy the objects the variable points to.
Integer, reals, strings and booleans are primitive data types and cannot be nulled/destroyed.
The most common data types in GUI that leak are points and unit groups.
Sounds should be destroyed. You can use:

• Sound - Destroy VillagerMale_Sound[RNG]

edit: you must not destroy the sound in your case, because it is a global sound handle that is created once and just played whenever it is used. If you create new sound handles to play them, you would have to destroy them.

2. It's better to initialize them at the beginning of the map, if their values never change. A leak only occurs, if you create an object and don't destroy it. By setting a variable you usually do not create objects, so there is no leak. It is possible to create objects when you set a variable though.

• Set p1 = (Position of (Triggering unit))

Creates a new location. -> leaks if not destroyed

• Set p1 = p2

Does not create a new location. -> does not leak

To better understand this you read this tutorial about leaks: Memory Leaks

3. You should always put him in a variable. Only on map initialization you set your variable to that specific unit. That has the advantage, that when you destroy the unit in the editor you only have to repair one trigger instead of like 20. Apart from that in case the unit is replaced, you can fix this.
Dying and reviving does not change a unit's reference as it is still the same unit, but replacing does, so in case the unit is replaced, you have to update your variable to the new unit.

4. I don't know if it would be approved, as I rarely visit the spell section. I would guess no, because it's a very simple and limited approach for this problem. For an approved spell you would probably want to have MUI, no waits and more configuration options.



You do not have to destroy floating texts manually. They are destroyed, if their lifespan expires.

 

[KILLCIDE]

I just wanted to add that simply creating an object does not make it a leak. Losing access to that object, such as overwriting variables or not setting the object to a variable, is what makes a leak.

 

[Blood Raven]

Thanks for the quick replies, it certainly helped a lot.
Why are waits bad and what should I replace them with though?
EDIT: Also destroying the sound causes each of the sounds to only play once for the duration of the game.

 

[Jampion]

Waits are bad because of inaccuracy. The minimum wait duration is something like 0.15 seconds and wait time is also increased by latency in multiplayer maps. I think waits are also real time and not game time, but not sure about that one.
You usually use timers for it in JASS. Not sure what the best solution is in GUI though. You would need more triggers and variables to use timers in GUI which is a bit annyoing.

I don't know if you saw my edit, but in your case the sound must not be destroyed, as it is created at the start of the map and used whenever the sound is played, so no objects are dynamically created.