Are there any possible memory leaks?

Hi, soo recently i decided to take look into OOP, and i made debounce module, i needed it because usually my frameworks require many players to have debounce on different tools and stuff

local Debounce = {}

Debounce.__index = Debounce

local AllDebounces = {}
-------------------------------------------------------------
-------------------------------------------------------------

--[[Create new debounce for invidual player]]
function Debounce.new(player:Player,name:string)
	local newDebounce = {}
	setmetatable(newDebounce,Debounce)
	
	AllDebounces[player] = newDebounce
	
	newDebounce.Player = player
	newDebounce.DebounceList = {}
	
	return newDebounce
end
--[[Adds or overwrite debounce type of invidual player]]
function Debounce:Add(name:string,timeout:number)
	self.DebounceList[name] = tick() + timeout
end
--[[Remove debounce from invidual player]]
function Debounce:Remove(name:string) : "removes specific debounce from player"
	self.DebounceList[name] = nil
end
--[[Remove all debounces from invidual player]] 
--[[it's reccomended to use it when player leaves to prevent memory leaks]]
function Debounce:RemoveAll(isPlayerLeaving:boolean)  : "removes all player's debounces"
	local DebounceList = self.DebounceList
	for i, debounce in DebounceList  do
		DebounceList[i] = nil
	end
	
	if isPlayerLeaving then
		self.DebounceList = nil
		AllDebounces[self.Player] = nil
		self.Player = nil
		
		setmetatable(self,nil)
	end
	
end
--[[Get acces to all player debounces]]
function Debounce:getPlayerDebounce(player:Player)
	return AllDebounces[player]
end

return Debounce

I made this module but i’m afraid that i made some stupid mistake and after using it 100+ times game’s performance would be terrible, soo i ask you if there are any memory-leaks or broken garbage collection

local Debounce = {
--[[
For some reason this is a debounce object
--]]
--[[ NOTES
- After some looking over of this I don't see any direct memory leeks
What I would suggest though is that you remove the need for these to be 
use as you need and just let them be stored whereever you are using them. Then just 
add a kill call and and clean them up when your done.  That way you can just reuse them 
in the context of use. Changing this around a bit to show waht i mean

Now if you want some sort of way you can group a set of them so you could kill
all at the same time, if it was me.  I would just write a utility here that lets you get maybe 
a special table or something that lets you place them inside it for the context area you are using it
that maybe can clear them all out in one go.
-------
It's not 100% but this is a bit more generic and you can let your area hold it and once you release
all references it will get cleaned up by collections

If I was going to write this though, I would instead use a function pass in as a predicate true false condition 
that way you could have a more diverse use of the debouce to be wahtever the function you wanted. 
You could build some built in accessors and or things like time checks and such but have a way 
to do debounces based on other things.   But thats just me.
-------

The point here is, in this specific case of what your doing less is more
you could make a much simplier object with a good breadth of features to make your life easier 
but there really isnt a reason to memory manage it here.  In fact, it would be more prone to leak here
then it would be just to store it relative to where you use it, and ensure that area clears up. 
Once your object loses all refrences and since we are not storing any references to instances here directly 
it should auto clean itself up
--]]
}

Debounce.__index = Debounce
-------------------------------------------------------------
-------------------------------------------------------------

--[[
You have a name pass in right now that you are not using ? 
Is this intentional or do you have other uses. 
]]
--[[Create new debounce for invidual player]]
function Debounce.new()
	
	local newDebounce = {}
	setmetatable(newDebounce,Debounce)

	newDebounce.Debounce = nil
	newDebounce.isPass = true
	newDebounce.setNextPass = false
	newDebounce.timeout = 0
		
	return newDebounce
end

--[[
When ever we see if we can get through the debounce we
check if the state of debounce should change state before returning value
]]
function Debounce:CanPass()
	
	if self.setNextPass then
		self:Set()
	end
	
	-- once we trip the debounce we g
	if self.Debounce >= tick() then 
		self.isPass = true		
		self.setNextPass = true
	end
	
	return self.isPass
end

--[[Adds or overwrite debounce type of invidual player]]
function Debounce:Set(timeout:number)
	--Predicate that if we are already active we dont need to set again
	if self.isPass == false then return end
	self.isPass = false
		
	self.timeout = timeout or self.timeout
	
	self.Debounce = tick() + self.timeout
end

--[[]]
function Debounce:ForcePass()
	self.Debounce = tick()
end

--[[]]
function Debounce:UpdateTimeout(timeout:number)
	self.timeout = timeout
end

--[[]]
function Debounce:ResetTimer()
	self.Debounce = tick() + self.timeout
end



return Debounce

just some suggestions otherwise seems ok, but as I said here, it is a touch spicy that you reference the player here which is a direct instance that could get suspended here if you don’t clean it up properly. In your old case you probably should build in a self kill function which will clean up your kill all which is just iterating over a table of the objects and calling kill.

I do think there is a touch of overloading in the current one and grouping them together as well as being singular is a bit “node” esk and I don’t see the immedant benefit but if it helps you that’s all that’s important.

2 Likes

I have removeAll function that cleans everything when player leaves, which also removes player instance

Also, i use this to handle many types of debounce for one player, soo for instance:

  • Player used tool
  • Player swapped and used weapon

Again, I just was showing what I would do, it still works but technically you could be more agnostic with it as a whole and let whatever area context they are being used manage the reference storage for them and release it that way.

Centralizing is fine, but this seems a bit more like a utility object vs other ways. Again if it was me I would pair it with a Top level Utility Module/Manager, that would not be the object but perhaps the thing that can manage the grouping feature ect thats seperate from the object.

Just trying to think of the logical use and seperation of logic when I can but again I am not saying it’s bad and if its useful and works for you then YAY!!! I am just suggesting a few things that may make it more broadly usable and less necessary to finagle to cram into use elsewhere.

1 Like

yhmm, soo you tell to make this code less. centralized. what does it mean?

2 things →

Think how often you use debounce and how you use it.

In this way this is great you are making your life easier.

But what if you wanted to use it in a place with no player reference, the behavior would almost be identical but you don’t have a player to attach it too.

Now you have to write a new one or create some finalge condition for it.

So by making the object more pure to what it does, you can give it a bit more features, maybe even consider pooling them once you release it so you could reuse a data object as you wanted since its more generic.

But minus pooling, which would have need to store ones that have been cleaned out.

I would suggest letting them be managed by where they are being used.

At best, create a manager that is not an object, that is how one is gotten. And then has function utilities for grouping or latching to a player, and then cleaning up the player grouped ones.

This way the debounce object stays agnostic,and the feature pairing with a player and grouped managed aspect comes from the application of the object.

1 Like

Also by keeping it more rooted in its agnostic behavior, you could later think of ways of exstending it and making different types of debounces from the parent debounce.

Which where the REAL power of OOP comes from.

1 Like

Oh, i know what you wanted to say, yea i thought about it, but this system was only some play with OOP because i never used itin meta-tables way, i usually used it with micro-module when i had 10 modules that make some effects or function and 1 controller which fired functions and ect.

But for the first time it’s OK

I know that separating manager from object is idea and i will probably explore it in later projects

Anyways, thx for explaining some OOP techniques, i’ll use them

All good, also I could not praise “MiddleClass” which you can get on github more.

It is literally the perfect class module ever made end of line. I use it with everything.

1 Like

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.