[JASS] return syntax error

[redmarine]

Hi, I've finally gotten myself to start working with v/Jass but I'm having issues with retrieving a boolean.

Basically what the system does is to create a battalion. First step is to check whether the unit is listed and eligible. But here is where the problem starts. I simply don't know how to make UnitIdCondition take the 'i' variable from CreateB and change its value if the unit is eligible or not and then pass it back to CreateB.

Any suggestions? What I want is an effective fix not some crappy GUI convert.

Oh btw, the CreateBM function works flawlessly so you don't have to look it up.

Problem fixed. Although I'd appreciate if you could help me optimize my code though.(reposed fixed code)

scope Initialization initializer Init

private function UnitIdCondition takes nothing returns boolean //Check unit list wether the unit is allowed.
    if GetUnitTypeId(GetTrainedUnit()) == 'hfoo' then
        return true
    endif
        return false
endfunction 

private function CreateBM takes nothing returns nothing //Create Battalion Members,
    local unit temp = GetTrainedUnit()                  //remove trained unit,
    local integer i = 0                                 //create 12 group members
    call RemoveUnit(temp)
    loop
        exitwhen i > 11
        call CreateUnit(GetOwningPlayer(temp), GetUnitTypeId(temp), GetUnitX(temp), GetUnitY(temp), 0.00)
        set i = i + 1
    endloop
endfunction

private function CreateB takes nothing returns nothing  //Creating a battalion step by step by
    local boolean i = false
    set i = UnitIdCondition()
    if i == true then
        call CreateBM()
    endif
endfunction 

private function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_TRAIN_FINISH)
    call TriggerAddAction(t, function CreateB)
endfunction

endscope

 

[Mooglefrooglian]

private function UnitIdCondition takes nothing returns boolean //Check unit list wether the unit is allowed.
    if GetUnitTypeId(GetTrainedUnit()) == 'hfoo' then
        return true
    endif
        return false
endfunction

Should be:

private function UnitIdCondition takes nothing returns boolean //Check unit list wether the unit is allowed.
    return GetUnitTypeId(GetTrainedUnit()) == 'hfoo'
endfunction

And also for that, learn to indent properly please. Anything within an "if" chunk shoudl have one more tab. To write it properly:

private function UnitIdCondition takes nothing returns boolean //Check unit list wether the unit is allowed.
    if GetUnitTypeId(GetTrainedUnit()) == 'hfoo' then
        return true
    endif
    return false
endfunction

private function CreateBM takes nothing returns nothing //Create Battalion Members,
    local unit temp = GetTrainedUnit() //remove trained unit,
    local integer i = 0 //create 12 group members
    call RemoveUnit(temp)
    loop
        exitwhen i > 11
        call CreateUnit(GetOwningPlayer(temp), GetUnitTypeId(temp), GetUnitX(temp), GetUnitY(temp), 0.00)
        set i = i + 1
    endloop
endfunction

I am quite unsure, but all of those GetUnitX() and such should be placed in variables I think. Not strictly neccesary though, and the way you have it makes code much simpler.

Also, why are you using GetUnitTypeId()? You KNOW it has to be 'hf00' thanks to your condition.

call CreateUnit(GetOwningPlayer(temp), 'hfoo', GetUnitX(temp), GetUnitY(temp), 0.00)

BUT:

call RemoveUnit(temp)

Should be placed at the end or your code will NOT work. Because once you remove a unit, you cant get its X, Y, or unit type or anything. SO fix that.

private function CreateB takes nothing returns nothing //Creating a battalion step by step by
    local boolean i = false
    set i = UnitIdCondition()
    if i == true then
        call CreateBM()
    endif
endfunction

What is this? Use conditions properly in your trigger if you want something like that, or else just do a one line if in your function: if its false, just call return to exit. This is a huge waste of code. And try to name your variables properly. i? i is the basic loop variable. Use b or else use "isRightUnit" or something. For the sake of my sanity.

private function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_TRAIN_FINISH)
    call TriggerAddAction(t, function CreateB)
endfunction

Set t = null at the end or else it leaks. Not that it matters for such a small leak, but if you insist on doing a scope out of all of this....

This entire thing is in a scope. Why? You have no need of scopes here because you arent using globals. Unless you want this as a part of a normal library or larger chunk of code, create a gui trigger, convert to custom text, and use that init function.

Unless your trying to save yourself a global trigger, in which case I guess its fine. But you really dont have to put private in front of your function names in that case.

Your final code should be:

function Trig_Untitled_Trigger_001_Actions takes nothing returns nothing
    local unit u = GetTrainedUnit()
    local integer i = 0
    //Make sure our unit-type is right.
    if GetUnitTypeId(u) != 'hfoo' then
        set u = null
        return
    endif
    //create 12 group members
    loop
        exitwhen i > 11
        call CreateUnit(GetOwningPlayer(u), 'hfoo', GetUnitX(u), GetUnitY(u), 0.0) 
        set i = i + 1
    endloop
    //remove the trained unit
    call RemoveUnit(u)
endfunction

//===========================================================================
function InitTrig_Untitled_Trigger_001 takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_TRAIN_FINISH)
    call TriggerAddAction(t, function Trig_Untitled_Trigger_001_Actions)
    set t = null
endfunction

Strictly speaking, using t wasnt even required and I could/shoud lhave used the global. But whatever.

I apologise if I seemed passive aggressive, I'm sorta tired.

As a final note: variable names really dont matter, but please NEVER use "i" unless your using it as the index in a loop. Its very very bad practice. I am also unsure on the efficiency of using GetUnitX() a bunch of times, but I frankly wouldnt waste the space coding in 5 new reals and players.

If you ever need anything more, do pm me if you want :O.

 

[TriggerHappy]

scope Initialization initializer Init

	private function CreateBM takes nothing returns nothing //Create Battalion Members,
		local unit temp = GetTrainedUnit()                  //remove trained unit,
		local integer i = 0                                 //create 12 group members
		if (GetUnitTypeId(temp) == 'hfoo')
			call RemoveUnit(temp)
			loop
				exitwhen i > 11
				call CreateUnit(GetOwningPlayer(temp), GetUnitTypeId(temp), GetUnitX(temp), GetUnitY(temp), 0.00)
				set i = i + 1
			endloop
		endif
		set u = null
		return false
	endfunction

	private function Init takes nothing returns nothing
		local trigger t = CreateTrigger()
		call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_TRAIN_FINISH)
		call TriggerAddCondition(t, function CreateBM)
	endfunction

endscope

 

[Mooglefrooglian]

	private function CreateBM takes nothing returns nothing
	return false

What?

 

[TriggerHappy]

Conditions are faster.
I forgot to add a returns boolean at the end.

 

[Mooglefrooglian]

Can you give me a link where it says coniditions are faster by chance? Because logic dictates that would be a major coding error by Blizzard if it was true.

 

[YourNameHere]

Can you give me a link where it says coniditions are faster by chance? Because logic dictates that would be a major coding error by Blizzard if it was true.

Use your common sense, conditions get evaluated before actions get executed, right?

 

[Mooglefrooglian]

Use your common sense, conditions get evaluated before actions get executed, right?

Use your logic, that doesn't make them faster. They just get executed first. If there is no condition, then it executes the action. It's just as fast.

 

[GhostWolf]

scope Initialization initializer Init

	private function CreateBM takes nothing returns nothing //Create Battalion Members,
		local unit temp = GetTrainedUnit()                  //remove trained unit,
		local integer i = 0                                 //create 12 group members
		if (GetUnitTypeId(temp) == 'hfoo')
			call RemoveUnit(temp)
			loop
				exitwhen i > 11
				call CreateUnit(GetOwningPlayer(temp), GetUnitTypeId(temp), GetUnitX(temp), GetUnitY(temp), 0.00)
				set i = i + 1
			endloop
		endif
		set u = null
		return false
	endfunction

	private function Init takes nothing returns nothing
		local trigger t = CreateTrigger()
		call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_TRAIN_FINISH)
		call TriggerAddCondition(t, function CreateBM)
	endfunction

endscope

You're trying to get parameters of a destroyed unit. You're also trying to do it 12 times instead of one. And finally, I suppose that "set u" should've been "set temp"?

 

[PurplePoot]

Conditions are faster.

So we should use them even when it's pointless and stupid? Back to the Helper with those attitudes please.