Crafting with tables

So is this script efficient?:

local player = game.Players.LocalPlayer
local Stats = player:WaitForChild("Stats")
local Level = Stats:WaitForChild("Level")
local Item1= Stats:WaitForChild("Item1")
local Item2= Stats:WaitForChild("Item2")

local Craft= {Item1, 1, Item2, 1}
local Craft2= {Item1, 1, Item2, 100}

local Success = 0

Item1.Changed:Connect(function()
	for i = 1, #Craft, 2 do
		print(i)
		if Craft[i]:FindFirstAncestor("Stats") then
			if Craft[i].Value >= Craft[i+1] then
				Success = Success + 1
				if Success == #Craft/2 then
					Success = 0
				end
			end
		end
	end
    	for i = 1, #Craft2, 2 do
		print(i)
		if Craft2[i]:FindFirstAncestor("Stats") then
			if Craft2[i].Value >= Craft2[i+1] then
				Success = Success + 1
				if Success == #Craft2/2 then
					Success = 0
				end
			end
		end
	end
-- and so on and so fourth
end)

how can I improve this?

I wouldn’t advise using dozens of loops for each craft table. I’b probably create one function that does all the work for me like such:

local player = game.Players.LocalPlayer
local Stats = player:WaitForChild("Stats")
local Level = Stats:WaitForChild("Level")
local Item1= Stats:WaitForChild("Item1")
local Item2= Stats:WaitForChild("Item2")

local Craft= {Item1, 1, Item2, 1}
local Craft2= {Item1, 1, Item2, 100}

local function VerifyRequirements(CraftTable)
    local Success = 0
    for i, v in ipairs(CraftTable) do
        if v:IsA("IntValue") then
            if v.Value >= CraftTable[i + 1] then
                Success = Success + 1
                if Success == #CraftTable/2 and i == #CraftTable then
                    print("Requirements Complete")
                elseif Success ~= #CraftTable/2 and i== #CraftTable then
                    print("Requirements Incomplete")
                end
            end
        end
    end
end

VerifyRequirements(Craft)
VerifyRequirements(Craft2)

Note:
I just realised that the previous code samples I posted did not take into account when the crafting requirements were incomplete, hence breaking the code logic. Please use this code as a reference instead of the previous ones.

2 Likes

so this script would verify the requirements whenever the item values change? Because I just need to make sure(I don’t know, but is there supposed to be a .Changed in there?)

There doesn’t need to be a Changed event in the function as it will always get the updated values upon running it.

1 Like

no, like when the item values change I want it to fire again

Just setup an event connection

local Item1= Stats:WaitForChild("Item1")
local Item2= Stats:WaitForChild("Item2")

Item1.Changed:Connect(function() VerifyRequirements(Craft) end)
Item2.Changed:Connect(function() VerifyRequirements(Craft2) end)

However, I don’t think this is very efficient if you will be having a dozen more Items so I’d create a background loop by using spawn() or while loops or coroutine or Heartbeat

Ive never used spawn() coroutine or Heartbeat before, can you explain further? May you give me an example?

Spawn

spawn(function()
    while true do
        VerifyRequirements(Craft)
        VerifyRequirements(Craft2)
        wait(0.1)
    end
end)

Coroutine

local Coroutine = coroutine.create(function()
    while true do
        VerifyRequirements(Craft)
        VerifyRequirements(Craft2)
        wait(0.1)
    end
end)

coroutine.resume(Coroutine)

While Loops

while ThisConditionIsTrue do
    VerifyRequirements(Craft)
    VerifyRequirements(Craft2)
    wait(0.1)
end

Heartbeat

RunService.Heartbeat:Connect(function()
    VerifyRequirements(Craft)
    VerifyRequirements(Craft2)
end)

I personally prefer coroutines, but it comes down to what exactly you need for what you’re coding

1 Like

Can you elaborate on the things you showed be a bit? Everything except for the while loop. Like, what does coroutine and heartbeat do?, and what is spawn? Thank you so much.

Spawn will create a function on a separate thread other than its own. Note that creating a lot of these will have some performance impacts.

Coroutine is like creating a task that you can control. You can check if that task is running, what’s currently the status and yield the task. I also think this is better in terms of controlling loops.

Heartbeat is an event that fires after physics calculations per frame

1 Like

Isn’t looping them over and over again going to cause tremendous lag?

You aren’t actually looping through anything, you’re just using the function several times a second or less to get updated the crafting status. The function itself isn’t very heavy in terms of computation, so it shouldn’t really create an impact when called in loops.

1 Like

Which one do you think is the best for this type of situation?

I think the coroutine would be the best way to go.

1 Like

Spawn and coroutine are both used for pseudothreading and perform relatively the same function, but you should aim to use coroutines moreso than spawn.

Spawn runs on the legacy task scheduler and has a built-in wait. Wait, as you may or may not know, never actually waits the amount of time that you expect. There are two parts: waiting for the amount of time specified (or the minimum of ~0.03) as well as until there’s an available slot in the scheduler to resume. This is why wait returns a value which is the actual time wait spent waiting.

Always try to go for coroutines.

This discussion has been held a few times before. Most recent thread if you’re interested:

2 Likes

Is there a way to have .Changed like this(I want it so I don’t have to do .Changed for every item)?

local Items =  {Item1, Item2}

Items.Changed:Connect(function()
    coroutine.resume(Coroutine)
end)

So when an item value changes in the table, the Changed event fires. Is that the way to do it?

You don’t necessarily need to do this. Once the coroutine is resumed/started, the loop will keep on updating the values. However, if you’d still wish to have this kind of setup, you could create something like this:

local Items = {Item1, Item2}
local CraftData = {
    [1] = {Item1, 1, Item2, 1}
    [2] = {Item1, 1, Item2, 100}
}

local function VerifyRequirements(CraftTable)
    local Success = 0
    for i, v in ipairs(CraftTable) do
        if v:IsA("IntValue") then
            if v.Value >= CraftTable[i + 1] then
                Success = Success + 1
                if Success == #CraftTable/2 and i == #CraftTable then
                    print("Requirements Complete")
                elseif Success ~= #CraftTable/2 and i== #CraftTable then
                    print("Requirements Incomplete")
                end
            end
        end
    end
end

for i, v in ipairs(Items) do
    v.Changed:Connect(function()
        VerifyRequirements(CraftData[i])
    end
end
1 Like

I thought that I can use pairs, but then, doesn’t in ipars run only once? Thank you!

Pairs and ipairs doesn’t have much of a difference other than pairs supposedly being used with dictionaries and ipairs being used with numerically arranged tables (array was the word I was looking for). The only good thing about using ipairs is readability and actually using its intended function. You could always use pairs since it supports both dictionaries and tables with numbered indexes arrays.

Read more here:

pairs:

  • Designed for dictionaries
  • Calls next, which finds the next key based on a given key
  • Ordered iteration is not guaranteed
  • Optimised but slower than ipairs
  • All keys are accounted for
  • Does not stop at a nil value

ipairs:

  • Designed for arrays
  • Index lookup starts at 1 and increases every iteration
  • Ordered iteration is guaranteed
  • Quicker than pairs with known indices
  • Ignores non-numeric indices
  • Stops iterating at the first nil value for an indice

There’s a noticeable difference when using each depending on the circumstances. Use ipairs for arrays.

3 Likes