#17 Fix protector crash with many nil checks to prevent indexing nil values

Closed
MisterE123 wants to merge 0 commits from deleted into MeseCraft/master

I saw the protector bug report...

It is caused here: https://notabug.org/MeseCraft/mesecraft/src/master/mods/protector/init.lua#L641

with

if v:get_luaentity().name == "protector:display" then

get_luaentity() cannot be called on all objects. Also, I am not sure that all luaentities contain the 'name' field. Most do for sure...

Anyhow the fix to to check for nil at every step.

In this PR, I add if ... then checks to first check that v:get_luaentity() isnt nil, and then that v:get_luaentity().name isnt nil. Then it is safe to check if it is "protector:display".

merging should repair the crash

I saw the protector bug report... It is caused here: https://notabug.org/MeseCraft/mesecraft/src/master/mods/protector/init.lua#L641 with if v:get_luaentity().name == "protector:display" then get_luaentity() cannot be called on all objects. Also, I am not sure that all luaentities contain the 'name' field. Most do for sure... Anyhow the fix to to check for nil at every step. In this PR, I add if ... then checks to first check that v:get_luaentity() isnt nil, and then that v:get_luaentity().name isnt nil. Then it is safe to check if it is "protector:display". merging should repair the crash
MeseCraft commented 1 year ago
Owner

This should be merged via the development branch, not master.

I'll try to add these manually.

This should be merged via the development branch, not master. I'll try to add these manually.
MeseCraft commented 1 year ago
Owner

Added. Not sure how to check this with nil objects? Can you think of a way to test it? I'm going to merge this into stable/master anyways since the patch fixes two critical bugs.

Added. Not sure how to check this with nil objects? Can you think of a way to test it? I'm going to merge this into stable/master anyways since the patch fixes two critical bugs.
MisterE123 commented 1 year ago
Poster

MeseCraft wrote: "Not sure how to check this with nil objects?"

I'm not sure what you want to check. The purpose of this portion of code is to remove the protector area display (that might have been created by punching the protector) when the node is dug. The protector display entity will always be a luaentity, and will always have the name "protector:display". This bit of code should always work its intended purpose. When it crashed before, it was trying to check if it should delete objects other than the "protector:display", and at least one of the checks it ran on the object returned nil, which crashed the server.

MeseCraft wrote: "Not sure how to check this with nil objects?" --------------------- I'm not sure what you want to check. The purpose of this portion of code is to remove the protector area display (that might have been created by punching the protector) when the node is dug. The protector display entity will always be a luaentity, and will always have the name "protector:display". This bit of code *should* always work its intended purpose. When it crashed before, it was trying to check if it should delete objects other than the "protector:display", and at least one of the checks it ran on the object returned nil, which crashed the server.
Please reopen this pull request to perform merge operation.
Sign in to join this conversation.
No Label
No Milestone
No assignee
2 Participants
Loading...
Cancel
Save
There is no content yet.