Hello,
I’m working on getting used to using --!strict in my code, and I’m trying to figure out this:
function self:Destroy(): ()
self.Maid:DoCleaning()
self = nil
end
This is the Destroy method for my OOP class. It does work, but self = nil is flagged as a type error, because nil can’t be converted to the Entity class I casted, so what’s the ‘right’ way of going about destroying the object? The class is a dictionary with some values.
If you type-casted self to the Entity type somewhere in your code, just change that type to Entity?. (I think this should silence the type checker)
There’s a lot of flags with my constructor method because self could be nil, I’m going to try rewriting the constructor method and see if that sorts everything out.
Can I see your full script? I am confused as to whether you are writing a singleton or not.
function self:Destroy(): ()
self.Maid:DoCleaning()
self = nil :: any
end
This should resolve it, you can cast to any to silence. No need to re-construct your code.
Initially I was against this, I feel like casting any defeats the point of using strict typing, but it isn’t like I’ll be using that value again so it doesn’t really matter. I’m going to wait a bit, then if there are no preferable solutions then I’ll mark this as the solution.
--!strict
--- Services & Modules
local HttpService = game:GetService("HttpService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Maid = require(ReplicatedStorage.Utility.Maid)
--- Types
export type Entity = {
EntityId: string,
Model: Model,
StateMachine: nil, -- Temporary
Maid: any,
Destroy: (self: Entity) -> ()
}
export type EntityData = {
Model: Model
}
export type EntityModule = {
Start: () -> (),
new: (entityId: string, location: Vector3, additionalData: any?) -> Entity?,
GetEntityById: (entityId: string) -> Entity?
}
--- Module
local module = {} :: EntityModule
--- Variables
local loaded: boolean = false
local cachedEntityData: {[string]: EntityData} = {}
local entities: {[string]: Entity} = {}
setmetatable(entities, {__mode = "kv"}) -- Table values are weak, so they are removed when the entity is destroyed.
--- Functions
function getEntityId(entityModel: Model): string
local id: string
repeat id = HttpService:GenerateGUID(false) until entities[id] == nil
return id
end
--- Module Functions
function module.Start(): ()
-- Load animal templates.
for _, template in script.EntityData:GetChildren() do
if not template:IsA("ModuleScript") then
continue
end
cachedEntityData[string.sub(template.Name, 2, #template.Name)] = require(template) :: EntityData
end
loaded = true
end
function module.new(entityId: string, location: Vector3, additionalData: any?): Entity?
if not (entityId and location) then
error(`{script.Name} | Missing parameters.`)
return
end
-- Make sure all entity data is loaded.
if not loaded then
repeat task.wait(.001) until loaded
end
-- Make sure entity ID is real.
local entityDataToConstruct = cachedEntityData[entityId]
if not entityDataToConstruct then
print(cachedEntityData)
error(`{script.Name} | Attempt to create new entity with unknown entity ID '{entityId}'.`)
return
end
-- Construct entity.
local self = {} :: Entity
self.Model = entityDataToConstruct.Model:Clone() :: Model
self.Model.Parent = workspace.Entities
local humrp = self.Model:WaitForChild("HumanoidRootPart") :: BasePart
humrp.Position = location
self.StateMachine = nil -- Temporary.
-- Give entity id.
self.EntityId = getEntityId(self.Model)
self.Model:SetAttribute("EntityId", self.EntityId)
entities[self.EntityId] = self
-- Construct maid.
self.Maid = Maid.new()
self.Maid:GiveTask(self.Model)
-- self.Maid:GiveTask(self.StateMachine) -- Temporary as well.
function self:Destroy(): ()
self.Maid:DoCleaning()
self = nil :: any
end
return self
end
function module.GetEntityById(entityId: string): Entity?
return entities[entityId]
end
---
return module
there is no reason to use self = nil there unless it’s an upvalue of some sort
Ahh Okay, I get it now.
So the fix I suggested earlier would still silence the type checker but I am not sure if it would clean up the object.
You are basically inserting a unique Destroy function into your object. Not sure if this is what you want, unless the object is a singleton, you are probably wasting memory by inserting the same repeated function into each object.
Another thing, I do not see the point of doing self = nil in this function, at all. You are accessing the local parameter self of the Destroy function not the external local variable. Setting either to nil however, won’t guarantee that the table representing your object is collected by the garbage collector unless all references to the table are set to nil.
So if you are accessing one of these objects, outside the script, for example. Calling :Destroy() won’t be enough, you also have to set that reference to nil, if my knowledge of lua is correct.
Forgot to mention, the references inside the module.new() function, unless there’s a closure I missed, will be cleaned anyways after the constructor returns. So don’t worry about setting those references to nil. Find where this return value goes, that is, where this constructor is actually being called, and make that reference nil after calling :Destroy() on it.
Okay, sorry for the wait. I’ve created this mock-up for a new module as a response to your reply.
--!strict
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Maid = require(ReplicatedStorage.Utility.Maid)
---
type EntityModule = {
new: () -> Entity?
}
type Entity = {
Model: Model,
Maid: Maid.Maid<T>, -- Typing error here but I'll ignore it for now.
Destroy: (Entity) -> ()
}
---
function Destroy(self: Entity): ()
-- Make the external reference nil after calling this for Luau GC.
self.Maid:DoCleaning()
return
end
---
local Entity = {} :: EntityModule
function Entity.new(): Entity?
local self = {} :: Entity
self.Model = script.EntityData._Pig.EntityModel:Clone()
self.Model.Parent = workspace.Entities
local humrp = self.Model:WaitForChild("HumanoidRootPart") :: BasePart
humrp.Position = Vector3.new(0, 10, 0)
self.Maid = Maid.new()
self.Maid:GiveTask(self.Model)
self.Destroy = Destroy
return self
end
return Entity
I think this makes every object use the same destroy method, if you could verify that, that’d be great.
And yes, after doing some testing, you’re right about setting external values to nil after calling the destroy method, thank you for that information.
Thank you very much for the help.
This would make a global function Destroy (I know you are doing self.Destroy = Destroy but you’ll still be wasting a bit of memory storing that reference inside each object) you’ll need to put it in the Entity module, like so:
function Entity.Destroy(self: Entity): () ... end
(Just make that change to function header)
Now outside, if you want to do entity:Destroy(). You’ll need to do some stuff with metatables and __index that will make your types pretty complicated. (unless you are using the new type solver)
An easier way would be to just do Entity.Destroy(entity), and I am pretty sure updating your types for this won’t be so hard.
Other than that, everything looks good to me!
This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.