Are there any hidden problems with this?

[Weep]

Will this cause any problems I don't know about? Like, will it pollute the string table or anything?

I am, of course, aware of the obvious (taking a lot of computation to do something simple: avoiding having to update the rawcode for an ability every time you paste into a new map, without having ObjectMerger available).

globals
    integer start = 'A000'
    integer end = 'A500'
endglobals

globals//locals
    integer index = 0
    string goalName = ""
    integer result = 0
endglobals

function FindAbilityNewThread takes nothing returns nothing
    local integer i = index+250

    loop
        exitwhen index == i
        if GetObjectName(index) == goalName then
            set result = index
            exitwhen true
        endif
        set index = index+1
    endloop
endfunction

function FindAbilityByName takes string abilName returns integer
    local trigger t = CreateTrigger()
    local triggeraction a = TriggerAddAction(t, function FindAbilityNewThread)
    
    set index = start
    set result = 0
    set goalName = abilName
    loop
        exitwhen result > 0 or index > end
        call TriggerExecute(t)
    endloop

    call TriggerRemoveAction(t, a)
    call DestroyTrigger(t)
    set t = null
    set a = null
    return result
endfunction

 

[SineCosine]

Why not:

function FindAbilityByName takes string abilName returns integer
    set index = start
    set result = 0
    set goalName = abilName
    loop
        exitwhen result > 0 or index > end
        set result = FindAbilityNewThread()
    endloop

    return result
endfunction

//Or even this, if you don't need that loop
function FindAbilityByName takes string abilName returns integer
    set index = start
    set result = 0
    set goalName = abilName

    return FindAbilityNewThread()
endfunction

//Better yet, make FindAbilityNewThread() take the string 'abilName' and return an integer, so you save a function call ._.

I don't really know what this is for but..
Yea =x

if Result never becomes higher than 0..
Or index never becomes higher than end..

Won't it loop infinitely? o.0
If it breaks its loop after one go..

Wouldn't it make the loop pointless?

 

[Bribe]

Why don't you use [ljass]ExecuteFunc("FindAbilityNewThread")[/ljass] instead of wasting all those local triggers each time?

Not just that, but this whole thing is a mess. Why wouldn't you just run this once at map initialization and set everything to variables?

 

[SineCosine]

ExecuteFunc("FindAbilityNewThread") <-- That's horrible >.>

It's the same as: [lJASS]call FindAbilityNewThread(/*Arguments*/)[/lJASS]
Isn't it?

 

[Bribe]

No, it starts a new thread, just like TriggerExecute does.

 

[Weep]

It's the same as: [lJASS]call FindAbilityNewThread(/*Arguments*/)[/lJASS]
Isn't it?

No, as Bribe mentioned, it starts a new thread in order to avoid the operation limit. It might take hundreds of thousands of loops in order to find the ability, far beyond the single-thread op limit at which WC3 would have prematurely terminated the procedure.

Why don't you use [ljass]ExecuteFunc("FindAbilityNewThread")[/ljass] instead of wasting all those local triggers each time?

I thought I'd read that ExecuteFunc can be broken by map optimizers, which may rename a function but not notice that it was called in an ExecuteFunc. Besides, doesn't JassHelper use triggers for .execute and claims it's faster than ExecuteFunc?

Not just that, but this whole thing is a mess. Why wouldn't you just run this once at map initialization and set everything to variables?

Uh, I would just run it once at map init and set everything to variables. That's its purpose.

Any further comments? I'm particularly concerned about any problems with strings, because this is abnormally intensive with them.

 

[Bribe]

Well running this once at map initialization for one ability, or running this for all abilities? Due to the power of this thing, it would be better served to log each ability during this process, instead of being spent to find merely one.

But yeah, in that case, since you're not doing this more than once, might as well use the TriggerAction bit over the ExecuteFunc, because it will be faster.

 

[PurgeandFire]

Any further comments? I'm particularly concerned about any problems with strings, because this is abnormally intensive with them.

Considering GetObjectName() would probably return a null string most of the time (or "", or whatever), I don't think it will be that terrible.

 

[Weep]

Considering GetObjectName() would probably return a null string most of the time (or "", or whatever), I don't think it will be that terrible.

Actually, it's "Default string", but thanks for the feedback.

 

[Sevion]

In my experience, triggers will leak that until it ends. I learned this from Nestharus's old infinite loops in MFN. While they worked, if they leaked too much, you would eventually freeze until the loop finished.

 

[Weep]

In my experience, triggers will leak that until it ends.

I'm sorry, what is the "that" which leaked?

I learned this from Nestharus's old infinite loops in MFN. While they worked, if they leaked too much, you would eventually freeze until the loop finished.

Isn't it normal for the game to hang during a loop that takes a long time to compute, having nothing to do with leaking?

 

[Sevion]

What I meant was leaks with TriggerExecute or w/e it was.

And the thing was that the loop actually didn't cause it to hang unless your input values were screwy. If you found the perfect combination, it'd actually run smoothly.

 

[Bribe]

What? did I post something?

 

[PurgeandFire]

Conditions run on the same thread as the trigger. Unless I'm mistaken.

 

[Bribe]

Uh, I didn't post that. Someone must have hacked my account

 

[Troll-Brain]

I wouldn't go that way.
I would perform the check only one time at map initialisation and store the valid spell ids inside an hashtable.
Also strings are horribly handled in jass2, i would use a dummy unit and the returned boolean of UnitAddAbility instead for the initial scan.

Btw, do you know that GetObjectName returns a local string which depends wc3 language ?
Ofc it happens only if you don't have edited the name of the spell yourself, else it will be obviously the same for all.

 

[Weep]

I wouldn't go that way.
I would perform the check only one time at map initialisation and store the valid spell ids inside an hashtable.

Good idea - I had the same thought last night. Although, I'd probably store ability IDs against the StringHash of the object name.

Also strings are horribly handled in jass2, i would use a dummy unit and the returned boolean of UnitAddAbility instead for the initial scan.

That wouldn't really help with the goal of finding a specific ability's rawcode, just whether any ability exists for a certain rawcode, right?

Btw, do you know that GetObjectName returns a local string which depends wc3 language ?
Ofc it happens only if you don't have edited the name of the spell yourself, else it will be obviously the same for all.

Well, since this is meant only for custom abilities, it wouldn't be a problem.

 

[Troll-Brain]

Good idea - I had the same thought last night. Although, I'd probably store ability IDs against the StringHash of the object name.

That is what i had in mind.

That wouldn't really help with the goal of finding a specific ability's rawcode, just whether any ability exists for a certain rawcode, right?

Wrong, you still can link the ability name with the spell id in an hashtable or even gamecache.

 

[Weep]

Wrong, you still can link the ability name with the spell id in an hashtable or even gamecache.

Oh, I see. Well, I've tried that now, and for unknown reasons is causing a freeze.

This works, using strings:

function GFL_IndexAbilitiesNewThread takes nothing returns nothing
	local integer i = bj_meleeTwinkedHeroes[4576]+250

	loop
		exitwhen bj_meleeTwinkedHeroes[4576] == i or bj_meleeTwinkedHeroes[4577] >= 500
		if GetObjectName(bj_meleeTwinkedHeroes[4576]) != "Default string" then
			call SaveInteger(udg_GFL__Hashtable, StringHash(GetObjectName(bj_meleeTwinkedHeroes[4576])), 0, bj_meleeTwinkedHeroes[4576])
			set bj_meleeTwinkedHeroes[4577] = 0
		else
			set bj_meleeTwinkedHeroes[4577] = bj_meleeTwinkedHeroes[4577]+1
		endif
		set bj_meleeTwinkedHeroes[4576] = bj_meleeTwinkedHeroes[4576]+1
	endloop
endfunction                                                               

function GFL_IndexAbilities takes nothing returns nothing
	local trigger t = CreateTrigger()
	local triggeraction a = TriggerAddAction(t, function GFL_IndexAbilitiesNewThread)
	local integer cache1 = bj_meleeTwinkedHeroes[4576]
	local integer cache2 = bj_meleeTwinkedHeroes[4577]
	
	set bj_meleeTwinkedHeroes[4576] = 'A000'
	loop
		exitwhen bj_meleeTwinkedHeroes[4577] >= 500
		call TriggerExecute(t)
	endloop

	set bj_meleeTwinkedHeroes[4576] = cache1
	set bj_meleeTwinkedHeroes[4577] = cache2
	call TriggerRemoveAction(t, a)
	call DestroyTrigger(t)
	set t = null
	set a = null
endfunction

function GFL_GetAbilityByName takes string name returns integer
	if udg_GFL__Hashtable == null then
		set udg_GFL__Hashtable = InitHashtable()
		call GFL_IndexAbilities()
	endif
	return LoadInteger(udg_GFL__Hashtable, StringHash(name), 0)
endfunction

This freezes on launch, using UnitAddAbility/UnitRemoveAbility:

function GFL_IndexAbilitiesNewThread takes nothing returns nothing
	local integer i = bj_meleeTwinkedHeroes[4576]+250

	loop
		exitwhen bj_meleeTwinkedHeroes[4576] == i or bj_meleeTwinkedHeroes[4577] >= 500
		if UnitAddAbility(bj_ghoul[4672], bj_meleeTwinkedHeroes[4576]) or UnitRemoveAbility(bj_ghoul[4672], bj_meleeTwinkedHeroes[4576]) then
			call SaveInteger(udg_GFL__Hashtable, StringHash(GetObjectName(bj_meleeTwinkedHeroes[4576])), 0, bj_meleeTwinkedHeroes[4576])
			set bj_meleeTwinkedHeroes[4577] = 0
		else
			set bj_meleeTwinkedHeroes[4577] = bj_meleeTwinkedHeroes[4577]+1
		endif
		set bj_meleeTwinkedHeroes[4576] = bj_meleeTwinkedHeroes[4576]+1
		call UnitRemoveAbility(bj_ghoul[4672], bj_meleeTwinkedHeroes[4576])
	endloop
endfunction

function GFL_IndexAbilities takes nothing returns nothing
	local trigger t = CreateTrigger()
	local triggeraction a = TriggerAddAction(t, function GFL_IndexAbilitiesNewThread)
	local integer cache1 = bj_meleeTwinkedHeroes[4576]
	local integer cache2 = bj_meleeTwinkedHeroes[4577]
	local unit cache3 = bj_ghoul[4672]
	
	set bj_ghoul[4672] = CreateUnit(Player(PLAYER_NEUTRAL_PASSIVE), 'hfoo', 0, 0, 0)
	set bj_meleeTwinkedHeroes[4576] = 'A000'
	loop
		exitwhen bj_meleeTwinkedHeroes[4577] >= 500
		call TriggerExecute(t)
	endloop

	call RemoveUnit(bj_ghoul[4672])
	set bj_meleeTwinkedHeroes[4576] = cache1
	set bj_meleeTwinkedHeroes[4577] = cache2
	set bj_ghoul[4672] = cache3
	call TriggerRemoveAction(t, a)
	call DestroyTrigger(t)
	set t = null
	set a = null
	set cache3 = null
endfunction

function GFL_GetAbilityByName takes string name returns integer
	if udg_GFL__Hashtable == null then
		set udg_GFL__Hashtable = InitHashtable()
		call GFL_IndexAbilities()
	endif
	return LoadInteger(udg_GFL__Hashtable, StringHash(name), 0)
endfunction

 

[Troll-Brain]

The freez is most likely because of the first add of each ability (preload), but be happy, it won't freez later in game if you plan to add these abilities during the game :thup:

You can use [ljass]UnitRemoveAbility[/ljass] only is the if is true, no need to remove it if it's false.
Also the [ljass]or UnitRemoveAbility[/ljass] is useless as you long you use a dummy unit without any ability owned.

Oh and btw i've read that widgetize the map (a .slk file for abilities) make the preloading useless, no more lag on first ability adding, but i have never tested this claim though.

Also you shouldn't notice the freez if you use the 'event' map initialization , right ?

EDIT :

But i've just realized that if you don't care about this preload thing, and don't use custom rawcode, the first solution is just better.

EDIT 2 : I've found my old test map, i confirm that i used GetUnitAbilityLevel for the scan, so i can't state that UnitAdd/RemoveAbility is safe with invalid ability id.