141 lines
4.1 KiB
Markdown
141 lines
4.1 KiB
Markdown
---
|
|
title: Common Mistakes
|
|
layout: default
|
|
root: ../..
|
|
idx: 8.1
|
|
redirect_from: /en/chapters/common_mistakes.html
|
|
---
|
|
|
|
## Introduction <!-- omit in toc -->
|
|
|
|
This chapter details common mistakes, and how to avoid them.
|
|
|
|
- [Be Careful When Storing ObjectRefs (ie: players or entities)](#be-careful-when-storing-objectrefs-ie-players-or-entities)
|
|
- [Don't Trust Formspec Submissions](#dont-trust-formspec-submissions)
|
|
- [Set ItemStacks After Changing Them](#set-itemstacks-after-changing-them)
|
|
|
|
## Be Careful When Storing ObjectRefs (ie: players or entities)
|
|
|
|
An ObjectRef is invalidated when the player or entity it represents leaves
|
|
the game. This may happen when the player goes offline, or the entity is unloaded
|
|
or removed.
|
|
|
|
The methods of ObjectRefs will always return nil when invalid, since Minetest 5.2.
|
|
Any call will essentially be ignored.
|
|
|
|
You should avoid storing ObjectRefs where possible. If you do to store an
|
|
ObjectRef, you should make sure you check it before use, like so:
|
|
|
|
```lua
|
|
-- This only works in Minetest 5.2+
|
|
if obj:get_pos() then
|
|
-- is valid!
|
|
end
|
|
```
|
|
|
|
## Don't Trust Formspec Submissions
|
|
|
|
Malicious clients can submit formspecs whenever they like, with
|
|
whatever content they like.
|
|
|
|
For example, the following code has a vulnerability which allows players to
|
|
give themselves moderator privileges:
|
|
|
|
```lua
|
|
local function show_formspec(name)
|
|
if not core.check_player_privs(name, { privs = true }) then
|
|
return false
|
|
end
|
|
|
|
core.show_formspec(name, "modman:modman", [[
|
|
size[3,2]
|
|
field[0,0;3,1;target;Name;]
|
|
button_exit[0,1;3,1;sub;Promote]
|
|
]])
|
|
return true
|
|
})
|
|
|
|
core.register_on_player_receive_fields(function(player,
|
|
formname, fields)
|
|
-- BAD! Missing privilege check here!
|
|
|
|
local privs = core.get_player_privs(fields.target)
|
|
privs.kick = true
|
|
privs.ban = true
|
|
core.set_player_privs(fields.target, privs)
|
|
return true
|
|
end)
|
|
```
|
|
|
|
Add a privilege check to solve this:
|
|
|
|
```lua
|
|
core.register_on_player_receive_fields(function(player,
|
|
formname, fields)
|
|
if not core.check_player_privs(name, { privs = true }) then
|
|
return false
|
|
end
|
|
|
|
-- code
|
|
end)
|
|
```
|
|
|
|
## Set ItemStacks After Changing Them
|
|
|
|
Have you noticed that it's simply called an `ItemStack` in the API, not an `ItemStackRef`,
|
|
similar to `InvRef`? This is because an `ItemStack` isn't a reference - it's a
|
|
copy. Stacks work on a copy of the data rather than the stack in the inventory.
|
|
This means that modifying a stack won't actually modify that stack in the inventory.
|
|
|
|
For example, don't do this:
|
|
|
|
```lua
|
|
local inv = player:get_inventory()
|
|
local stack = inv:get_stack("main", 1)
|
|
stack:get_meta():set_string("description", "Partially eaten")
|
|
-- BAD! Modification will be lost
|
|
```
|
|
|
|
Do this instead:
|
|
|
|
```lua
|
|
local inv = player:get_inventory()
|
|
local stack = inv:get_stack("main", 1)
|
|
stack:get_meta():set_string("description", "Partially eaten")
|
|
inv:set_stack("main", 1, stack)
|
|
-- Correct! Item stack is set
|
|
```
|
|
|
|
The behaviour of callbacks is slightly more complicated. Modifying an `ItemStack` you
|
|
are given will change it for the caller too, and any subsequent callbacks. However,
|
|
it will only be saved in the engine if the callback caller sets it.
|
|
|
|
```lua
|
|
core.register_on_item_eat(function(hp_change, replace_with_item,
|
|
itemstack, user, pointed_thing)
|
|
itemstack:get_meta():set_string("description", "Partially eaten")
|
|
-- Almost correct! Data will be lost if another
|
|
-- callback cancels the behaviour
|
|
end)
|
|
```
|
|
|
|
If no callbacks cancel this, the stack will be set and the description will be updated,
|
|
but if a callback does cancel this, then the update may be lost.
|
|
|
|
It's better to do this instead:
|
|
|
|
```lua
|
|
core.register_on_item_eat(function(hp_change, replace_with_item,
|
|
itemstack, user, pointed_thing)
|
|
itemstack:get_meta():set_string("description", "Partially eaten")
|
|
user:get_inventory():set_stack("main", user:get_wield_index(),
|
|
itemstack)
|
|
-- Correct, description will always be set!
|
|
end)
|
|
```
|
|
|
|
If the callbacks cancel or the callback runner doesn't set the stack,
|
|
then the update will still be set.
|
|
If the callbacks or the callback runner set the stack, then the use of
|
|
set_stack doesn't matter.
|