TheTermos / mobkit

Entity API for Minetest
MIT License
26 stars 12 forks source link

add nil check to fix crash #27

Closed fluxionary closed 3 years ago

fluxionary commented 4 years ago

Caveats: I'm not entirely sure why :get_rotation() is sometimes returning nil, but I'm not the only one to get a crash because it sometimes is. I'm suspecting it has to do w/ something w/ minetest 5.3.0+. I don't know enough about this mod to say if skipping the call to set_rotation() won't have any important consequences, but my intuition says it won't.

Example crash stacktrace:

2020-09-01 04:34:03: ERROR[Main]: ServerError: AsyncErr: ServerThread::run Lua: Runtime error from mod 'default' in callback luaentity_Step(): .../flux/.minetest/worlds/ta_like/worldmods/mobkit/init.lua:1151: attempt to index local 'rot' (a nil value)
2020-09-01 04:34:03: ERROR[Main]: stack traceback:
2020-09-01 04:34:03: ERROR[Main]:       .../flux/.minetest/worlds/ta_like/worldmods/mobkit/init.lua:1151: in function 'func'
2020-09-01 04:34:03: ERROR[Main]:       .../flux/.minetest/worlds/ta_like/worldmods/mobkit/init.lua:741: in function 'execute_queues'
2020-09-01 04:34:03: ERROR[Main]:       .../flux/.minetest/worlds/ta_like/worldmods/mobkit/init.lua:959: in function 'stepfunc'
2020-09-01 04:34:03: ERROR[Main]:       ...worlds/ta_like/worldmods/paleotest/mobs/velociraptor.lua:271: in function 'func'
2020-09-01 04:34:03: ERROR[Main]:       /usr/share/minetest/builtin/profiler/instrumentation.lua:106: in function </usr/share/minetest/builtin/profiler/instrumentation.lua:100>
TheTermos commented 4 years ago

This probably has to do with the entity being removed while the behavior is still running. how are you calling lq_fallover, just regular from hq_die or some custom thing?

Anyway, the mobkit way would be to terminate the behavior if the entity is no longer around like this: if not mobkit.exists(self) then return true end

fluxionary commented 4 years ago

@ElCeejo can you comment on TheTermos's question? I haven't dug through the paleotest code much yet.

ElCeejo commented 4 years ago

I'm using a custom death function but it has multiple failsafes to avoid the entity being removed while any behaviors may be running. the area where hq_fallover is called is the same has the mobkit default death function.

TheTermos commented 4 years ago

Anyway, there's no point keeping a behavior running if the entity is no longer around, so can you please change it like below and report back whether the problem is gone?

function mobkit.lq_fallover(self)
    local zrot = 0
    local init = true
    local func=function(self)
        if not mobkit.exists(self) then return true end -- here
        if init then
        -- and so on ...
fluxionary commented 4 years ago

(not forgetting this; just been busy)

ElCeejo commented 4 years ago

It's been a while so I assumed that your suggested solution of adding if not mobkit.exists(self) then return true end worked, but the crash happened again so it appears that the issue has to do with something other than the entity being prematurely removed.

TheTermos commented 4 years ago

Can you provide a trace and maybe describe a way to reproduce this?

If get_rotation() returns nil while an entity is active it's probably the engine's fault. I want to avoid putting nil checks everywhere or the code is going to get really ugly.

ghost commented 3 years ago
ElCeejo commented 3 years ago

Both of those were already done, I forgot to post an update here. The issue has been fixed already and you can feel free to close this now.

TheTermos commented 3 years ago

Ok. I'll be moving example behaviors to a separate file, they're meant to be modified anyways.