Crafting with tables

VegetationBush · December 7, 2019, 9:24pm

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?

wevetments · December 8, 2019, 6:45am

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.

VegetationBush · December 8, 2019, 1:27pm

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?)

wevetments · December 8, 2019, 7:27pm

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

VegetationBush · December 9, 2019, 1:35am

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

wevetments · December 9, 2019, 12:33pm

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

VegetationBush · December 9, 2019, 12:57pm

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

wevetments · December 9, 2019, 1:13pm

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

VegetationBush · December 9, 2019, 1:18pm

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.

wevetments · December 9, 2019, 1:23pm

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

VegetationBush · December 9, 2019, 1:24pm

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

wevetments · December 9, 2019, 1:27pm

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.

VegetationBush · December 9, 2019, 1:28pm

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

wevetments · December 9, 2019, 1:31pm

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

colbert2677 · December 9, 2019, 4:46pm

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:

VegetationBush · December 10, 2019, 2:51am

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?

wevetments · December 10, 2019, 11:59am

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

VegetationBush · December 10, 2019, 12:57pm

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

wevetments · December 10, 2019, 3:15pm

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.