Can I even optimize this?

Nvm characjiajw idjqdo nmqwdon qwodn qwiod nqwiodni qwd qwd wqd q

1 Like

yeah i dont think theres a fix, just if there are too much spawning in a second then spawn less but give more

dont put the while loop inside the event but put it outside and change it to heartbeat or other run service stuff

I rewrote your code and added a gazillion comments to try to explain my thought process here. This won’t be useful to you unless you take the time to read it and understand the differences to your code base. It’s meant to be more readable, reliable, and probably more performant, but I obviously can’t realistically test it without the rest of your codebase/game setup.

I tried to keep the same logic and commented where it intentionally deviates.

--!strict

local CollectionService = game:GetService("CollectionService")
local RunService = game:GetService("RunService")
local Workspace = game:GetService("Workspace")
local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")

-- This assumes you apply a matching tag to your generator template model.
local GENERATOR_TAG = "Generator"

-- This assumes this value doesn't change at run time. If it does, you'll need
-- to update a variable when it changes.
local TICKS_PER_TIX_GEN = ReplicatedStorage.Values.TicksPerTixGen.Value
local TICKS_PER_SECOND = 60

local itemsFolder = Workspace.Items

type GeneratorInfo = {
	player: Player,
	isTixGenerator: boolean,
	humanoid: Humanoid,
	genSpeed: number,
	ticks: number?,
	ticksPerTixGen: number?,
	attachedNote: string,
}

-- Since different types of value objects don't all inherit from a single base
-- class, we have this custom function to check if the class name ends with "Value"
local function isValueObject(instance: Instance)
	return instance.ClassName:sub(-5) == "Value"
end

local function updateTableEntryOnValueChanged(dataTable: { [string]: any }, key: string, valueObject: Instance)
	assert(isValueObject(valueObject), `Instance is not a valid value object: {valueObject.ClassName}`)

	-- This gets disconnected when the value object is destroyed
	valueObject:GetPropertyChangedSignal("Value"):Connect(function()
		-- Casting to any here (and below) because Luau can't figure out that we
		-- already checked that it's a value object
		dataTable[key] = (valueObject :: any).Value
	end)

	return (valueObject :: any).Value
end

-- We'll maintain a list of generators to update and cache their info in a dictionary
-- in memory. Alternatively, we could loop over all generators with
-- CollectionService:GetTagged(GENERATOR_TAG), but this may be slow to call
-- every frame and does not allow us to cache generator properties.
local generatorInfoByInstance: { [Instance]: GeneratorInfo } = {}

-- This function guarantees all conditions are met that are required to update a
-- generator, such as properties existing and player data being loaded. Once all
-- conditions are met, it adds the generator to the list of generators to be
-- updated in the main loop.
local function onGeneratorAdded(generatorInstance: Instance)
	-- Avoid setting up generators that are outside of the items folder,
	-- e.g. sitting in ReplicatedStorage
	while not generatorInstance:IsDescendantOf(itemsFolder) do
		generatorInstance.AncestryChanged:Wait()
	end

	-- Generally branching behavior based on type of generator like this is not
	-- ideal. The separate behavior should be pulled out into a different
	-- function to keep things as clear as possible.
	local isTixGenerator = generatorInstance.Name == "TixGenerator"

	-- Wait for all the info objects to load. If using Streaming, you could
	-- set the Generator model to Atomic behavior to avoid having to use
	-- WaitForChild here.
	local humanoid = generatorInstance:WaitForChild("Humanoid")
	local attachedNote = generatorInstance:WaitForChild("AttachedNote")
	local info = generatorInstance:WaitForChild("Info")
	local playerName = (generatorInstance:WaitForChild("Player") :: StringValue).Value

	if isTixGenerator then
		info:WaitForChild("Ticks")
		info:WaitForChild("TicksPerTixGen")
	end

	assert(#playerName > 0, `Generator {generatorInstance} has no associated player name.`)

	local player = Players:GetPlayer(playerName)
	if not player then
		-- The player left at some point during this async loading sequence
		return
	end

	-- Wait for the player's data to load
	if not MainModule.DataLoaded(player) then
		player:WaitForChild("Data")
		player.Data:WaitForChild("GenSpeed")
	end

	-- It's faster to cache these object values in memory because reading so many instance properties is
	-- slow to do every frame. This is a 1-way sync from value object to memory.
	-- If reading these properties outside of this script is important, then
	-- this won't work for you. I'd recommend moving away from value objects and
	-- toward a fully in-memory solution where a value can be fetched from a
	-- single source if that's the case.
	local generatorInfo = {
		player = player,
		isTixGenerator = isTixGenerator,
	}

	for key, valueObject in
		{
			humanoid = humanoid,
			genSpeed = player.Data.GenSpeed,
			ticks = if isTixGenerator then info.Ticks else nil,
			ticksPerTixGen = if isTixGenerator then info.TicksPerTixGen else nil,
			attachedNote = attachedNote,
		}
	do
		generatorInfo[key] = valueObject.Value
		updateTableEntryOnValueChanged(generatorInfo, key, valueObject)
	end

	generatorInfoByInstance[generatorInstance] = generatorInfo :: GeneratorInfo
end

-- In case this script runs after generators already exist, we call
-- onGeneratorAdded for all existing generators
for _, generatorInstance in CollectionService:GetTagged(GENERATOR_TAG) do
	onGeneratorAdded(generatorInstance)
end

-- Listen for any new generators to get added
CollectionService:GetInstanceAddedSignal(GENERATOR_TAG):Connect(onGeneratorAdded)

-- When a generator is removed, remove it from the list of generators to update
CollectionService:GetInstanceRemovedSignal(GENERATOR_TAG):Connect(function(generatorInstance: Instance)
	generatorInfoByInstance[generatorInstance] = nil
end)

-- A single update loop to iterate over all generators being tracked and update
-- them based on their cached data.
RunService.Heartbeat:Connect(function(deltaTime: number)
	for generatorInstance, generatorInfo in generatorInfoByInstance do
		if generatorInfo.humanoid.Health <= 0 then
			-- Skip generating if the player is dead (this seems weird but I'm
			-- carrying the logic over from your code)
			continue
		end

		if generatorInfo.isTixGenerator then
			-- Using deltaTime allows for variable run rate, since Heartbeat isn't
			-- guaranteed to run exactly 60 times per second. Ticks added should be
			-- a function of actual time passed, like so.
			generatorInfo.ticks += deltaTime / TICKS_PER_SECOND / generatorInfo.genSpeed
			if generatorInfo.ticks >= TICKS_PER_TIX_GEN then
				-- Since the update rate may be slow, ticks could be >
				-- TICKS_PER_TIX_GEN and we don't want to reset it to 0, or it
				-- would lose that extra generation time that already passed
				generatorInfo.ticks -= TICKS_PER_TIX_GEN
				GameCore.SpawnTix(generatorInstance, generatorInfo.player)
			end
		else
			-- Regular generator
			GameCore.SpawnNote(generatorInstance, generatorInfo.player)
		end
	end
end)