Revivable Units v1.3.3b

[Moderator]

20:23, 22nd Mar 2016
BPower:
Fair enough. Approved with a rating of useful.

Think about what I said concerning race handle ids.

I feel it's a bit limitating that you can only revive one unit of a specific type id, no matter how many units of that type id died.

 

[Wietlol]

Nice stuff here.
Code is very neat and well readable.

Havent tested it in-game but a few things to improve:

- TriggerAddAction -> TriggerAddCondition
Conditions are faster because actions create a new thread per action, conditions do not really care about them so do not have a few things... for example TriggerSleepAction() wont work in conditions.
However, because you wont be using any of those things, you can just place your actions inside a condition.
Also, place everything that should run on a trigger in one function, so you only have to add one condition to each trigger at max.

- local location dLoc = GetUnitLoc(dyingUnit)
You dont need the location if you use coordinates (aka GetUnitX(), GetUnitY()).
So CreateUnitAtLoc would be CreateUnit.

- Hashtable -> Table or TimerUtils
You use a hashtable but do not use very much of it.
It is recommended to use Table if you want to stick to a hashtable like that (you can make it be optional with static ifs if you dont want to force Table as a requirement).
You do have to use integers like 0, 1, 2 etc instead of 'unit' and 'opac'.
You can make global constants with names like "TABLE_ROW_UNIT" and "TABLE_ROW_OPACITY" and set those to 0 and 1 so you can still know what those rows are for.
An alternative to Table would be TimerUtils and arrays.
You will have to use an allocater to make indices and an array for the units and for the opacity, then you can remove the hashtable from your list and use the one by TimerUtils.
This will also slightly improve timer allocation/deallocation.

- Effects & sound paths -> configurable
This is not really required but it could be usefull to make these configurables... maybe.

- if(revUnits[iSize] == wUnit and revPlayers[iSize] == iPlayer+1) then
You may want to keep track of how many units are used by the players... but its not really necessary.

 

[Arad MNK]

You can't make a single unit revivable with this system. It only works with unit types. Maybe adding this feature would result in a higher rating.

Maybe all the system features (Is unit Revivable? or maybe the dissipate effect) would be better to be configurable for both unit types and units.

I can imagine configuring unit types, and then adding single unit exceptions.

 

[loktar]

Hmm, I'm making this system for my own uses really, don't want to put more effort into it than needed
But I encourage anyone to take this code, add features, and upload it to the hive.

Nice stuff here.
Code is very neat and well readable.

Havent tested it in-game but a few things to improve:

- TriggerAddAction -> TriggerAddCondition
Conditions are faster because actions create a new thread per action, conditions do not really care about them so do not have a few things... for example TriggerSleepAction() wont work in conditions.
However, because you wont be using any of those things, you can just place your actions inside a condition.
Also, place everything that should run on a trigger in one function, so you only have to add one condition to each trigger at max.

- local location dLoc = GetUnitLoc(dyingUnit)
You dont need the location if you use coordinates (aka GetUnitX(), GetUnitY()).
So CreateUnitAtLoc would be CreateUnit.

- Hashtable -> Table or TimerUtils
You use a hashtable but do not use very much of it.
It is recommended to use Table if you want to stick to a hashtable like that (you can make it be optional with static ifs if you dont want to force Table as a requirement).
You do have to use integers like 0, 1, 2 etc instead of 'unit' and 'opac'.
You can make global constants with names like "TABLE_ROW_UNIT" and "TABLE_ROW_OPACITY" and set those to 0 and 1 so you can still know what those rows are for.
An alternative to Table would be TimerUtils and arrays.
You will have to use an allocater to make indices and an array for the units and for the opacity, then you can remove the hashtable from your list and use the one by TimerUtils.
This will also slightly improve timer allocation/deallocation.

- Effects & sound paths -> configurable
This is not really required but it could be usefull to make these configurables... maybe.

- if(revUnits[iSize] == wUnit and revPlayers[iSize] == iPlayer+1) then
You may want to keep track of how many units are used by the players... but its not really necessary.

Nice, thanks for the feedback.
I'm not sure how to proceed with the hashtables. I'd prefer not to use any external code if I can avoid it.
I only chose hashtables over global vars because it's easy to attach information to a specific timer that way (thus avoiding collisions when the dissipate effect is created multiple times at once), and get rid of all that information when I'm done with it.
I don't think there's a way to do that without hashtables (other than using Table or TimerUtils)?

Also are these the correct downloads? TimerUtils, Table

Edit: on closer inspection, Table could make unittype/players storage much simpler too with 2D arrays... considering it

 

[Wietlol]

We have Table here on the hive as well, remade by Bribe (NewTable) iirc.
I dont really bother about Table because I dont use hashtables at all... at all at all
So I dont really know what the difference is between Table and NewTable... you have to search that for yourself.

TimerUtils is correct.

"Hmm, I'm making this system for my own uses really"

There are two kinds of resources...
1, The ones that do what you want.
2, The ones that do what other people want.
... 3, Those who dont do what anyone wants.

Making things for own usage makes things much simpler because you only have to care about the things you care about.
Making things for public usage requires you to search for all related information, use the latest techniques to make it work in the best way and make it as efficient and readable as possible... and then there are the one thing I hate... configurables

None is really better than the other... except that 1 and 2 are both better than 3
It is just what you are using/making it for.
If you have your own project or something like that, then there might be some serious systems that are really useless for public usage but are a requirement to make the map work. You may eventually end up publishing them afterwards.

 

[loktar]

Oh yeah, I agree, I have been adding things that I don't really need, i.e. all the configurables (which I don't like either ). Don't even really need a function to make unit types revivable, as it would be much easier to just do it manually for every unit type I need. But when it comes to adding features, I'll leave that up to other people who want to do that

I'm sharing this for two reasons: 1) sharing is caring and 2) the feedback you get when you submit something to be used by others is invaluable when you don't really know what you're doing

Will have a look at NewTable etc and see what I can use. Mostly the ability to have 2D arrays seems attractive for storing multiple values/players per unit type. Thanks again!

Edit: actually, I'm going to turn revPlayers into a boolean array, and reserve 17 slots per unit. I.e. the index for each player per unit will be unitIndex*17+playerId, where playerId 0-15 is just the playerId and 16 is for "all players". This is waaay easier, can't believe I didn't think of that earlier.

So definitely won't be using Table or anything, still have to take a look at TimerUtils to see if I can be bothered with it seeing as the simple hashtable works quite well already

Edit again: updated

 

[Bribe]

There exist revive systems based on resurrection on an isolated part of the map which keep you from needing to do this remove unit/create unit stuff. If a unit leaves behind no corpse, that is detectable because the resurrection order will return false.

Regardless, what you've done with a pseudo-dissipate effect is cool. I recommend you remake this resource specifically for that.

Sound FX string paths should be configurable. Leave nothing which could be changed by users hardcoded unless there will otherwise be functional flaws.

Eaually, special effect string art should be configurable.

Rawcode integers absolutely should be configurable.

Tip: Make your configurables in GUI like I did with Damage Engine and Spell System. Easier for newer users.

 

[loktar]

Mm, I think being able to use this on any map without having to designate a hidden part of the map to it is an advantage. I like it better this way ^^

Sound FX & Effect configurables - will do (other than those I think everything that can be is already configurable).

Configurables in GUI - do you mean through UDGs?

 

[Bribe]

Mm, I think being able to use this on any map without having to designate a hidden part of the map to it is an advantage. I like it better this way ^^

Actually, I just checked it again, and it revives on the spot. No hidden point on the map required. http://www.hiveworkshop.com/forums/jass-resources-412/snippet-reviveunit-186696/

Sound FX & Effect configurables - will do (other than those I think everything that can be is already configurable).

The rawcode integers must be able to be configured. They should not be hardcoded in case the user has modified those raw abilities and needs to use a copied ability in order to fulfill that role. Unlikely to happen, but the idea is to cover the predictable possibilities.

Configurables in GUI - do you mean through UDGs?

Yes. Jass2/vJass as the base of the system, but with a separate GUI trigger for the configurable things. This way, for people to use your system they need no vJass knowledge.

 

[loktar]

Actually, I just checked it again, and it revives on the spot. No hidden point on the map required.

O rly. Will have a look at that

abilities

Oh right, the abilities, mostly the meld abilities, looked over those.

Yes. Jass2/vJass as the base of the system, but with a separate GUI trigger for the configurable things. This way, for people to use your system they need no vJass knowledge.

Alright, will do. Though I'm not sure how I'm gonna do that for adding units, as a function call is necessary for it. But I'll see what I can do

Thanks for the feedback.

 

[loktar]

Updated - everything is configurable either through GUI or JASS. Hope this approach is acceptable. Seems the best way to offer both GUI and JASS config in one package.

About the ReviveUnit snippet - I don't see how I can implement that. It resurrects a unit immediately where it dies, whereas in my system you have to revive it in the altar (or another building) to revive it.
So a dummy unit isn't of any use. The reason I need to remove the original dying unit and replace it with a new one is that you can't add the fly ability, change flying height, or change opacity on a dead unit. So I need to create a living one and "simulate" death.

 

[BPower]

Cool system idea.

Correct me, but what happens if a player loses for example two or more unit of same type ( grunts ).
Can he revive the same amount or just one?

The following are just notes and not required for approval.

As everything is race handle based, you could also use one array for sound / effect per race
instead of a scalar variables per race. Handles ids:

// Reference: 1 - Human, 2 - Orc, 3 - Undead, 4 - Nightelf, 5 - Demon, 7 - Other, 11 - Naga

I don't understand why you have to replace dying units with new equal units, then kill those.
Can you explain? I'm curious.

				set dUnit = CreateUnit(dyingOwner, dyingTypeId, GetUnitX(dyingUnit), GetUnitY(dyingUnit), GetUnitFacing(dyingUnit))
				call RemoveUnit(dyingUnit)

I think that function AllowAllPlayers takes integer wTypeId, boolean allow returns boolean has a too generic function name.

function AddMeldAbil takes integer newAbilMeld returns boolean. This one also.

I think those two functions are not really necessary.

		//===========================================================================
		// Set Flying ability
		//===========================================================================
		function SetFlyAbil takes integer newAbilFly returns nothing
			set abilFly = newAbilFly
		endfunction

		//===========================================================================
		// Set Locus ability (unselectable)
		//===========================================================================
		function SetSelectAbil takes integer newAbilSelect returns nothing
			set abilSelect = newAbilSelect
		endfunction

Config

		//===========================================================================
		// Configure dissipate effect model
		//===========================================================================
		function ConfigureDissEffect takes string wRace, string dEffect returns boolean			
			if(wRace == "human") then
				set dEffectH = dEffect
			elseif(wRace == "orc") then
				set dEffectO = dEffect
			elseif(wRace == "nightelf") then
				set dEffectNE = dEffect
			elseif(wRace == "undead") then
				set dEffectU = dEffect
			elseif(wRace == "demon") then
				set dEffectD = dEffect
			else
				return false
			endif
			
			return true
		endfunction

		//===========================================================================
		// Configure dissipate sound
		//===========================================================================
		function ConfigureDissSound takes string wRace, sound dSound returns boolean			
			if(wRace == "human") then
				set dSoundH = dSound
			elseif(wRace == "orc") then
				set dSoundO = dSound
			elseif(wRace == "nightelf") then
				set dSoundNE = dSound
			elseif(wRace == "undead") then
				set dSoundU = dSound
			elseif(wRace == "demon") then
				set dSoundD = dSound
			else
				return false
			endif
			
			return true
		endfunction

		//===========================================================================
		// Configure revive effect model
		//===========================================================================
		function ConfigureRevEffect takes string wRace, string rEffect returns boolean			
			if(wRace == "human") then
				set rEffectH = rEffect
			elseif(wRace == "orc") then
				set rEffectO = rEffect
			elseif(wRace == "nightelf") then
				set rEffectNE = rEffect
			elseif(wRace == "undead") then
				set rEffectU = rEffect
			elseif(wRace == "demon") then
				set rEffectD = rEffect
			else
				return false
			endif
			
			return true
		endfunction

		//===========================================================================
		// Configure revive sound
		//===========================================================================
		function ConfigureRevSound takes string wRace, sound rSound returns boolean			
			if(wRace == "human") then
				set rSoundH = rSound
			elseif(wRace == "orc") then
				set rSoundO = rSound
			elseif(wRace == "nightelf") then
				set rSoundNE = rSound
			elseif(wRace == "undead") then
				set rSoundU = rSound
			elseif(wRace == "demon") then
				set rSoundD = rSound
			else
				return false
			endif
			
			return true
		endfunction

^Consider what I said about handle ids. You may change the string argument for race.

 

[loktar]

Cool system idea.

Thank you!

Correct me, but what happens if a player loses for example two or more unit of same type ( grunts ).
Can he revive the same amount or just one?

If the owner of the unit has been set to revive that unit-type, only one can be revived. If the owner has not been set to revive, either he can't train the unit-type at all, or unlimited like normal if set through AllowAllPlayers().

As everything is race handle based, you could also use one array for sound / effect per race
instead of a scalar variables per race. Handles ids:

// Reference: 1 - Human, 2 - Orc, 3 - Undead, 4 - Nightelf, 5 - Demon, 7 - Other, 11 - Naga

Hmm yeah, using arrays would be a lot better. If I'm understanding you correctly I could replace the if-statements with e.g. dSound[dRace] because e.g. RACE_HUMAN resolves to 1?
Does the race variable type have 11 possible values? Because I thought there were only the ones I used (human/orc/undead/NE/demon).

I don't understand why you have to replace dying units with new equal units, then kill those.
Can you explain? I'm curious.

I can't apply vertex colouring or lift the unit with the fly ability on a corpse, so I have to create a new, living unit and play the death animation.
Although thinking about it, I guess I could catch the unit right before it dies? But I'm not sure if that's reliable?

I think that function AllowAllPlayers takes integer wTypeId, boolean allow returns boolean has a too generic function name.

function AddMeldAbil takes integer newAbilMeld returns boolean. This one also.

I think those two function are not really necessary.

Hmm, I will see if I can think of something more descriptive About the AddMeldAbil and SetSelectAbil, should I replace them with udg's? Because I was asked to make all abilities configurable.

^Consider what I said about handle ids. You may change the string argument for race.

Yeah, could be a lot shorter with arrays. Same question as above though.



Thanks for your feedback! I'll implement it as much as possible when I have some time.

 

[BPower]

Hmm yeah, using arrays would be a lot better. If I'm understanding you correctly I could replace the if-statements with e.g. dSound[dRace] because e.g. RACE_HUMAN resolves to 1?

Yes.

Does the race variable type have 11 possible values? Because I thought there were only the ones I used (human/orc/undead/NE/demon).

Make sure you support the classic races orc/human/undead/nightelf. The others are optional.

I can't apply vertex colouring or lift the unit with the fly ability on a corpse, so I have to create a new, living unit and play the death animation.
Although thinking about it, I guess I could catch the unit right before it dies? But I'm not sure if that's reliable?

Mmh ok. The dummy unit for vertex color and death effect makes totally sense.
I think it's better to hide the original unit instead of removing it upon death event.

 

[loktar]

Thanks for approval! I will definitely implement race handle id based arrays for effects/sounds and their config functions, seems a lot better than what I have now.
I'll see about implementing an extra "stock" array so each unit that dies adds to the amount that can be revived.

Hiding the unit instead of deleting - because it gets erased from memory anyway when it has decayed? Makes sense.

Edit: I'm curious, where can I find a list of all race handle ids? (1-11)? Can't find any documentation on it.
Edit2: alright, this works quite nicely, and it's so much simpler

				set rRace = GetHandleId(GetUnitRace(rUnit))
					
				if(rEffect[rRace] != null and rEffect[rRace] != "") then
					call DestroyEffect(AddSpecialEffectTarget(rEffect[rRace], rUnit, "origin"))
				endif
				if(rSound[rRace] != null) then
					set rSoundTemp = rSound[rRace]
					call AttachSoundToUnit(rSoundTemp, rUnit)
					call StartSound(rSoundTemp)
					set rSoundTemp = null
				endif

still have to update the configuration stuff, and then do the rest of your feedback

Edit: alright, done. Might add the possibility to revive a unit-type for each unit that dies later