Shrinking a function and mindset advice

Godot Version

4.7.2

Question

So i created a function for the shop system to reroll the items and it works. I know that when starting out, if it works leave it but I keep telling my sell you can make it better, they’re ways of making it better. That alone make’s it hard for me to anything I’m interested in doing not just coding. Is there a why to shake this mindset?

Also the code if you want to see it:

class_name StoreInventory extends Resource


@export var store_items: Array[SlotData] = []

@export var loot_table: ItemDropTable

var rng: RandomNumberGenerator

var previous_shops: Dictionary[int, Array] = {}
var store_index: int

func reroll_store() -> void:
	var temporary_array: Array[SlotData]
	temporary_array.resize(10)

	for index: int in range(temporary_array.size()):
		temporary_array[index] = SlotData.new()

	for index: int in temporary_array.size():
		var item: ItemData = LootGenerator.generate_loot(loot_table)

		if not index == 0 and temporary_array.any(
			func(slot: SlotData) -> bool: 
				if slot == null or slot.item_data == null:
					return false
				return slot.item_data.name == item.name):
			continue

		temporary_array[index].item_data = item

		if temporary_array[index].item_data == null:
			continue

		if temporary_array[index].item_data is not  EquippableItemData:
			temporary_array[index].quantity = Globals.rng.randi_range(1, 5)
			continue

		temporary_array[index].quantity = 1

	temporary_array = temporary_array.filter(func (slot: SlotData) -> bool:
		return slot.item_data != null)

	store_index += 1
	previous_shops = {store_index: store_items.duplicate(true)}

	store_items.clear()

	temporary_array.sort_custom(
		func(slot_A: SlotData, slot_B: SlotData) -> bool: 
			if slot_A.item_data == null or slot_B.item_data == null:
				return false
			
			return slot_A.item_data.cost > slot_B.item_data.cost)

	store_items.append_array(temporary_array)

It could be tidied up a bit, but otherwise looks fine.

and for the mindset difficulties?

Do a game jam.

so for clean up move some chunks into there own functions or what?

No, just eliminate annoying repetition like temporary_array[index].item_data.

there’s another way of doing that line?

You hold the same reference in item. Much more readable to just use that instead.

so like doing the item checks before adding it to the array besides the middle one

It doesn’t matter when you do it. Once you do the assignment, item and temporary_array[index].item_data refer to the same object.

is this what you were thinking of pertaining to clean up or is it same problem different skin?

	for slot: SlotData in temporary_array:
		var item: ItemData = LootGenerator.generate_loot(loot_table)

		if temporary_array.any(
			func(slots: SlotData) -> bool: 
				if slots == null or slots.item_data == null:
					return false
				return slots.item_data.name == item.name):
			continue
		
		
		slot.item_data = item

		if slot.item_data == null:
			continue

		if item is not EquippableItemData:
			slot.quantity = Globals.rng.randi_range(1, 5)
		else:
			slot.quantity = 1

How about something like this ^^

class_name StoreInventory
extends Resource

@export var store_items: Array[SlotData] = []

@export var loot_table: ItemDropTable

var previous_shops: Dictionary[int, Array] = {}
var number_of_rerolls: int = 0


func reroll_store() -> void:
	var temporary_array: Array[SlotData] = []

	# Generate new shop contents. Discard invalid and duplicate loot.
	for i in 10:
		var item: ItemData = LootGenerator.generate_loot(loot_table)
		if item == null:
			continue

		# Prevent duplicates
		if temporary_array.any(
			func(slot: SlotData) -> bool: 
				return slot.item_data.name == item.name
		):
			continue

		if item is not EquippableItemData:
			item.quantity = Globals.rng.randi_range(1, 5)
		else:
			item.quantity = 1

		var new_slot_data := SlotData.new()
		new_slot_data.item_data = item
		temporary_array.append(new_slot_data)

	# Sort the new shop contents by cost
	temporary_array.sort_custom(
		func(slot_a: SlotData, slot_b: SlotData) -> bool: 
			return slot_a.item_data.cost > slot_b.item_data.cost)

	# Update trackers
	previous_shops[number_of_rerolls] = store_items.duplicate(true)
	number_of_rerolls += 1

	# Update the shop's contents
	store_items.clear()
	store_items.append_array(temporary_array)

the issue is with item equals null check can’t roll a null value unless i purposely add an empty slot with a weighting and the item doesn’t hold the quantity value the slot data does, but everything else looks cleaner than mine.

example:

Edit: With some reordering it works and looks better too, thanks Sam from left field :+1:.

func reroll_store() -> void:
	var temporary_array: Array[SlotData]

	for index: int in 10:
		var item: ItemData = LootGenerator.generate_loot(loot_table)

		if temporary_array.any(
			func(slot: SlotData) -> bool: 
				return slot.item_data.name == item.name):
			continue

		var new_slot: SlotData = SlotData.new()
		new_slot.item_data = item

		if item is not EquippableItemData:
			new_slot.quantity = Globals.rng.randi_range(1, 5)
		else:
			new_slot.quantity = 1

		temporary_array.append(new_slot)

	rerolls += 1
	previous_shops = {rerolls: store_items.duplicate(true)}
	store_items.clear()

	temporary_array.sort_custom(
		func(slot_A: SlotData, slot_B: SlotData) -> bool: 
			return slot_A.item_data.cost > slot_B.item_data.cost)

	store_items.append_array(temporary_array)

I’m not good with time constraints, I would more than likely be panicking over what to make than actually making it. I’m not disregarding the idea just confidence is not high for it.

The whole point is to get out of the comfort zone and challenge yourself. It’s good for building confidence.

I thought that, too, but signed up for one anyway. OK, the game I made was not good, but I did it in the time allotted and it helped me more than I would have thought. I recommend the experience. BC_Thum