To many if statments I need help

Help
,
1

Godot 4.2

I really want to know how to optimize my code I know it working is the thing that matters but this is awful to type and it looks atrocious and I have no clue how else to do it

it’s an inventory script if you could not tell

extends Control

@export var Sl1 = 0
@export var Sl2 = 0
@export var Sl3 = 0
@export var Sl4 = 0
@export var Sl5 = 0
@export var Sl6 = 0
@export var Sl7 = 0
@export var Sl8 = 0
@export var Sl9 = 0

@export var Slot1item = "nothing"
@export var Slot2item = "nothing"
@export var Slot3item = "nothing"
@export var Slot4item = "nothing"
@export var Slot5item = "nothing"
@export var Slot6item = "nothing"
@export var Slot7item = "nothing"
@export var Slot8item = "nothing"
@export var Slot9item = "nothing"

@onready var StoneTex = preload("res://Sprites/stone.png")
@onready var CopperTex = preload("res://Sprites/Copper.png")
@onready var IronTex = preload("res://Sprites/iron.png")
@onready var CoalTex = preload("res://Sprites/coal.png")
@onready var SilverTex = preload("res://Sprites/silverore.png")
@onready var GoldTex = preload("res://Sprites/gold .png")
@onready var SulferTex = preload("res://Sprites/Sulfurclump.png")
@onready var JadeTex = preload("res://Sprites/Jade.png")
@onready var BismuthTex = preload("res://Sprites/Bismite.png")
@onready var RubyTex = preload("res://Sprites/ruby.png")

@onready var IronBarTex = preload("res://Sprites/iron ingot.png")





func Item_Type(atlas_coord):
	
	var stone = atlas_coord.y < 5
	var copper = atlas_coord.y < 8 and atlas_coord.y > 4
	var iron = atlas_coord.y < 11 and atlas_coord.y > 7
	var coal = atlas_coord.y < 14 and atlas_coord.y > 10
	var silver = atlas_coord.y < 17 and atlas_coord.y > 13
	var gold = atlas_coord.y < 20 and atlas_coord.y > 16
	var sulfer = atlas_coord.y < 23 and atlas_coord.y > 19
	var jade = atlas_coord.y < 25 and atlas_coord.y > 22
	var bismuth = atlas_coord.y < 27 and atlas_coord.y > 24
	var ruby = atlas_coord.y < 29 and atlas_coord.y > 26
	
	
	var item = "nothing"
	
	if stone:
		item = "Stone"
		Item_Give(item)
	if copper:
		item = "Copper"
		Item_Give(item)
	if iron:
		item = "Iron"
		Item_Give(item)
	if coal:
		item = "Coal"
		Item_Give(item)
	if silver:
		item = "Silver"
		Item_Give(item)
	if gold:
		item = "Gold"
		Item_Give(item)
	if sulfer:
		item = "Sulfer"
		Item_Give(item)
	if jade:
		item = "Jade"
		Item_Give(item)
	if bismuth:
		item = "Bismuth"
		Item_Give(item)
	if ruby:
		item = "Ruby"
		Item_Give(item)
func Item_Give(item):
	
	var Slot1Filled = $R1SL1/Label.visible
	var Slot2Filled = $R1SL2/Label.visible
	var Slot3Filled = $R1SL3/Label.visible
	var Slot4Filled = $R1SL4/Label.visible
	var Slot5Filled = $R1SL5/Label.visible
	var Slot6Filled = $R1SL6/Label.visible
	var Slot7Filled = $R1SL7/Label.visible
	var Slot8Filled = $R1SL8/Label.visible
	var Slot9Filled = $R1SL9/Label.visible
	
	if Slot1Filled == false:
		$R1SL1/Label.visible = true
		Sl1 = Sl1 + 1
		$R1SL1/Label.text = str(Sl1)
		if item == "Stone":
			$R1SL1.icon = StoneTex
			Slot1item = "Stone"
		if item == "Copper":
			$R1SL1.icon = CopperTex
			Slot1item = "Copper"
		if item == "Iron":
			$R1SL1.icon = IronTex
			Slot1item = "Iron"
		if item == "Coal":
			$R1SL1.icon = CoalTex
			Slot1item = "Coal"
		if item == "Silver":
			$R1SL1.icon = SilverTex
			Slot1item = "Silver"
		if item == "Gold":
			$R1SL1.icon = GoldTex
			Slot1item = "Gold"
		if item == "Sulfer":
			$R1SL1.icon = SulferTex
			Slot1item = "Sulfer"
		if item == "Jade":
			$R1SL1.icon = JadeTex
			Slot1item = "Jade"
		if item == "Bismuth":
			$R1SL1.icon = BismuthTex
			Slot1item = "Bismuth"
		if item == "Ruby":
			$R1SL1.icon = RubyTex
			Slot1item = "Ruby"
	elif Slot1item == item:
		Sl1 = Sl1 + 1
		$R1SL1/Label.text = str(Sl1)
	elif Slot2Filled == false:
		$R1SL2/Label.visible = true
		Sl2 = Sl2 + 1
		$R1SL2/Label.text = str(Sl2)
		if item == "Stone":
			$R1SL2.icon = StoneTex
			Slot2item = "Stone"
		if item == "Copper":
			$R1SL2.icon = CopperTex
			Slot2item = "Copper"
		if item == "Iron":
			$R1SL2.icon = IronTex
			Slot2item = "Iron"
		if item == "Coal":
			$R1SL2.icon = CoalTex
			Slot2item = "Coal"
		if item == "Silver":
			$R1SL2.icon = SilverTex
			Slot2item = "Silver"
		if item == "Gold":
			$R1SL2.icon = GoldTex
			Slot2item = "Gold"
		if item == "Sulfer":
			$R1SL2.icon = SulferTex
			Slot2item = "Sulfer"
		if item == "Jade":
			$R1SL2.icon = JadeTex
			Slot2item = "Jade"
		if item == "Bismuth":
			$R1SL2.icon = BismuthTex
			Slot2item = "Bismuth"
		if item == "Ruby":
			$R1SL2.icon = RubyTex
			Slot2item = "Ruby"
	elif Slot2item == item:
		Sl2 = Sl2 + 1
		$R1SL2/Label.text = str(Sl2)
	elif Slot3Filled == false:
		$R1SL3/Label.visible = true
		Sl3 = Sl3 + 1
		$R1SL3/Label.text = str(Sl3)
		if item == "Stone":
			$R1SL3.icon = StoneTex
			Slot3item = "Stone"
		if item == "Copper":
			$R1SL3.icon = CopperTex
			Slot3item = "Copper"
		if item == "Iron":
			$R1SL3.icon = IronTex
			Slot3item = "Iron"
		if item == "Coal":
			$R1SL3.icon = CoalTex
			Slot3item = "Coal"
		if item == "Silver":
			$R1SL3.icon = SilverTex
			Slot3item = "Silver"
		if item == "Gold":
			$R1SL3.icon = GoldTex
			Slot3item = "Gold"
		if item == "Sulfer":
			$R1SL3.icon = SulferTex
			Slot3item = "Sulfer"
		if item == "Jade":
			$R1SL3.icon = JadeTex
			Slot3item = "Jade"
		if item == "Bismuth":
			$R1SL3.icon = BismuthTex
			Slot3item = "Bismuth"
		if item == "Ruby":
			$R1SL3.icon = RubyTex
			Slot3item = "Ruby"
	elif Slot3item == item:
		Sl3 = Sl3 + 1
		$R1SL3/Label.text = str(Sl3)
	elif Slot4Filled == false:
		$R1SL4/Label.visible = true
		Sl4 = Sl4 + 1
		$R1SL4/Label.text = str(Sl4)
		if item == "Stone":
			$R1SL4.icon = StoneTex
			Slot4item = "Stone"
		if item == "Copper":
			$R1SL4.icon = CopperTex
			Slot4item = "Copper"
		if item == "Iron":
			$R1SL4.icon = IronTex
			Slot4item = "Iron"
		if item == "Coal":
			$R1SL4.icon = CoalTex
			Slot4item = "Coal"
		if item == "Silver":
			$R1SL4.icon = SilverTex
			Slot4item = "Silver"
		if item == "Gold":
			$R1SL4.icon = GoldTex
			Slot4item = "Gold"
		if item == "Sulfer":
			$R1SL4.icon = SulferTex
			Slot4item = "Sulfer"
		if item == "Jade":
			$R1SL4.icon = JadeTex
			Slot4item = "Jade"
		if item == "Bismuth":
			$R1SL4.icon = BismuthTex
			Slot4item = "Bismuth"
		if item == "Ruby":
			$R1SL4.icon = RubyTex
			Slot4item = "Ruby"
	elif Slot4item == item:
		Sl4 = Sl4 + 1
		$R1SL4/Label.text = str(Sl4)
	elif Slot5Filled == false:
		$R1SL5/Label.visible = true
		Sl5 = Sl5 + 1
		$R1SL5/Label.text = str(Sl5)
		if item == "Stone":
			$R1SL5.icon = StoneTex
			Slot5item = "Stone"
		if item == "Copper":
			$R1SL5.icon = CopperTex
			Slot5item = "Copper"
		if item == "Iron":
			$R1SL5.icon = IronTex
			Slot5item = "Iron"
		if item == "Coal":
			$R1SL5.icon = CoalTex
			Slot5item = "Coal"
		if item == "Silver":
			$R1SL5.icon = SilverTex
			Slot5item = "Silver"
		if item == "Gold":
			$R1SL5.icon = GoldTex
			Slot5item = "Gold"
		if item == "Sulfer":
			$R1SL5.icon = SulferTex
			Slot5item = "Sulfer"
		if item == "Jade":
			$R1SL5.icon = JadeTex
			Slot5item = "Jade"
		if item == "Bismuth":
			$R1SL5.icon = BismuthTex
			Slot5item = "Bismuth"
		if item == "Ruby":
			$R1SL5.icon = RubyTex
			Slot5item = "Ruby"
	elif Slot5item == item:
		Sl5 = Sl5 + 1
		$R1SL5/Label.text = str(Sl5)
	elif Slot6Filled == false:
		$R1SL6/Label.visible = true
		Sl6 = Sl6 + 1
		$R1SL6/Label.text = str(Sl6)
		if item == "Stone":
			$R1SL6.icon = StoneTex
			Slot6item = "Stone"
		if item == "Copper":
			$R1SL6.icon = CopperTex
			Slot6item = "Copper"
		if item == "Iron":
			$R1SL6.icon = IronTex
			Slot6item = "Iron"
		if item == "Coal":
			$R1SL6.icon = CoalTex
			Slot6item = "Coal"
		if item == "Silver":
			$R1SL6.icon = SilverTex
			Slot6item = "Silver"
		if item == "Gold":
			$R1SL6.icon = GoldTex
			Slot6item = "Gold"
		if item == "Sulfer":
			$R1SL6.icon = SulferTex
			Slot6item = "Sulfer"
		if item == "Jade":
			$R1SL6.icon = JadeTex
			Slot6item = "Jade"
		if item == "Bismuth":
			$R1SL6.icon = BismuthTex
			Slot6item = "Bismuth"
		if item == "Ruby":
			$R1SL6.icon = RubyTex
			Slot6item = "Ruby"
	elif Slot6item == item:
		Sl6 = Sl6 + 1
		$R1SL6/Label.text = str(Sl6)
	elif Slot7Filled == false:
		$R1SL7/Label.visible = true
		Sl7 = Sl7 + 1
		$R1SL7/Label.text = str(Sl7)
		if item == "Stone":
			$R1SL7.icon = StoneTex
			Slot7item = "Stone"
		if item == "Copper":
			$R1SL7.icon = CopperTex
			Slot7item = "Copper"
		if item == "Iron":
			$R1SL7.icon = IronTex
			Slot7item = "Iron"
		if item == "Coal":
			$R1SL7.icon = CoalTex
			Slot7item = "Coal"
		if item == "Silver":
			$R1SL7.icon = SilverTex
			Slot7item = "Silver"
		if item == "Gold":
			$R1SL7.icon = GoldTex
			Slot7item = "Gold"
		if item == "Sulfer":
			$R1SL7.icon = SulferTex
			Slot7item = "Sulfer"
		if item == "Jade":
			$R1SL7.icon = JadeTex
			Slot7item = "Jade"
		if item == "Bismuth":
			$R1SL7.icon = BismuthTex
			Slot7item = "Bismuth"
		if item == "Ruby":
			$R1SL7.icon = RubyTex
			Slot7item = "Ruby"
	elif Slot7item == item:
		Sl7 = Sl7 + 1
		$R1SL7/Label.text = str(Sl7)
	elif Slot8Filled == false:
		$R1SL8/Label.visible = true
		Sl8 = Sl8 + 1
		$R1SL8/Label.text = str(Sl8)
		if item == "Stone":
			$R1SL8.icon = StoneTex
			Slot8item = "Stone"
		if item == "Copper":
			$R1SL8.icon = CopperTex
			Slot8item = "Copper"
		if item == "Iron":
			$R1SL8.icon = IronTex
			Slot8item = "Iron"
		if item == "Coal":
			$R1SL8.icon = CoalTex
			Slot8item = "Coal"
		if item == "Silver":
			$R1SL8.icon = SilverTex
			Slot8item = "Silver"
		if item == "Gold":
			$R1SL8.icon = GoldTex
			Slot8item = "Gold"
		if item == "Sulfer":
			$R1SL8.icon = SulferTex
			Slot8item = "Sulfer"
		if item == "Jade":
			$R1SL8.icon = JadeTex
			Slot8item = "Jade"
		if item == "Bismuth":
			$R1SL8.icon = BismuthTex
			Slot8item = "Bismuth"
		if item == "Ruby":
			$R1SL8.icon = RubyTex
			Slot8item = "Ruby"
	elif Slot8item == item:
		Sl8 = Sl8 + 1
		$R1SL8/Label.text = str(Sl8)
	elif Slot9Filled == false:
		$R1SL9/Label.visible = true
		Sl9 = Sl9 + 1
		$R1SL9/Label.text = str(Sl9)
		if item == "Stone":
			$R1SL9.icon = StoneTex
			Slot9item = "Stone"
		if item == "Copper":
			$R1SL9.icon = CopperTex
			Slot9item = "Copper"
		if item == "Iron":
			$R1SL9.icon = IronTex
			Slot9item = "Iron"
		if item == "Coal":
			$R1SL9.icon = CoalTex
			Slot9item = "Coal"
		if item == "Silver":
			$R1SL9.icon = SilverTex
			Slot9item = "Silver"
		if item == "Gold":
			$R1SL9.icon = GoldTex
			Slot9item = "Gold"
		if item == "Sulfer":
			$R1SL9.icon = SulferTex
			Slot9item = "Sulfer"
		if item == "Jade":
			$R1SL9.icon = JadeTex
			Slot9item = "Jade"
		if item == "Bismuth":
			$R1SL9.icon = BismuthTex
			Slot9item = "Bismuth"
		if item == "Ruby":
			$R1SL9.icon = RubyTex
			Slot9item = "Ruby"
	elif Slot9item == item:
		Sl9 = Sl9 + 1
		$R1SL9/Label.text = str(Sl9)
	else:
		pass
2

first, you have related data: e.g. “coal” and CoalTex
you can organize your data into a smaller package

enum {
	STONE, COPPER, IRON, COAL,
	SILVER, GOLD, SULFER, JADE,
	BISMUTH, RUBY,
	MATERIAL_COUNT
}

var materials = {
	STONE:{"name":"stone", "icon": StoneTex},
	COPPER:{"name":"copper", "icon":CopperTex},
	IRON:{"name":"iron", "icon":IronTex},
	COAL:{"name":"coal", "icon":CoalTex},
	SILVER:{"name":"silver", "icon":SilverTex},
	GOLD:{"name":"gold", "icon":GoldTex},
	SULFER:{"name":"sulfer", "icon":SulferTex},
	JADE:{"name":"jade", "icon":JadeTex},
	BISMUTH:{"name":"bismuth", "icon":BismuthTex},
	RUBY:{"name":"ruby", "icon":RubyTex},
}

Then select the resources in item_type function.
although sometimes its not possible to get rid of all if statements. you can leverage if/elif to handle boundaries though


func Item_Type(atlas_coord):
	var type = MATERIAL_COUNT # use count for validity check
	if atlas_coord.y < 5:
		type = STONE
	elif atlas_coord.y < 8:
		type = COPPER
	elif atlas_coord.y < 11:
		type = IRON
	elif atlas_coord.y < 14:
		type = COAL
	elif atlas_coord.y < 17:
		type = SILVER
	elif atlas_coord.y < 20:
		type = GOLD
	elif atlas_coord.y < 23:
		type = SULFER
	elif atlas_coord.y < 25:
		type = JADE
	elif atlas_coord.y < 27:
		type = BISMUTH
	elif atlas_coord.y < 29:
		type = RUBY
	else:
		printerr("Atlas coordinate out of range of materials!")

	var item = {}
	if type < MAX_MATERIALS:
		item = materials[type]
	Item_Give(item)

then in your item_give function you can collapse the slot resource assignment like this. We saved on if statements by leveraging past logic and stored them into the item data.

func Item_Give(item):
	if item.is_empty():
		return
...
	if Slot1Filled == false:
		$R1SL1/Label.visible = true
		Sl1 = Sl1 + 1
		$R1SL1/Label.text = str(Sl1)
		$R1SL1.icon = item.icon
		Slot1item = item.name
	elif Slot1item == item.name:
		Sl1 = Sl1 + 1
		$R1SL1/Label.text = str(Sl1)
	elif Slot2Filled == false:
		$R1SL2/Label.visible = true
		Sl2 = Sl2 + 1
		$R1SL2/Label.text = str(Sl2)
		$R1SL2.icon = item.icon
		Slot2item = item.name
	elif Slot2item == item.name:
		Sl2 = Sl2 + 1
		$R1SL2/Label.text = str(Sl2)
	... # etc

You may be able to collapse your slots and labels but I’ll leave that a challenge for you.

3 Likes
3

There’s a lot going on here, but overall always consider structuring your data such that it minimises repetition, enables iteration and promotes re-usability. This should generally result in code that more easily scales and is easier to manage/read.

For example your Item_Type logic and dependent information could be compressed down to:

@onready var items = {
	"Stone": {"texture": preload("res://Sprites/stone.png"), "min": 0, "max": 4},
	"Copper": {"texture": preload("res://Sprites/Copper.png"), "min": 5, "max": 7},
	"Iron": {"texture": preload("res://Sprites/iron.png"), "min": 8, "max": 10},
	"Coal": {"texture": preload("res://Sprites/coal.png"), "min": 11, "max": 13},
	"Silver": {"texture": preload("res://Sprites/silverore.png"), "min": 14, "max": 16},
	"Gold": {"texture": preload("res://Sprites/gold.png"), "min": 17, "max": 19},
	"Sulfer": {"texture": preload("res://Sprites/Sulfurclump.png"), "min": 20, "max": 22},
	"Jade": {"texture": preload("res://Sprites/Jade.png"), "min": 23, "max": 25},
	"Bismuth": {"texture": preload("res://Sprites/Bismite.png"), "min": 26, "max": 28},
	"Ruby": {"texture": preload("res://Sprites/ruby.png"), "min": 29, "max": 31}
}


func Item_Type(atlas_coord) -> void:
	for name in items.keys():
		if atlas_coord.y >= items[name]["min"] and atlas_coord.y <= items[name]["max"]:
			Item_Give(name)

Here the many individual variables and separate checks have been refactored into a single data table (a Dictionary) containing the same information without repetition, and then a for loop that iterates over this table, running the atlas_coord lookup for each element in the Dictionary (~55 rows down to ~17).

The Item_Type function now just loops through all the ‘keys’ in the dictionary (for name in items.keys() which will give you [“Stone”,“Copper”,“Iron”…]) and checks if the y value of atlas_coord is in the min and max range of an item. When a match is found, it calls Item_Give with the name of the item.

This means if you ever need to add an item, you just add one new row to the items dictionary, instead of 4 new rows to the Item_Type function. Pulling your information into a separate dictionary also makes this re-usable to other functions if necessary.

The Item_Give function can be refactored in essentially the same way, creating a ‘slots’ dictionary where the key is just a number representing the slot number, then each row is contains the count, node reference, a reference to what item is associated, and maybe the label.

3 Likes
4

First Thank you this is so much better than what I was doing but I am having trouble understanding what MATERIAL_COUNT and MAX_MATERIAL are for?

5

I might have made a typo, i changed names a few times and missed refactoring.

The name i landed on was MATERIAL_COUNT, AS MAX_MATERIAL was a little vague and confusing.

But MATERIAL_COUNT is meant to be placed at the end of the enumeration and can be checked to make sure you are not passing a value out of range of range.

enum {
ROCK,  # 0
PAPER, # 1
SCISSOR, # 2
HAND_SIGN_COUNT # 3
}

func _ready()
   attack(463763355)

func attack(hand:int):
  If hand < HAND_SIGN_COUNT:
     return # invalid hand

  If hand == ROCK:
    ...
  ...

We can technically pass any number but we dont have code to handle it so we can gate keep.

But if we want to add a new attack and keep HAND_SIGN_COUNT as the last element in the enum:

enum {
ROCK,  # 0
PAPER, # 1
SCISSOR, # 2
FIRE, #3
ICE, #4
WIND, #5
HAND_SIGN_COUNT # 6
}

It automatically gets updated and we can add new features and still check if the attack is within range of defined behaviors.

Basically its defensive programming to prevent bugs