Looking for code optimizers, please

Talk about creating Grimrock 1 levels and mods here. Warning: forum contains spoilers!
Post Reply
User avatar
cromcrom
Posts: 549
Joined: Tue Sep 11, 2012 7:16 am
Location: Chateauroux in a socialist s#!$*&% formerly known as "France"

Looking for code optimizers, please

Post by cromcrom »

¨Please don't make fun of me :oops:
I have been working on a portable scavenging item (it works really great, I will include it in next release of TLC.)

however, my code looks like this (I am a self taught coder :-( ) :

Code: Select all

function scavengehammerCheck(mouseitem,champ,slot)
local objecttoscavenge = mouseitem.name
if string.find(objecttoscavenge,"dagger") ~=nil then scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)
else if string.find(objecttoscavenge,"cutlass") ~=nil then scavengehammer("weaponsmith","iron_chunks",5,7,champ,slot)
else if string.find(objecttoscavenge,"fist_dagger") ~=nil then scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)
else if string.find(objecttoscavenge,"flail") ~=nil then scavengehammer("weaponsmith","iron_chunks",3,6,champ,slot)
else if string.find(objecttoscavenge,"great_axe") ~=nil then scavengehammer("weaponsmith","iron_chunks",5,7,champ,slot)
else if string.find(objecttoscavenge,"hand_axe") ~=nil then scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)
else if string.find(objecttoscavenge,"knife") ~=nil then scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)
else if string.find(objecttoscavenge,"long_sword") ~=nil then scavengehammer("weaponsmith","iron_chunks",5,7,champ,slot)
else if string.find(objecttoscavenge,"machete") ~=nil then scavengehammer("weaponsmith","iron_chunks",4,6,champ,slot)
else if string.find(objecttoscavenge,"shuriken") ~=nil then scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)	
else if string.find(objecttoscavenge,"throwing_axe") ~=nil then scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)	
else if string.find(objecttoscavenge,"throwing_knife") ~=nil then scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)	
else if string.find(objecttoscavenge,"warhammer") ~=nil then scavengehammer("weaponsmith","iron_chunks",5,7,champ,slot)	
else if string.find(objecttoscavenge,"battle_axe") ~=nil then scavengehammer("weaponsmith","iron_chunks",5,7,champ,slot)
	
	
else if string.find(objecttoscavenge,"arrow") ~=nil then scavengehammer("woodworker","wood_branch",1,1,champ,slot)	
else if string.find(objecttoscavenge,"crossbow") ~=nil then scavengehammer("woodworker","wood_branch",1,2,champ,slot)	
else if string.find(objecttoscavenge,"knoffer") ~=nil then scavengehammer("woodworker","wood_branch",1,2,champ,slot)	
else if string.find(objecttoscavenge,"short_bow") ~=nil then scavengehammer("woodworker","wood_branch",1,2,champ,slot)	
else if string.find(objecttoscavenge,"longbow") ~=nil then scavengehammer("woodworker","wood_branch",1,2,champ,slot)	
else if string.find(objecttoscavenge,"quarrel") ~=nil then scavengehammer("woodworker","wood_branch",1,1,champ,slot)	
else if string.find(objecttoscavenge,"wooden_box") ~=nil then scavengehammer("woodworker","wood_branch",2,4,champ,slot)	
else if string.find(objecttoscavenge,"camp_component") ~=nil then scavengehammer("woodworker","wood_branch",1,1,champ,slot)	
else if string.find(objecttoscavenge,"torch") ~=nil then scavengehammer("woodworker","wood_branch",1,1,champ,slot)
else if string.find(objecttoscavenge,"legionary_spear") ~=nil then scavengehammer("woodworker","wood_branch",1,3,champ,slot)
else if string.find(objecttoscavenge,"legionary_shield") ~=nil then scavengehammer("armorsmith","iron_chunks",2,4,champ,slot)	
	
	
else if string.find(objecttoscavenge,"hide_vest") ~=nil then scavengehammer("leathercraft","leather",2,4,champ,slot)	
else if string.find(objecttoscavenge,"huntsman_cloak") ~=nil then scavengehammer("leathercraft","leather",1,1,champ,slot)	
else if string.find(objecttoscavenge,"leather_boots") ~=nil then scavengehammer("leathercraft","leather",1,1,champ,slot)	
else if string.find(objecttoscavenge,"leather_brigandine") ~=nil then scavengehammer("leathercraft","leather",1,2,champ,slot)	
else if string.find(objecttoscavenge,"leather_cap") ~=nil then scavengehammer("leathercraft","leather",1,1,champ,slot)	
else if string.find(objecttoscavenge,"leather_gloves") ~=nil then scavengehammer("leathercraft","leather",1,1,champ,slot)	
else if string.find(objecttoscavenge,"leather_greaves") ~=nil then scavengehammer("leathercraft","leather",1,1,champ,slot)	
else if string.find(objecttoscavenge,"leather_pants") ~=nil then scavengehammer("leathercraft","leather",2,3,champ,slot)	
else if string.find(objecttoscavenge,"nomad_boots") ~=nil then scavengehammer("leathercraft","leather",1,1,champ,slot)	
else if string.find(objecttoscavenge,"nomad_mittens") ~=nil then scavengehammer("leathercraft","leather",1,1,champ,slot)	
else if string.find(objecttoscavenge,"sack") ~=nil then scavengehammer("leathercraft","leather",1,2,champ,slot)	
else if string.find(objecttoscavenge,"sling") ~=nil then scavengehammer("leathercraft","leather",1,1,champ,slot)	

else if string.find(objecttoscavenge,"round_shield") ~=nil then scavengehammer("armorsmith","iron_chunks",4,6,champ,slot)	
else if string.find(objecttoscavenge,"heavy_shield") ~=nil then scavengehammer("armorsmith","iron_chunks",6,8,champ,slot)	
else if string.find(objecttoscavenge,"plate_boots") ~=nil then scavengehammer("armorsmith","iron_chunks",4,6,champ,slot)	
else if string.find(objecttoscavenge,"plate_greaves") ~=nil then scavengehammer("armorsmith","iron_chunks",5,7,champ,slot)	
else if string.find(objecttoscavenge,"plate_cuirass") ~=nil then scavengehammer("armorsmith","iron_chunks",6,8,champ,slot)	
else if string.find(objecttoscavenge,"plate_gauntlets") ~=nil then scavengehammer("armorsmith","iron_chunks",4,6,champ,slot)	
else if string.find(objecttoscavenge,"ring_mail") ~=nil then scavengehammer("armorsmith","iron_chunks",5,7,champ,slot)	
else if string.find(objecttoscavenge,"ring_boots") ~=nil then scavengehammer("armorsmith","iron_chunks",4,6,champ,slot)	
else if string.find(objecttoscavenge,"ring_gauntlets") ~=nil then scavengehammer("armorsmith","iron_chunks",3,5,champ,slot)	
else if string.find(objecttoscavenge,"ring_greaves") ~=nil then scavengehammer("armorsmith","iron_chunks",4,6,champ,slot)	
else if string.find(objecttoscavenge,"full_helmet") ~=nil then scavengehammer("armorsmith","iron_chunks",5,7,champ,slot)	
else if string.find(objecttoscavenge,"iron_basinet") ~=nil then scavengehammer("armorsmith","iron_chunks",5,7,champ,slot)	

else if string.find(objecttoscavenge,"peasant_breeches") ~=nil then scavengehammer("tailor","silk",1,1,champ,slot)
else if string.find(objecttoscavenge,"peasant_tunic") ~=nil then scavengehammer("tailor","silk",1,2,champ,slot)
else if string.find(objecttoscavenge,"peasant_cap") ~=nil then scavengehammer("tailor","silk",1,1,champ,slot)
else if string.find(objecttoscavenge,"loincloth") ~=nil then scavengehammer("tailor","silk",1,1,champ,slot)
else if string.find(objecttoscavenge,"pointy_shoes") ~=nil then scavengehammer("tailor","silk",1,1,champ,slot)
else if string.find(objecttoscavenge,"silk_sac") ~=nil then scavengehammer("tailor","silk",2,3,champ,slot)
else if string.find(objecttoscavenge,"silk_hose") ~=nil then scavengehammer("tailor","silk",1,1,champ,slot)
else if string.find(objecttoscavenge,"cloak") ~=nil then scavengehammer("tailor","silk",1,2,champ,slot)
else if string.find(objecttoscavenge,"bandages") ~=nil then scavengehammer("tailor","silk",1,2,champ,slot)
else 
hudPrint("You cannot scavenge this item.")
end
end end end end end  end end end end  end end end end  end end end end  end end end end  end end end end 
 end end end end  end end end end  end end end end  end end end end  end end end end  end end end end 
 end end end end  end end end
end end
Is there any way to make it "nicer" or more efficient ? I have many codes like this, I would reproduce a possible optimization, it would be very helpfull. Thanks in advance :-)
A trip of a thousand leagues starts with a step.
Top
User avatar
JohnWordsworth
Posts: 1397
Joined: Fri Sep 14, 2012 4:19 pm
Location: Devon, United Kingdom
Contact:
Contact JohnWordsworth

Re: Looking for code optimizers, please

Post by JohnWordsworth »

If I were building a system like this, I would create a global table of options and then iterate over that table of options every time you wanted to check for a possible scavenge. For example, the following script works in a simple Lua script entity (although, I don't know if it would play friendly with save games, but I expect it would).
SpoilerShow

Code: Select all

function setupScavengeOptions()
	scavengeOptions = {
		{input="dagger", skill="weaponsmith", output="iron_chunks", min=3, max=5},
		{input="cutlass", skill="weaponsmith", output="iron_chunks", min=5, max=7},
	}
end

function scavengeItem(itemname, champ, slot)
	local objectToScavenge = itemname;

	for i,option in pairs(scavengeOptions) do
		if ( objectToScavenge == option.input ) then
			-- Comment out this line to replace it with your real code
			print("Scavenging item " .. option.input .. " for " .. option.output);
			-- scavengehammer(option.skill, option.output, option.min, option.max, champ, slot)
			return;	-- Jump out of the function early, as we have found a result
		end
	end

	hudPrint("You cannot scavenge this item.");
end

setupScavengeOptions();
scavengeItem("dagger");
scavengeItem("cutlass");
In this example, the setupScavengeOptions sets a global table of options that all of your future calls to scavengeItem will use to check what various inputs/outputs you have for scavenging. You could probably just put that code straight into the script_entity, but I like to wrap everything in functions. You will need to ensure that you call it in the script entity, as 'setupScavengeOptions' needs to be called before your first 'scavengeItem' call.

This is essentially setting up a table of data like...

Code: Select all

input		skill				output		min		max
dagger	weaponsmith		iron_chunks	3		5
cutlass	weaponsmith		iron_chunks	5		7
Then in scavenge item, you can just iterate over the options you've already setup in the earlier call to setupScavengeOptions to see if any of the available options matches your input. If it does, then you use that row to do what you need to do.

Not necessarily the best way, but I think it's pretty tidy - as you end up with a data table you can iterate over. You could theoretically 'optimise' it by having some sort of search tree, but I think this is probably fast enough if it's only being called now and again (and by that, I mean, not every single frame).
My Grimrock Projects Page with links to the Grimrock Model Toolkit, GrimFBX, Atlas Toolkit, QuickBar, NoteBook and the Oriental Weapons Pack.
Top
naim
Posts: 16
Joined: Mon Mar 26, 2012 11:40 am

Re: Looking for code optimizers, please

Post by naim »

I have seen worse. ;)

Here are some tips, especially for this code:
If you have switch-case like if statements, use the elseif construct instead of the regular else if to avoid hundreds of ends.
Try to use a proper intendation to make your code readable.
Follow some naming conventions for variable and function names. For instance: Functions start always with a capital letter; Variables start with a lower case letter; Every new word in a name starts with a capital letter;
String comparison methods are usually expensive; i did not look very deep into the LoG API, but maybe instead of comparing the string names, you can compare an item ID instead.

Code: Select all

function ScavengeHammerCheck(mouseItem,champ,slot)
	local objectToScavenge = mouseItem.name
	
	if string.find(objectToScavenge,"dagger") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)
	elseif string.find(objectToScavenge,"cutlass") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",5,7,champ,slot)
	elseif string.find(objectToScavenge,"fist_dagger") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)
	elseif string.find(objectToScavenge,"flail") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",3,6,champ,slot)
	elseif string.find(objectToScavenge,"great_axe") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",5,7,champ,slot)
	elseif string.find(objectToScavenge,"hand_axe") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)
	elseif string.find(objectToScavenge,"knife") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)
	elseif string.find(objectToScavenge,"long_sword") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",5,7,champ,slot)
	elseif string.find(objectToScavenge,"machete") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",4,6,champ,slot)
	elseif string.find(objectToScavenge,"shuriken") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)   
	elseif string.find(objectToScavenge,"throwing_axe") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)   
	elseif string.find(objectToScavenge,"throwing_knife") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",3,5,champ,slot)   
	elseif string.find(objectToScavenge,"warhammer") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",5,7,champ,slot)   
	elseif string.find(objectToScavenge,"battle_axe") ~=nil then 
		scavengehammer("weaponsmith","iron_chunks",5,7,champ,slot)
	elseif string.find(objectToScavenge,"arrow") ~=nil then 
		scavengehammer("woodworker","wood_branch",1,1,champ,slot)   
	elseif string.find(objectToScavenge,"crossbow") ~=nil then 
		scavengehammer("woodworker","wood_branch",1,2,champ,slot)   
	elseif string.find(objectToScavenge,"knoffer") ~=nil then 
		scavengehammer("woodworker","wood_branch",1,2,champ,slot)   
	elseif string.find(objectToScavenge,"short_bow") ~=nil then 
		scavengehammer("woodworker","wood_branch",1,2,champ,slot)   
	elseif string.find(objectToScavenge,"longbow") ~=nil then 
		scavengehammer("woodworker","wood_branch",1,2,champ,slot)   
	elseif string.find(objectToScavenge,"quarrel") ~=nil then 
		scavengehammer("woodworker","wood_branch",1,1,champ,slot)   
	elseif string.find(objectToScavenge,"wooden_box") ~=nil then 
		scavengehammer("woodworker","wood_branch",2,4,champ,slot)   
	elseif string.find(objectToScavenge,"camp_component") ~=nil then 
		scavengehammer("woodworker","wood_branch",1,1,champ,slot)   
	elseif string.find(objectToScavenge,"torch") ~=nil then 
		scavengehammer("woodworker","wood_branch",1,1,champ,slot)
	elseif string.find(objectToScavenge,"legionary_spear") ~=nil then 
		scavengehammer("woodworker","wood_branch",1,3,champ,slot)
	elseif string.find(objectToScavenge,"legionary_shield") ~=nil then 
		scavengehammer("armorsmith","iron_chunks",2,4,champ,slot)   
	elseif string.find(objectToScavenge,"hide_vest") ~=nil then 
		scavengehammer("leathercraft","leather",2,4,champ,slot)   
	elseif string.find(objectToScavenge,"huntsman_cloak") ~=nil then 
		scavengehammer("leathercraft","leather",1,1,champ,slot)   
	elseif string.find(objectToScavenge,"leather_boots") ~=nil then 
		scavengehammer("leathercraft","leather",1,1,champ,slot)   
	elseif string.find(objectToScavenge,"leather_brigandine") ~=nil then 
		scavengehammer("leathercraft","leather",1,2,champ,slot)   
	elseif string.find(objectToScavenge,"leather_cap") ~=nil then 
		scavengehammer("leathercraft","leather",1,1,champ,slot)   
	elseif string.find(objectToScavenge,"leather_gloves") ~=nil then 
		scavengehammer("leathercraft","leather",1,1,champ,slot)   
	elseif string.find(objectToScavenge,"leather_greaves") ~=nil then 
		scavengehammer("leathercraft","leather",1,1,champ,slot)   
	elseif string.find(objectToScavenge,"leather_pants") ~=nil then 
		scavengehammer("leathercraft","leather",2,3,champ,slot)   
	elseif string.find(objectToScavenge,"nomad_boots") ~=nil then 
		scavengehammer("leathercraft","leather",1,1,champ,slot)   
	elseif string.find(objectToScavenge,"nomad_mittens") ~=nil then 
		scavengehammer("leathercraft","leather",1,1,champ,slot)   
	elseif string.find(objectToScavenge,"sack") ~=nil then 
		scavengehammer("leathercraft","leather",1,2,champ,slot)   
	elseif string.find(objectToScavenge,"sling") ~=nil then 
		scavengehammer("leathercraft","leather",1,1,champ,slot)   
	elseif string.find(objectToScavenge,"round_shield") ~=nil then
		scavengehammer("armorsmith","iron_chunks",4,6,champ,slot)   
	elseif string.find(objectToScavenge,"heavy_shield") ~=nil then
		scavengehammer("armorsmith","iron_chunks",6,8,champ,slot)   
	elseif string.find(objectToScavenge,"plate_boots") ~=nil then
		scavengehammer("armorsmith","iron_chunks",4,6,champ,slot)   
	elseif string.find(objectToScavenge,"plate_greaves") ~=nil then
		scavengehammer("armorsmith","iron_chunks",5,7,champ,slot)   
	elseif string.find(objectToScavenge,"plate_cuirass") ~=nil then
		scavengehammer("armorsmith","iron_chunks",6,8,champ,slot)   
	elseif string.find(objectToScavenge,"plate_gauntlets") ~=nil then
		scavengehammer("armorsmith","iron_chunks",4,6,champ,slot)   
	elseif string.find(objectToScavenge,"ring_mail") ~=nil then
		scavengehammer("armorsmith","iron_chunks",5,7,champ,slot)   
	elseif string.find(objectToScavenge,"ring_boots") ~=nil then
		scavengehammer("armorsmith","iron_chunks",4,6,champ,slot)   
	elseif string.find(objectToScavenge,"ring_gauntlets") ~=nil then
		scavengehammer("armorsmith","iron_chunks",3,5,champ,slot)   
	elseif string.find(objectToScavenge,"ring_greaves") ~=nil then
		scavengehammer("armorsmith","iron_chunks",4,6,champ,slot)   
	elseif string.find(objectToScavenge,"full_helmet") ~=nil then
		scavengehammer("armorsmith","iron_chunks",5,7,champ,slot)   
	elseif string.find(objectToScavenge,"iron_basinet") ~=nil then
		scavengehammer("armorsmith","iron_chunks",5,7,champ,slot)   
	elseif string.find(objectToScavenge,"peasant_breeches") ~=nil then
		scavengehammer("tailor","silk",1,1,champ,slot)
	elseif string.find(objectToScavenge,"peasant_tunic") ~=nil then
		scavengehammer("tailor","silk",1,2,champ,slot)
	elseif string.find(objectToScavenge,"peasant_cap") ~=nil then
		scavengehammer("tailor","silk",1,1,champ,slot)
	elseif string.find(objectToScavenge,"loincloth") ~=nil then
		scavengehammer("tailor","silk",1,1,champ,slot)
	elseif string.find(objectToScavenge,"pointy_shoes") ~=nil then
		scavengehammer("tailor","silk",1,1,champ,slot)
	elseif string.find(objectToScavenge,"silk_sac") ~=nil then
		scavengehammer("tailor","silk",2,3,champ,slot)
	elseif string.find(objectToScavenge,"silk_hose") ~=nil then
		scavengehammer("tailor","silk",1,1,champ,slot)
	elseif string.find(objectToScavenge,"cloak") ~=nil then
		scavengehammer("tailor","silk",1,2,champ,slot)
	elseif string.find(objectToScavenge,"bandages") ~=nil then
		scavengehammer("tailor","silk",1,2,champ,slot)
	else 
		hudPrint("You cannot scavenge this item.")
	end
end
Top
User avatar
cromcrom
Posts: 549
Joined: Tue Sep 11, 2012 7:16 am
Location: Chateauroux in a socialist s#!$*&% formerly known as "France"

Re: Looking for code optimizers, please

Post by cromcrom »

Thank you so much guys, you have both been very helpfull, I love the "elseif" tip, just removing the bunch of "ends" is cool ^^

And thanks again JohnWordsworth, its a great lesson you gave me.

Take care guys :-)
A trip of a thousand leagues starts with a step.
Top
Post Reply

Return to “Modding”