[Klei Publishing] U.V.S. Nirmana - Out Now!

OnEntitySleep/Wake doesn't account for entities being removed or becoming awake/asleep

March 2, 2025

Fixed

This can result in OnEntitySleep functions on insts and components to be called on an entity that no longer exists, and having to manually account for it is.. not ideal and kinda awkward..

In mainfunctions.lua:

function OnEntitySleep(guid)
    local inst = Ents[guid]
    if inst then
        inst:PushEvent("entitysleep")

        -- a listener might have removed us
        if not inst:IsValid() then
            return
        end

        if inst.OnEntitySleep then
            inst:OnEntitySleep()

            -- OnEntitySleep might have removed us
            if not inst:IsValid() then
                return
            end
        end

        inst:StopBrain()

        if inst.sg then
            SGManager:Hibernate(inst.sg)
        end

        if inst.emitter then
            EmitterManager:Hibernate(inst.emitter)
        end

        for k,v in pairs(inst.components) do

            if v.OnEntitySleep then
                v:OnEntitySleep()

                -- more optional here but could still happen if it's a component handling removal of an entity..?
                if not inst:IsValid() then
                    break
                end
            end
        end

    end
end

Checking for Ents[guid] could be a cheaper alternative to the valid checks, but I'm not sure if it's possible or how likely it is for a guid of an entity that was just removed to be given to a new one if said functions were to spawn some other entity. If it's possible at all, then doing it could be a bad idea. That said, if a simple boolean were to be added when calling inst:Remove(), you could check for that way more cheaply.

These issues also apply to OnEntityWake, even if it's more unlikely to cause a removal, the possibility is there.

As mentioned in the title, these also don't account for an entity going awake (or asleep) again due to an "entitysleep" listener function or OnEntitySleep function teleporting the unit back into wake/sleep range, but checking for not inst:IsAsleep() might end up being too much to be checking here whenever an entity goes through this (maybe use a boolean for this too? Unless the engine call for it isn't expensive, in which case it'll suffice).

This also applies to OnEntityWake, and the possibility of entities being able to go asleep mid-process.

I'm not sure if this is even an issue outside of W.O.B.O.T., but you could make a note to warn about it and to keep it in mind regardless of how this gets addressed, as it's pretty severe, it's lucky that it doesn't crash or cause more severe issues right now.

And since I mentioned W.O.B.O.T., let me point out how and why this is an issue, in storage_robot.lua:

local function OnEntitySleep(inst)
    if inst:IsInLimbo() or inst.components.fueled:IsEmpty() or inst.sg:HasStateTag("drowning") or inst.sg:HasStateTag("falling") then
        return
    end

    inst.components.fueled:StopConsuming()
    inst.SoundEmitter:KillAllSounds()

    inst.Physics:Teleport(StorageRobotCommon.GetSpawnPoint(inst):Get())

    if inst.brain ~= nil then
        inst.brain:UnignoreItem()
    end

    -- First store the item we are holding.

    local item = inst.components.inventory:GetFirstItemInAnySlot() or inst.components.inventory:GetActiveItem() -- This is intentionally backwards to give the bigger stacks first.

    if item ~= nil then
        local container = StorageRobotCommon.FindContainerWithItem(inst, item)

        if container ~= nil then
            BufferedAction(inst, container, ACTIONS.STORE, item):Do()
            inst.components.inventory:CloseAllChestContainers()
        else
            inst.components.inventory:DropItem(item, true, true)
        end
    end

    -- Then start pickuping others.

    inst:StartOffscreenPickupTask(1.5)
end

First of all, the teleport should be delayed by one frame with a task, not be done immediately upon going asleep. This is even if things were addressed.

Second, UnignoreItem should be called prior to the teleport (I'm surprised that the brain doesn't have a unique OnStop function to call UnignoreItem, might be a good idea to do that too).

And third, due to attempting a teleport in the middle of the OnEntitySleep process, the entity can easily awake again, and sleep and awake code can mix and run in the same stack, which can result in not having a brain and having a hibernated stategraph despite being awake, because the awake code will run, and then the rest of the OnEntitySleep code in mainfunctions.lua will proceed (which is why checking for entities being valid or that they're still in their asleep/awake state is important).

Here's how I handled it myself to fix these issues, with some code comments for more context:

local function OnEntityWake(inst)
    if inst._sleeptask ~= nil then
        inst._sleeptask:Cancel()
        inst._sleeptask = nil
    end

    -- stop teleport task if went awake in that frame
    if inst._sleepteleporttask ~= nil then
        inst._sleepteleporttask:Cancel()
        inst._sleepteleporttask = nil
    end
end

-- do a teleport delayed by 1 tick to avoid mixing sleep and wake code and other game-breaking issues
local function DoSleepTeleport(inst)
    inst._sleepteleporttask = nil

    -- check for limbo or drowning/falling state here for safety
    -- do we want to also check for fuel and the broken state? or do we commit to the teleport if we scheduled the task while having fuel?
    if inst:IsInLimbo() or inst.sg:HasAnyStateTag("drowning", "falling") then
        return
    end

    inst.Physics:Teleport(StorageRobotCommon.GetSpawnPoint(inst):Get())

    -- check we didn't become awake due to teleport
    if not inst:IsAsleep() then
        return
    end

    -- store or drop the item we are holding
    local item = inst.components.inventory:GetFirstItemInAnySlot() or inst.components.inventory:GetActiveItem()

    if item ~= nil then
        local container = StorageRobotCommon.FindContainerWithItem(inst, item)

        if container ~= nil then
            BufferedAction(inst, container, ACTIONS.STORE, item):Do()
            inst.components.inventory:CloseAllChestContainers()
        else
            inst.components.inventory:DropItem(item, true, true)
        end
    end

    -- start picking up other items
    inst:StartOffscreenPickupTask(1.5)
end

local function OnEntitySleep(inst)
    if inst:IsInLimbo() or inst.components.fueled:IsEmpty() or inst.sg:HasAnyStateTag("drowning", "falling") then
        return
    end

    inst.components.fueled:StopConsuming()
    inst.SoundEmitter:KillAllSounds()

    -- brain will get removed due to entity sleep right after this function, so do this prior to attempting to teleport
    if inst.brain ~= nil then
        inst.brain:UnignoreItem()
    end

    -- might be redundant given the fueled:IsEmpty check
    if not inst.sg:HasStateTag("broken") then
        -- do it after 1 tick, otherwise we mix sleep and wake code, which is bad (stuff can break, like wobot getting lobotomized)
        inst._sleepteleporttask = inst:DoTaskInTime(0, DoSleepTeleport)
    end
end

But once again, I think it's important to address these in mainfunctions.lua and then tweak things as necessary. Teleporting an entity on sleep or wake should probably always be delayed one frame even if all this gets addressed.

Steps to Reproduce

Deploy a repaired wobot, with a chest within range.
Drop or spawn a lot of the same item right on the edge of wobot's range. This will cause wobot to stay there and take a while to collect all the items.
Move in the opposite direction of where wobot moved to, until wobot is considered offscreen, then stop and head back. Use c_select() or so to keep track of wobot more easily, stop moving when the brain is removed then head back.
Wobot should now be awake, but non-functional (due to no brain) and idling in place until picked up or considered offscreen again.

Followers 1

Go to bugs

User Feedback

1 Comment

A developer has marked this issue as fixed. This means that the issue has been addressed in the current development build and will likely be in the next update.

Sign In

OnEntitySleep/Wake doesn't account for entities being removed or becoming awake/asleep

User Feedback

V2C 16618

Share this comment

Link to comment

Share on other sites

Create an account or sign in to comment

Create an account

Sign in

Browse

Forums

Activity

My Activity Streams