Jump to content

Entity sleep state pending isn't accounted for a few things


hoxi
  • Pending

I reported this a while back when the sleep state pending checks were added, but I looked into it more recently and I figured I should bring some attention to this again.

 

As a first example, despite it being accounted for when an entity's brain is set, it's not for when setting the stategraph:

function EntityScript:SetStateGraph(name)
    if self.sg ~= nil then
        SGManager:RemoveInstance(self.sg)
    end
    local sg = LoadStateGraph(name)
    assert(sg ~= nil)
    if sg ~= nil then
        self.sg = StateGraphInstance(sg, self)
		SGManager:AddInstance(self.sg, self:IsAsleep())
        self.sg:GoToState(self.sg.sg.defaultstate)
		if self:IsInLimbo() then
			self.sg:Stop()
		end
        return self.sg
    end
end

 

There's also many cases of components like amphibiouscreature, combat, sleeper, growable, etc, that do this as well, because they either check for IsAsleep (which currently doesn't account for pending sleep state), or don't.

Yet their Wake or Sleep functions will stop some of that behavior immediately when receiving a state from the engine anyway, so it's essentially a waste of performance, there's no gameplay impact if this is kept or fixed.

 

Cases like the sleeper component need to have a check like this added:

function Sleeper:StartTesting(time)
    -- normally missing
    if self.inst:IsAsleep() then
        return
    end
    if self.isasleep then
        self:SetTest(ShouldWakeUp, time)
    else
        self:SetTest(ShouldSleep)
    end
end

Because Sleeper:StartTesting is called whenever test functions for sleeping or waking up are set, which occurs during prefab construction a lot of the time (plus, on it's own, it's an oversight to not verify entity sleep state when setting a sleep/wake test function, but it would be better to make it all-encompassing as shown above, and maybe sleeper should also be checking for limbo as well in general).

(same goes for Growable:StartGrowing, and there's probably some other similar cases, although for growable it's only fine when offscreen asleep is enabled though)

 

I feel like ideally, and to prevent any future issues and keeping things optimized, rather than adding specific inst.sleepstatepending checks to certain things, the following should be changed instead:

function EntityScript:IsAsleep()
    -- if sleep state is pending, act as if we're asleep
    -- we don't want to execute or schedule things in this state, let inst.OnEntityWake or "entitywake" events handle it
    --return not self.entity:IsAwake()
    return self.sleepstatepending or not self.entity:IsAwake()
end

 

For this to work as expected, you'd have to slightly change this here:

function OnEntityWake(guid)
    local inst = Ents[guid]
    if inst then
        inst.sleepstatepending = nil -- set it here, before firing events and running any functions

        inst:PushEvent("entitywake")

        if inst.OnEntityWake then
            inst:OnEntityWake()
        end

        --V2C: Note that if this needs to work properly for networked
        --     entities on clients, then should use the slower check
        --     :HasTag("INLIMBO").  But there should be no networked
        --     entities on clients that can go to sleep.
        if not inst:IsInLimbo() then
			inst:_EnableBrain_Internal()
            if inst.sg then
                SGManager:Wake(inst.sg)
            end
        end

        --inst.sleepstatepending = nil

Same goes for OnEntitySleep.

 

This needs to be done because if you (or a modder) were to make a common handler function that checks for, say, limbo and entity sleep, and maybe even some other conditions, all in that function, and said function is triggered by the respective events related to those things, not setting that flag to nil before calling inst.OnEntitySleep or firing the event, will result in a false positive the first time "entitywake" or "entitysleep" events are fired.

 

(plus, it should be completely fine to do the change, as the only few sleepstatepending checks that are in the game wouldn't be affected)

 

EDIT: I almost forgot.. lastly, pretty much every single case of checking by using not inst.entity:IsAwake() instead of inst:IsAsleep() should probably be verified.. I feel like these should be all updated to call the EntityScript function instead, even more so if the change I suggested here goes through.

An example being:

function EmitterManagerClass:AddEmitter( inst, lifetime, updateFunc )
	inst.emitter = inst
	local statusTable = self.awakeEmitters
	if not inst.entity:IsAwake() then
		statusTable = self.sleepingEmitters
	end

 


Steps to Reproduce
  • Modify EntityScript:IsAsleep to print something whenever it's called while sleep state is still pending (prior to testing addressing this). Potentially use _TRACEBACK() to trace cases of it as well.
  • Notice how even when just launching a server, there will be many, many cases of this being done, as well as being followed by undoing the associated behavior/scheduling after finishing prefab construction and receiving a sleep state.
  • Like 1



User Feedback


There are no comments to display.



Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×
  • Create New...