Improve usability of max and compatibility in init.lua

1. Replace deprecated `minetest.setting_get()` with `minetest.settings:get()` ( https://github.com/minetest/minetest/search?q=setting_get%28 ). A few other functions are deprecated and should probably be addressed later (unless there is a specific reason to keep them, in which case the ones I replaced might need to be reverted).

2. Moved `tonumber()` fix for setting "multihome.max" in minetest.conf to where it's actually retrieved. It was previously added to the check performed when attempting to set a home, which did prevent the error, but any future uses of the `max` variable would potentially run into that same error. Fixing at the source ensures that `max` always has the correct type.

3. Added validation for `max` and `compat`. Gave `max` a lower-bound of 2 (instead of 1) because a max of 1 makes it no different than `sethome`, nor would it technically be _multi_ ;-). Not entirely sure what a "reasonable" upper-bound would/should be, so I just used 10,000, although I don't really see how even 1000 could be used / managed. Should it be lowered to 1000? or 2000 or 5000? or 200? I just figured that more then 10k is someone either testing to see if the limit works, or just trying to break things.

4. Improved error condition handling / messaging when setting a home. Previously it just checked for `>=` and suggested removing 1. However, the more recent addition of also checking to see if the name already exists rightly implies that overwriting an existing entry is valid, so it should be stated in the message. But, if (somehow) the current total is actually greater than the max, then a) removing 1 is only a fix if the total is over the max by only 1, and b) overwriting an existing entry _might_ seem acceptable, except it would then allow for someone having more than the desired max to effectively be grandfathered in with their overage as they could simply replace existing ones forever. So now that's not only handled, but the error message clearly states how many need to be removed before even replacing an existing one will work.
This commit is contained in:
Solomon Rutzky 2021-02-04 00:19:08 -05:00 committed by Elijah Duffy
parent fdc83a4bd1
commit 2ba85a7d5e

View File

@ -2,11 +2,26 @@
multihome = {}
local max = minetest.setting_get("multihome.max") or 5
local compat = minetest.setting_get("multihome.compatibility") or "none"
local import = minetest.setting_get("multihome.import") or "false"
-- Load settings from minetest.conf, else set to default values
local max = tonumber(minetest.settings:get("multihome.max")) or 5
local compat = minetest.settings:get("multihome.compatibility") or "none"
local import = minetest.settings:get("multihome.import") or "false"
if not minetest.get_modpath("sethome") then
-- Out of range is always bad, so log as error
if max < 2 or max > 10000 then
minetest.log("error", "multihome.max value of " .. dump(max) .. " in minetest.conf is outside of 2 - 10000 range. Resetting to 5.")
max = 5
end
-- Any other value is always bad, so log as error
if compat ~= "none" and compat ~= "deprecate" and compat ~= "override" then
minetest.log("error", "multihome.compatibility value of '" .. compat .. "' in minetest.conf is invalid. Valid values are: none (default), deprecate, or override. Resetting to 'none'.")
compat = "none"
end
-- The "sethome" mod can differ per world, so log as warning
if compat ~= "none" and not minetest.get_modpath("sethome") then
minetest.log("warning", "multihome.compatibility value of '" .. compat .. "' in minetest.conf is invalid when sethome mod not present. Resetting to 'none'.")
compat = "none"
end
@ -60,10 +75,14 @@ function multihome.set(player, name, pos)
local pos = pos or vector.round(player:getpos())
local homes = minetest.deserialize(player:get_attribute("multihome"))
local home_count = count_homes(homes)
-- if home doesn't already exist (i.e. a new home is being created), check for space
if not homes[name] and count_homes(homes) >= tonumber(max) then
return false, "Too many homes. Remove one with /multihome del <name> or /delhome <name>"
-- If home doesn't already exist (i.e. a new home is being created), check for space.
-- Else, if count > max (should only happen if max gets lowered), indicate how many to remove.
if not homes[name] and home_count == max then
return false, "Error: too many homes. Replace one by reusing an existing name, or remove one with /multihome del <name> or /delhome <name>"
elseif home_count > max then
return false, "Error: too many homes. Remove at least " .. dump(home_count - max) .. " with /multihome del <name> or /delhome <name>"
end
homes[name] = pos