[JASS] Crafting system efficiency

[IIazaH4er]

(Yes, I know I can find already-made system and copy it, but it isnt fun)
I have a custom map with a lot of recipes (200 for sure). Currently all recipes are written via GUI and scattered at ~30 triggers (sorted by type). Lately I thought about that and concluded that it takes tons of processing power (it works well and without delay tho).
I think about making one jass function with tons of inputs as base and one trigger to launch all possible combinations of recipes.

Current GUI base

• Events
  • Unit casts a spell
• Conditions
  • Spell Equal to Crafting
• Actions
  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    • If - Conditions
      • (Triggering unit) has an item of type Item1
      • (Triggering unit) has an item of type Item2
    • Then - Actions
      • Remove item of type Item1 from (Triggering unit)
      • Remove item of type Item2 from (Triggering unit)
      • Give item of type ItemRes to (Triggering unit)
    • Else - Actions

Future JASS base

function Craft takes unit crafter itemtype result itemtype item1 itemtype item2 .......... itemtype item6 returns nothing
 if (Unit has an item of type item1) and (Unit has an item of type item2) .... then
  Remove item of type item1 from crafter
  Remove item of type item2 from crafter
  .....
  Give item of type result to crafter
 endif
endfunction
//Too lazy to write actuall functions in

And then just lauch it 300+ times in response to casting Craft-skill.

call Craft(unit,I00A,I001,I002,I003....)
call Craft(unit,I00B,I004,I005,I006....)
call Craft(unit,I00C,I007,I008,I009....)

The Questions are:
1) Will it take less processing power than GUI one? If no, then there is no point in doing this
2) How will my jass-base react to 'null' in itemtype input in recipes with less than 6 items?
2.1) If trigger itself is okay, how will RemoveItem react to 'null'?
3) Should I "set null" to all used variables in the end or it wont make a leak w/o that?
4) Is there a jass analogy of C's 'short-circuiting' logical operators?

 

[TriggerHappy]

1. Barely.
2. If it's == 0 then ignore it & Check if it's null before removing
3. If there's a chance they will get used again without being overwritten, then set everything back to 0/null.
4. http://jass.sourceforge.net/doc/expressions.shtml

If you're not going to do it completely in vJASS I would just keep it in GUI.

Make multiple arrays which define the data for each recipe, then run a loop through all of them.

Manually writing an if statement for 200 recipes is a bad idea.

I suggest taking a look at RecipeSYS.

 

[Anitarf]

Native JASS code will be faster than GUI triggers since there will be fewer wrappers and trigger evaluations involved, but since your approach remains effectively the same do not expect significant improvements. To get better results, you should use a more intelligent algorithm instead of going through a list of all possible recipes. Also, it'd be nice if your code could support recipes that require multiple items of the same type.

 

[IIazaH4er]

If it's == 0 then ignore it & Check if it's null before removing

The question was "what will happen if", not "how to avoid nulls".

http://jass.sourceforge.net/doc/expressions.shtml

Could have just said "No, there isnt".

If you're not going to do it completely in vJASS I would just keep it in GUI.

Completely? What else except base function can be made in jass here?

Make multiple arrays which define the data for each recipe, then run a loop through all of them.

Good idea, but data-filling will take more space in triggers. I guess size-optimization is my goal too.

Manually writing an if statement for 200 recipes is a bad idea.

If it is barely better than jass one, then why is it bad? I mean, I know that jass is better than GUI almost at everything, but still...

I suggest taking a look at RecipeSYS.

I dont need already-made systems. They have too many functions that I dont need and its hard to extract specific function out of it.

To get better results, you should use a more intelligent algorithm instead of going through a list of all possible recipes.

Weeeelll... There is a possibility to sort recipes by used items and sort-out them in the process of checking, but I dont think that this will be very effective due to variety of recipes in my map.

Also, it'd be nice if your code could support recipes that require multiple items of the same type.

I can just write in same itemtype multiple times.

 

[Anitarf]

I can just write in same itemtype multiple times.

You're welcome to try.

 

[TriggerHappy]

You need to learn how to respond to someone trying to help you.

The question was "what will happen if", not "how to check for nulls".

You asked how will it react and I told you how it should react.

Could have just said "No, there isnt".

I don't even know what the term short circuit operators means so I linked to you all available JASS operators.

Good idea, but data-filling will take more space in triggers. I guess size-optimization is my goal too.

and 200+ if statements won't take up space?

If it is barely better than jass one, then why is it bad?

Writing 200 if statements manually in any language is probably a bad idea.

Use looping and arrays like I suggested before.

I dont need already-made systems. They have too many functions that I dont need.

I said to take a look at it. You can get an idea of how a recipe system should function.

 

[DracoL1ch]

IF statements actually faster than loop in many cases. normally it's advised to use IF in cases when you know exact amount of comparations, but we aren't talking about efficience here and its still negliglible.

overall, i'd suggest to go throw all combinations with loop/if, whatever you like.

 

[Wietlol]

function Craft takes unit crafter itemtype result itemtype item1 itemtype item2 .......... itemtype item6 returns nothing
 if (Unit has an item of type item1) and (Unit has an item of type item2) .... then
  Remove item of type item1 from crafter
  Remove item of type item2 from crafter
  .....
  Give item of type result to crafter
 endif
endfunction
//Too lazy to write actuall functions in

What about this?

    //Checks if the given unit has items of the given types.
    //Fill the parameters with 0 to have fewer required item types.
    //Works with multiple items of the same type.
    function UnitCheckItems takes unit whichUnit, integer i1, integer i2, integer i3, integer i4, integer i5, integer i6 returns boolean
        local integer array heldItems
        local integer array requestedItems
        local integer index1
        local integer index2
        local integer requestedItemsLength = 6
        
        set requestedItems[0] = i1
        set requestedItems[1] = i2
        set requestedItems[2] = i3
        set requestedItems[3] = i4
        set requestedItems[4] = i5
        set requestedItems[5] = i6
        set heldItems[0] = GetItemTypeId(UnitItemInSlot(whichUnit, 0))
        set heldItems[1] = GetItemTypeId(UnitItemInSlot(whichUnit, 1))
        set heldItems[2] = GetItemTypeId(UnitItemInSlot(whichUnit, 2))
        set heldItems[3] = GetItemTypeId(UnitItemInSlot(whichUnit, 3))
        set heldItems[4] = GetItemTypeId(UnitItemInSlot(whichUnit, 4))
        set heldItems[5] = GetItemTypeId(UnitItemInSlot(whichUnit, 5))
        
        set index1 = 0
        loop
            if requestedItems[index1] == 0 then
                set requestedItemsLength = requestedItemsLength-1
                set requestedItems[index1] = requestedItems[requestedItemsLength]
                if requestedItemsLength == 0 then
                    return true
                endif
            endif
            exitwhen index1 == 5
            set index1 = index1 +1
        endloop
        
        set index1 = 0
        loop
            set index2 = 0
            loop
                exitwhen index2 >= requestedItemsLength
                
                if heldItems[index1] == requestedItems[index2] then
                    set requestedItemsLength = requestedItemsLength-1
                    set requestedItems[index2] = requestedItems[requestedItemsLength]
                    if requestedItemsLength == 0 then
                        return true
                    endif
                    exitwhen true
                endif
                
                set index2 = index2 +1
            endloop
            
            exitwhen index1 == 5
            set index1 = index1 +1
        endloop
        
        return false
    endfunction
    
    function Craft takes unit crafter, integer result, integer item1, integer item2, integer item3, integer item4, integer item5, integer item6 returns boolean
        if UnitCheckItems(crafter, item1, item2, item3, item4, item5, item6) then
            if item1 > 0 then
                call UnitRemoveItem(crafter, GetItemOfTypeFromUnitBJ(crafter, item1)
            endif
            //etc
            
            call UnitAddItemById(crafter, result)
            return true
        endif
        return false
    endfunction
    
    function foo takes unit u returns nothing
        call Craft(u, 'I000', 'I001', 'I001', 'I002', 'I003', 0, 0)
    endfunction

UnitCheckItems() checks for all items you give in as parameters, you can use 0 for nothing and you can check the items multiple times.
That way you can use the Craft method to craft the items.
The return value will be wether the new item was created, aka if the unit had the required items.

The use of JASS is that you dont have to write this manually all the time in GUI, which is horribly slow to create triggers in.

 

[IIazaH4er]

You need to learn how to respond to someone trying to help you.

Sorry if I offended you somehow. English isnt my first language, after all.

You asked how will it react and I told you how it should react.

Oops. Your answer looked to me like suggestion to add null-checking in craft. Now I got what you meant.

and 200+ if statements won't take up space?

Thats why I'm trying to compress it in one recallable base function.

What about this?

Looks great, but:

1. What the point of using separated UnitCheckItems function if it will be used only one time in Craft? Isnt it better to combine them together?
2. Is lopped checking of all items hero owns actualy faster than default UnitHasItemOfTypeBJ?

 

[Zunariel]

I just want to give some input, maybe it serves your purpose.

The Crafting System I use in TEoP Sechiron Rises and in my new campaign works pretty easily with Hashtables.

• Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • 'IF'-Bedingungen
  • 'THEN'-Aktionen
    • Set EquipString = Adept Staff
    • Hashtabelle - Save Iron Bar as 1 of (Key EquipString) in CraftHash
    • Hashtabelle - Save 1 as 2 of (Key EquipString) in CraftHash
    • Hashtabelle - Save Metal Hilt as 3 of (Key EquipString) in CraftHash
    • Hashtabelle - Save 1 as 4 of (Key EquipString) in CraftHash
    • Hashtabelle - Save -------------- as 5 of (Key EquipString) in CraftHash
    • Hashtabelle - Save 0 as 6 of (Key EquipString) in CraftHash
    • Hashtabelle - Save -------------- as 7 of (Key EquipString) in CraftHash
    • Hashtabelle - Save 0 as 8 of (Key EquipString) in CraftHash
    • Hashtabelle - Save -------------- as 9 of (Key EquipString) in CraftHash
    • Hashtabelle - Save 0 as 10 of (Key EquipString) in CraftHash
    • Hashtabelle - Save 1 as 11 of (Key EquipString) in CraftHash
    • Hashtabelle - Save 5 as 12 of (Key EquipString) in CraftHash
  • 'ELSE'-Aktionen

Here is an example, which I made in the beginning for testing purposes.
The 'Iron Bar' and the 'Metal Hilt' arent saved unit/item handles, they are simply strings, so the system searches for certain strings and if these match the names in your item inventory, then the next row checks how many stacks you got and if you got all the items plus the stacks it checks your skill (Row 11 saved as 1 = Blacksmithing and Row 12 saved as 5 = the required Blacksmithing Skill) and if you also match that then it removes the stacks and so you get your item.

The EquipString at the top is the Name of the Equipment Part you want to craft.

If I want to make a new recipe I just copy paste this hashtable set the EquipString to the Crafted Item and easily type in Iron Bar, Gold Bar, Copper Bar, whatever one needs and the stacks and there you go. It needed a few triggers to make and is super flexible.

 

[Wietlol]

"What the point of using separated UnitCheckItems function if it will be used only one time in Craft? Isnt it better to combine them together?"

Take it from me, just do it.

As a programmer, I prefer my functions/methods to do one thing.
Craft will have the task of creating the items out of the recipe.
CheckUnitItems will have the task of checking if a unit has certain items.
You dont want to put the code from CheckUnitItems in Craft because you have to do the exact thing that is done in that.

It is also much more readable.
You can see in the code from Craft that a check is done on the crafter with the given item types.
If it succeeds, you remove the items and create the result.
If it fails, you do nothing.
If you would put all the code from CheckUnitItems in Craft, you would make it much less readable.

CheckUnitItems is a function that can be used by other things but is not necessarily used at all.

Also, remember that UnitHasItemOfTypeBJ does return true if you check if a unit has 2 of the same items even if your unit only has one.
CheckUnitItems will return false if done in one call.

That is one of the uses of that function.