Protect builder mode-switch API against inventory overrides

mods/lobby/functions.lua

@@ -257,6 +257,9 @@ function lobby.builder_to_player(player)
     local pl_name = player:get_player_name()
     local pl_attr = player:get_meta()
     local pl_priv = minetest.get_player_privs(pl_name)
+    local pl_mode = pl_attr:get_string('mode') or ''
+
+    if pl_mode ~= 'builder' then return end
     pl_priv.creative = nil
     pl_priv.fly = nil
     pl_priv.fast = nil
@@ -283,6 +286,9 @@ function lobby.player_to_builder(player)
     local pl_name = player:get_player_name()
     local pl_attr = player:get_meta()
     local pl_priv = minetest.get_player_privs(pl_name)
+    local pl_mode = pl_attr:get_string('mode') or ''
+
+    if pl_mode == 'builder' then return end
     pl_priv.creative = true
     pl_priv.fly = true
     pl_priv.fast = true

test/mocks/player.lua

@@ -2,12 +2,17 @@
 PlayerMock = {}
 PlayerMock.__index = PlayerMock
 
-function PlayerMock:new(pl_name)
+function PlayerMock:new(pl_name, is_builder)
     player = {}
     player.name = pl_name
     player.meta = SettingsMock:new()
     player.inventory = InventoryMock:new()
     setmetatable(player, self)
+    if is_builder == nil or not is_builder then
+        player.meta:set_string('mode', 'solo')
+    else
+        player.meta:set_string('mode', 'builder')
+    end
     return player
 end
 

test/mtmock.lua

@@ -29,8 +29,8 @@ end
 
 --[[ //////////////////// MTMOCK API -- BEGIN \\\\\\\\\\\\\\\\\\\\ ]]--
 
-function mtmock.add_player(pl_name)
-    player = PlayerMock:new(pl_name)
+function mtmock.add_player(pl_name, is_builder)
+    player = PlayerMock:new(pl_name, is_builder)
     minetest.connected_players[pl_name] = player
     return player
 end

test/suites/lobby_functions.lua

@@ -12,12 +12,12 @@ TestLobbyFunctions = {}
     end
 
     function TestLobbyFunctions:testIsBuilder()
-        p1 = mtmock.add_player('p1')
-        p2 = mtmock.add_player('p2')
-        p3 = mtmock.add_player('p3')
-        p4 = mtmock.add_player('p4')
-        b1 = mtmock.add_player('b1')
-        b2 = mtmock.add_player('b2')
+        local p1 = mtmock.add_player('p1')
+        local p2 = mtmock.add_player('p2')
+        local p3 = mtmock.add_player('p3')
+        local p4 = mtmock.add_player('p4')
+        local b1 = mtmock.add_player('b1', true)
+        local b2 = mtmock.add_player('b2', true)
         lu.assertIsNil(lobby.is_builder(nil))
         lu.assertIsFalse(lobby.is_builder(p1))
         lu.assertIsFalse(lobby.is_builder(p2))
@@ -29,14 +29,14 @@ TestLobbyFunctions = {}
 
     -- does lobby.builder_to_player() reset the player privileges?
     function TestLobbyFunctions:testBuilderToPlayerResetsPrivs()
-        b1 = mtmock.add_player('b1')
+        local b1 = mtmock.add_player('b1', true)
         lobby.builder_to_player(b1)
         b1_privs = minetest.get_player_privs('b1')
         lu.assertIsNil(b1_privs.creative)
         lu.assertIsNil(b1_privs.fly)
         lu.assertIsNil(b1_privs.fast)
 
-        b2 = mtmock.add_player('b2')
+        local b2 = mtmock.add_player('b2', true)
         lobby.builder_to_player(b2)
         b2_privs = minetest.get_player_privs('b2')
         lu.assertIsNil(b2_privs.creative)
@@ -46,41 +46,73 @@ TestLobbyFunctions = {}
 
     -- does lobby.builder_to_player() set the player mode to 'solo'?
     function TestLobbyFunctions:testBuilderToPlayerSetsSoloMode()
-        b1 = mtmock.add_player('b1')
+        local b1 = mtmock.add_player('b1', true)
         lobby.builder_to_player(b1)
         b1_attrs = b1:get_meta()
         lu.assertEquals(b1_attrs:get_string('mode'), 'solo')
 
-        b2 = mtmock.add_player('b2')
+        local b2 = mtmock.add_player('b2', true)
         lobby.builder_to_player(b2)
         b2_attrs = b2:get_meta()
         lu.assertEquals(b2_attrs:get_string('mode'), 'solo')
     end
 
+    -- does lobby.builder_to_player() save the main inventory list?
+    function TestLobbyFunctions:testBuilderToPlayerSavesMainInventory()
+        local b1 = mtmock.add_player('b1', true)
+        b1_inv = b1:get_inventory()
+        local b1_main = {'old_main'}
+        b1_inv:set_list('main', b1_main)
+
+        lobby.builder_to_player(b1)
+        local new_bldr = b1_inv:get_list('builder')
+        lu.assertEquals(new_bldr, b1_main)
+    end
+
+    -- does lobby.builder_to_player() respect the player mode?
+    function TestLobbyFunctions:testBuilderToPlayerRespectsMode()
+        local p1 = mtmock.add_player('p1')
+        p1_attr = p1:get_meta()
+        p1_inv = p1:get_inventory()
+        local exp_bldr = {'builder_invlist'}
+        local exp_main = {'player_invlist'}
+        local exp_mode = p1_attr:get_string('mode')
+        p1_inv:set_list('builder', exp_bldr)
+        p1_inv:set_list('main', exp_main)
+
+        lobby.builder_to_player(p1)
+        local act_bldr = p1_inv:get_list('builder')
+        local act_main = p1_inv:get_list('main')
+        local act_mode = p1_attr:get_string('mode')
+        lu.assertEquals(act_bldr, exp_bldr)
+        lu.assertEquals(act_main, exp_main)
+        lu.assertEquals(act_mode, exp_mode)
+    end
+
     -- does lobby.player_to_builder() restore builder privs?
     function TestLobbyFunctions:testPlayerToBuilderRestoresPrivs()
-        p1 = mtmock.add_player('p1')
+        local p1 = mtmock.add_player('p1')
         lobby.player_to_builder(p1)
         p1_privs = minetest.get_player_privs('p1')
         lu.assertIsTrue(p1_privs.creative)
         lu.assertIsTrue(p1_privs.fly)
         lu.assertIsTrue(p1_privs.fast)
 
-        p2 = mtmock.add_player('p2')
+        local p2 = mtmock.add_player('p2')
         lobby.player_to_builder(p2)
         p2_privs = minetest.get_player_privs('p2')
         lu.assertIsTrue(p2_privs.creative)
         lu.assertIsTrue(p2_privs.fly)
         lu.assertIsTrue(p2_privs.fast)
 
-        p3 = mtmock.add_player('p3')
+        local p3 = mtmock.add_player('p3')
         lobby.player_to_builder(p3)
         p3_privs = minetest.get_player_privs('p3')
         lu.assertIsTrue(p3_privs.creative)
         lu.assertIsTrue(p3_privs.fly)
         lu.assertIsTrue(p3_privs.fast)
 
-        p4 = mtmock.add_player('p4')
+        local p4 = mtmock.add_player('p4')
         lobby.player_to_builder(p4)
         p4_privs = minetest.get_player_privs('p4')
         lu.assertIsTrue(p4_privs.creative)
@@ -90,25 +122,60 @@ TestLobbyFunctions = {}
 
     -- does lobby.player_to_builder() set the player mode to 'builder'?
     function TestLobbyFunctions:testPlayerToBuilderSetsBuilderMode()
-        p1 = mtmock.add_player('p1')
+        local p1 = mtmock.add_player('p1')
         lobby.player_to_builder(p1)
         p1_attrs = p1:get_meta()
         lu.assertEquals(p1_attrs:get_string('mode'), 'builder')
 
-        p2 = mtmock.add_player('p2')
+        local p2 = mtmock.add_player('p2')
         lobby.player_to_builder(p2)
         p2_attrs = p2:get_meta()
         lu.assertEquals(p2_attrs:get_string('mode'), 'builder')
 
-        p3 = mtmock.add_player('p3')
+        local p3 = mtmock.add_player('p3')
         lobby.player_to_builder(p3)
         p3_attrs = p3:get_meta()
         lu.assertEquals(p3_attrs:get_string('mode'), 'builder')
 
-        p4 = mtmock.add_player('p4')
+        local p4 = mtmock.add_player('p4')
         lobby.player_to_builder(p4)
         p4_attrs = p4:get_meta()
         lu.assertEquals(p4_attrs:get_string('mode'), 'builder')
     end
 
+    -- does lobby.player_to_builder() restore the builder inventory?
+    function TestLobbyFunctions:testPlayerToBuilderRestoresBuilderInventory()
+        local p1 = mtmock.add_player('p1')
+        p1_inv = p1:get_inventory()
+        local p1_builder = {'p1_builder'}
+        local p1_main = {'p1_player'}
+        p1_inv:set_list('builder', p1_builder)
+        p1_inv:set_list('main', p1_main)
+
+        lobby.player_to_builder(p1)
+        local act_maininv = p1_inv:get_list('main')
+        lu.assertEquals(act_maininv, p1_builder)
+    end
+
+    -- does lobby.player_to_builder() respect the player mode?
+    -- (i.e., inventories do not get changed)
+    function TestLobbyFunctions:testPlayerToBuilderRespectsMode()
+        local b1 = mtmock.add_player('b1', true)
+        b1_attr = b1:get_meta()
+        b1_inv = b1:get_inventory()
+        local exp_bldr = {'invlist:builder'}
+        local exp_main = {'invlist:main'}
+        local exp_mode = b1_attr:get_string('mode')
+        b1_inv:set_list('builder', exp_bldr)
+        b1_inv:set_list('main', exp_main)
+
+        lobby.player_to_builder(b1)
+        local act_bldr = b1_inv:get_list('builder')
+        local act_main = b1_inv:get_list('main')
+        local act_mode = b1_attr:get_string('mode')
+        lu.assertEquals(act_bldr, exp_bldr)
+        lu.assertEquals(act_main, exp_main)
+        lu.assertEquals(act_mode, exp_mode)
+    end
+
 --[[ TestLobbyFunctions ]]--