attempt to index local 'v' (a nil value) #4258

New Issue

AliasAlreadyTaken · 2023-04-25T08:36:55Z

AliasAlreadyTaken commented

2023-04-25 08:36:55 +00:00

2023-04-25 08:34:13: ERROR[Main]: ServerError: AsyncErr: Lua: Runtime error from mod 'yl_events' in callback luaentity_run_simple_callback(): .../mt/5.6.1/Minetest_test/bin/../builtin/common/vector.lua:99: attempt to index local 'v' (a nil value)
2023-04-25 08:34:13: ERROR[Main]: stack traceback:
2023-04-25 08:34:13: ERROR[Main]: .../mt/5.6.1/Minetest_test/bin/../builtin/common/vector.lua:99: in function 'round'
2023-04-25 08:34:13: ERROR[Main]: ....1/Minetest_test/bin/../mods/yl_events/tribe_miocene.lua:89: in function 'func'
2023-04-25 08:34:13: ERROR[Main]: ...inetest_test/bin/../builtin/profiler/instrumentation.lua:107: in function <...inetest_test/bin/../builtin/profiler/instrumentation.lua:100>

2023-04-25 08:34:13: ERROR[Main]: ServerError: AsyncErr: Lua: Runtime error from mod 'yl_events' in callback luaentity_run_simple_callback(): .../mt/5.6.1/Minetest_test/bin/../builtin/common/vector.lua:99: attempt to index local 'v' (a nil value) 2023-04-25 08:34:13: ERROR[Main]: stack traceback: 2023-04-25 08:34:13: ERROR[Main]: .../mt/5.6.1/Minetest_test/bin/../builtin/common/vector.lua:99: in function 'round' 2023-04-25 08:34:13: ERROR[Main]: ....1/Minetest_test/bin/../mods/yl_events/tribe_miocene.lua:89: in function 'func' 2023-04-25 08:34:13: ERROR[Main]: ...inetest_test/bin/../builtin/profiler/instrumentation.lua:107: in function <...inetest_test/bin/../builtin/profiler/instrumentation.lua:100>

flux commented

2023-04-25 16:07:50 +00:00

looks like someone right-clicked on a miocene that died before the server got the input; a nil check is needed.

flux added the

1. kind/bug

2. prio/critical

labels 2023-04-25 16:08:05 +00:00

AliasAlreadyTaken commented

2023-04-26 05:54:56 +00:00

I fixed like so:

local pos = self.object:get_pos() or vector.new(0,0,0)

IMO vector.round and all the other vector functions should return nil, if they can#t compute and defend better against wrong input.

yl_commons or better vector itself needs a is_legit_pos(pos) function that checks the paramter whether its a position and whether its legit (= inside the world, depending on mapsize).

I fixed like so: ``` local pos = self.object:get_pos() or vector.new(0,0,0) ``` IMO vector.round and all the other vector functions should return nil, if they can#t compute and defend better against wrong input. yl_commons or better vector itself needs a `is_legit_pos(pos)` function that checks the paramter whether its a position and whether its legit (= inside the world, depending on mapsize).

AliasAlreadyTaken added the

4. step/ready to QA test

label 2023-04-26 13:30:52 +00:00

AliasAlreadyTaken added this to the 1.1.119 milestone 2023-04-26 13:30:55 +00:00

AliasAlreadyTaken commented

2023-05-09 09:21:29 +00:00

QA:

Is my dumbfix any good? I can't properly reproduce it anyways.

Since I implemented it, could anyone look at the code or test and greenlight by 👍 ?

QA: Is my dumbfix any good? I can't properly reproduce it anyways. Since I implemented it, could anyone look at the code or test and greenlight by 👍 ?

AliasAlreadyTaken added the

ugh/QA main

label 2023-05-09 09:21:34 +00:00

flux commented

2023-05-10 03:50:47 +00:00

local pos = self.object:get_pos() or vector.new(0,0,0)

i'm not a huge fan of defaulting to vector.zero() - it means that something unexpected is going to happen at 0,0,0. note that we've already had at least one issue that resulted in something weird being placed at that position (un-diggable, un-owned bones, i don't remember if i ever figured out why, but see #2796).

better would be to not do any further processing if self.object is not active, i.e. when :get_pos() (or most other methods) returns nil, which means we've got an object which has either been :remove()'ed or unloaded.

> `local pos = self.object:get_pos() or vector.new(0,0,0)` i'm not a huge fan of defaulting to `vector.zero()` - it means that something unexpected is going to happen at 0,0,0. note that we've already had at least one issue that resulted in something weird being placed at that position (un-diggable, un-owned bones, i don't remember if i ever figured out why, but see #2796). better would be to not do any further processing if `self.object` is not active, i.e. when `:get_pos()` (or most other methods) returns `nil`, which means we've got an object which has either been `:remove()`'ed or unloaded.

flux commented

2023-05-10 03:52:10 +00:00

oh apparently i did figure out #2796, but still, really really esoteric reasoning

AliasAlreadyTaken commented

2023-07-02 15:05:23 +00:00

Should we fix this in a different way than by defaulting to 0? If so, how?

AliasAlreadyTaken added the

4. step/question

label 2023-07-02 15:05:29 +00:00

whosit commented

2023-07-02 15:15:46 +00:00

Would this work?

diff --git a/tribe_miocene.lua b/tribe_miocene.lua
index 300c5c6..fec835f 100644
--- a/tribe_miocene.lua
+++ b/tribe_miocene.lua
@@ -83,7 +83,13 @@ end
 
 local on_right_click = function(self, clicker)
 
-    local pos = self.object:get_pos() or vector.new(0,0,0)
+    local pos = self.object:get_pos()
+    if not pos then
+        core.log("error",
+                 "[QUEST] Tribe Miocene id = " .. self._id .. " was right clicked but has no pos")
+        return
+    end
+
     local item = clicker:get_wielded_item()
     core.log("action",
         "[QUEST] Tribe Miocene rightclicked at " .. core.pos_to_string(vector.round(pos)) .. " id = " .. self._id)

In theory returning early would prevent mio from being picked up or healed, but in practice that should never happen, right?
Or, if no pos means it's inactive, then picking it up or healing would be the wrong thing to do?

Would this work? ```diff diff --git a/tribe_miocene.lua b/tribe_miocene.lua index 300c5c6..fec835f 100644 --- a/tribe_miocene.lua +++ b/tribe_miocene.lua @@ -83,7 +83,13 @@ end local on_right_click = function(self, clicker) - local pos = self.object:get_pos() or vector.new(0,0,0) + local pos = self.object:get_pos() + if not pos then + core.log("error", + "[QUEST] Tribe Miocene id = " .. self._id .. " was right clicked but has no pos") + return + end + local item = clicker:get_wielded_item() core.log("action", "[QUEST] Tribe Miocene rightclicked at " .. core.pos_to_string(vector.round(pos)) .. " id = " .. self._id) ``` In theory returning early would prevent mio from being picked up or healed, but in practice that should never happen, right? Or, if no `pos` means it's inactive, then picking it up or healing would be the wrong thing to do?

👍 1

flux commented

2023-07-04 01:42:10 +00:00

that looks right