I have this hypothetical scenario here and I was wondering if there were any implications by setting up this way.
Here is a simple class I have set-up in a module script. I was wondering if this would cause any sort of problems/memory leaks when I try to destroy it.
Is it alright if I don’t disconnect this event because its “static”??
local break_car : BindableEvent = game.ReplicatedStorage.Events.break_car
local Car = {}
Car.__index = Car
Car.spawned_cars = {}
function Car.new(ID: string)
local self = setmetatable({}, Car)
self.brand = "Honda"
self.wheels = 4
self.gas = 100
Car.spawned_cars[ID] = self
end
function Car:refuel()
-- increase gas
self.gas += 15
end
function Car.destroy(ID:string)
local found_car = Car.spawned_cars[ID]
if found_car then
found_car.brand = nil
found_car.wheels = nil
found_car.gas = nil
Car.spawned_cars[ID] = nil
end
end
--... random stuff
break_car.Event:Connect(function(carID)
Car.destroy(carID)
end)
return Car
Looks good to me. I dont think you need to set number or string values to nil. The most important part would be Car.spawned_cars[ID] = nil, which you have in your destroy method.
Something else that I found extremely useful was the Trove library. If you were to have actual parts from workspace inside that class, you could use it to clean them up, so you don’t have to individually delete each part, or let alone remember to do so. It works with connections, promises, and all things of that nature.
function Car.new(ID: string)
--... old stuff
self.NEWTHING = game.GetService("RunService").Stepped:Connect(function()
print("yo!")
end)
Car.spawned_cars[ID] = self
end
In order to completely destroy the instance, i would need to disconnect self.NEWTHING right? I also looked into Trove and it looked useful for what I’m trying to achieve
Honestly you shouldn’t be using bindable events in your module. If it’s to avoid circular dependency I suggest refactoring. If it’s a small project then sure, I guess it’s easy to implement and not worry about.
In an ideal situation, your break car event should be removed and any code that wants to break a car should just call Car.destroy(). You could also turn Car.destroy into Car:destroy() and have ID put into self.ID = ID.
What use case do you have for running .Stepped for every car made? Sure this works with a few dozen cars if you’re running low calculating code inside the function, but ideally you could try just one .Stepped and iterate through all active cars. Like so:
game.GetService("RunService").Stepped:Connect(function() --also .Heartbeat is best unless you need to use .Stepped for something specific
for i,v in pairs(Car.spawned_cars) do
end
end)
With remote events you’re crossing a client to server boundary. With bindable events it’s two contexts talking to each other.
The point of modules is to put everything inside of a container. In your case, this container is everything relative to cars. You could replace all your methods (refuel, new, and future ones) into bindables and it could work. It just isn’t ideal because now you have code that can either require your module to do something, or fire a bindable event. Destroying the idea of a one-stop container.
I think an issue you have here is that you’re storing attributes regarding the car class itself and not the object. For example, you have Car.spawned_cars. If you come back to your code after a break, it’s most likely that you’ll use Car.new() and see many attributes like .spawned_cars that don’t even work for that object itself if you understand what I mean.
You need a distinction between general info of the class and the attributes that the object will possess.
I don’t know what you mean, can you explain what you’re thinking?
My intention with the spawned_cars variable is to keep track of all the car objects, with their related attributes, within some static list. From there, I can access some specific Car object from another script. I guess in the example scenario I showed above didn’t necessarily need the BindableEvent, as I can just straight up call the Car.destroy() function (I think)
My point is that you should list non-class data somewhere else. spawned_cars shouldn’t be located in the Car table. That should only hold information that will be obtained by each specific car object.
Honestly, I write code like I’m still using Java (which probably isn’t ideal in Roblox). If there are advantages of using local variables, I’ll switch over to it.
I’d also like to note that if I were to use something like this within my module
local spawned_cars = {}
then I would need to call some method that would allow me to access those objects (unless I there is another way?), instead of just doing
local Car_class = require(some_path_to_module)
local my_car = Car_class.spawned_cars[my_id]
Maybe store that in a different modulescript such as a dictionary. I don’t know if it is more preferrable to keep dictionaries read-only, but that’s something I think I would do. I recommend asking more people about this idea too.
So if I have a module that creates a car, I usually store the stuff I create outside of the module script.
In a local variable inside my server script I would have local cars = {}. It’s really up to you how you do it, some people don’t like their modules storing state.
I would just store the list of cars within the parent script running the Car.new function. the way it doesn’t even return self kind of defeats the purpose of a class, no?