minetest-mods / skinsdb

Player skin mod, supporting unified_inventory, sfinv and smart_inventory
27 stars 41 forks source link

Fix error in '/skinsdb set' without parameter #46

Closed Lejo1 closed 4 years ago

SmallJoker commented 4 years ago

But the command ui does not need a parameter.

Lejo1 commented 4 years ago

Right, fixed that. Also fixed a crash when using /skinsdb list public but I'm not sure if I destroyed it's functionality as I'm not sure what it should list...

bell07 commented 4 years ago

The first change about the parameter is ok. The "set" does not sense without a parameter. Please add an error message to the chat that the "skinsdb set" requires skin key.

The second change in line 53 is not ok. The public skins are not assigned to any player. But I see it crashes in is_applicable_for_player. Please fix is_applicable_for_player line instead

return minetest.check_player_privs(playername, {server=true}) or assigned_player == nil or assigned_player == true or (playername and assigned_player:lower() == playername:lower())

Lejo1 commented 4 years ago

@bell07 Thanks for clearing this up!

Changed it accordingly

bell07 commented 4 years ago

See the second fix is still not "perfect". The "assigned_player == nil or assigned_player == true should be still visible...

return assigned_player == nil or assigned_player == true or
 (playername and (minetest.check_player_privs(playername, {server=true}) or 
            assigned_player:lower() == playername:lower())
Lejo1 commented 4 years ago

Changed it again

bell07 commented 4 years ago

Thank you! Last thing is to add the new string "Requires skin key" to the locale/template.txt and maybe to translation file of your language

Lejo1 commented 4 years ago

Added that!

bell07 commented 4 years ago

Hm, there is still an issue with public skins. The brace is on wrong place. My fault, sorry for that. Right is

    return assigned_player == nil or assigned_player == true or
            playername and (minetest.check_player_privs(playername, {server=true}) or
            assigned_player:lower() == playername:lower())
Lejo1 commented 4 years ago

Right, changed