Fix river water bucket interaction.

Added explicits tests for river water bucket.
Minor updates to code comments.

mods/ITEMS/mcl_cauldrons/register.lua

@@ -61,14 +61,15 @@ end
 MODENV.merge_tables = merge_tables
 
 
-----
+--------
+--------
 -- Item Proxies
 --
 -- Interaction with items are done through a proxy that presume certain
 -- behaviors of actual/registered items.
 -- The idea is to avoid invoking mod-specific functions by name
--- (relying on callbacks from node definition), as mods may be extended,
--- replaced, or disabled.
+-- (therefore relying on callbacks from node definition),
+-- as mods may be extend, replace, or disable items.
 --
 -- Initial item proxies include:
 --   BucketProxy - interacting with bucket-like items (change fill level by 3)
@@ -76,6 +77,14 @@ MODENV.merge_tables = merge_tables
 --   BannerProxy - banner-like items (wash banner, deplete 1 level)
 
 -- Items using rule-based FSM transition for interacting with cauldron.
+-- State-transition rules to specify transitions.
+--   State vector is tuple(substance:string, fill_level:number)
+--     Item is either item name string, or function(string,table) predicate.
+--   Input is Item
+--   Next State is tuple(next_substance:string, next_level:number)
+--   Output is another Item
+--      SoundSpecifier is soundspec string or function(itemstack,node) callback.
+
 mcl_cauldrons.RuleBasedProxyItem = {
 	-- prototype derivation to keep separate .rules
 	__call = function(self, ...)
@@ -134,14 +143,6 @@ mcl_cauldrons.RuleBasedProxyItem = {
 setmetatable(mcl_cauldrons.RuleBasedProxyItem, mcl_cauldrons.RuleBasedProxyItem)
 
 
--- Item/Cauldron interactions based on Finite State Machine,
--- rules to specify transitions.
---  State vector is tuple(substance:string, fill_level:number)
---    Item is either item name, or function(string,table) predicate.
---  Input is Item
---  Next State is tuple(next_substance:string, next_level:number)
---  Output is another Item
-
 -- add an item-exchange rule following the basic cauldron fill/empty and bucket fill/empty pattern:
 -- * cauldron consistes of a particular substance and specific level.
 -- * cauldron turns into one of a particular substance and level.
@@ -151,15 +152,15 @@ setmetatable(mcl_cauldrons.RuleBasedProxyItem, mcl_cauldrons.RuleBasedProxyItem)
 -- match_level: cauldron fill level matching this rule (nil means any, true maps to full)
 -- input_constraint:
 --    string => exact match with item name to trigger rule.
---    function(string) => predicate given item name, to trigger rule.
+--    function(ItemStack) => predicate given item stack, to trigger rule.
 -- next_substance: new cauldron content (nil means empty)
 -- next_level: new cauldron fill level (true means full, 0 means empty)
 -- output_specifer:
 --    string => specific item name to generate after successful rule match
---    function(string, table) => given old item name and new cauldron node, generate new item name on rule match
+--    function(ItemStack, table) => given old item stack and new cauldron node, generate new item stack on rule match
 -- sound_specifier:
 --    string => specific item name to play 'place' sound on match rule.
---    function(string, table) => given old item name and new cauldron node, return (soundspec, soundparam)
+--    function(ItemStack, table) => given ItemStack and new cauldron node, return (soundspec, soundparam)
 
 function mcl_cauldrons.RuleBasedProxyItem:add_basic_rule(match_substance, match_level, input_constraint, next_substance, next_level, output_specifier, sound_specifier)
 	local rule = {
@@ -221,7 +222,6 @@ function mcl_cauldrons.RuleBasedProxyItem:apply_basic_rules(itemstack, cauldron_
 		-- `true` as alias for maximum.
 		next_level = mcl_cauldrons.BUCKET_FILL_LEVELS
 	end
-	--local newnode = mcl_cauldrons.normalize_cauldron(match_rule.next_substance, next_level)
 	local newnode = mcl_cauldrons.node_set_level(cauldron_node, next_level, match_rule.next_substance)
 	local newitemstack
 	if type(match_rule.output_specifier) == "string" then
@@ -267,7 +267,8 @@ end
 
 
 
----
+-------
+-------
 -- Bucket(-like) interactions with cauldron.
 -- * empty bucket: requires full cauldron, produce something bucket, cauldron becomes empty.
 -- * something bucket: requires empty cauldron, produce empty bucket, cauldron becomes full.
@@ -323,8 +324,8 @@ BucketProxy:add_basic_rule("water", nil, BucketProxy.is_water,  "water", true, "
 BucketProxy:add_basic_rule(nil, 0, BucketProxy.is_water,  "water", true, "mcl_buckets:bucket_empty", BucketProxy.sound_water)
 
 
----
-
+-------
+-------
 -- Bottle(-like) interaction with Cauldron
 -- * empty bottle: requires non-empty cauldron, generate something bottle, reduce cauldron level by 1
 -- * something bottle: empty cauldron or same-liquid cauldron, generate empty bottle, increase cauldron level by 1
@@ -371,7 +372,8 @@ BottleProxy:add_basic_rule("water", 1, BottleProxy.is_water, "water", 2, BottleP
 BottleProxy:add_basic_rule("water", 2, BottleProxy.is_water, "water", 3, BottleProxy.make_empty, BottleProxy.sound_water)
 
 
----
+-------
+-------
 -- Banner(-like) interaction with Cauldron.
 
 local BannerProxy = mcl_cauldrons.RuleBasedProxyItem()
@@ -386,6 +388,7 @@ function BannerProxy.is_banner(itemstack)
 	-- other tests.
 end
 
+-- In the case of banners, the resulting itemstack is the old itemstack with metadata modification (handled by on_wash()).
 function BannerProxy.wash_banner(itemstack, cauldron_node)
 	local nodedef = itemstack:get_definition()
 	local washed_itemstack
@@ -477,10 +480,8 @@ function mcl_cauldrons.Cauldron:node_drain_levels(node, change_levels, substance
 end
 
 -- carry out all the duties of on_rightclick().
+-- returns itemstack for item in hand (may be same `itemstack`).
 function mcl_cauldrons.Cauldron:apply_item(pos, node, user, itemstack)
-	-- list of actions to carry out as a result of applying item.
-	-- allows for rollback/cancelling in case of errors or exceptions.
-	local plan = {}
 	for _, item_handler in ipairs(self.registered_proxies) do
 		local handled, retval = item_handler:do_apply(pos, node, user, itemstack)
 		if handled then return retval end
@@ -518,9 +519,7 @@ mcl_cauldrons.register_cauldron_node("mcl_cauldrons:cauldron", {
 	_mcl_hardness = 2,
 	_mcl_blast_resistance = 2,
 
-	-- delegated by mcl_buckets and mcl_potions
-	-- returns true to indicate event is finished (no further processing)
-	-- returns false if event not handled (expect caller to fallthrough to non-cauldron behavior).
+	-- delegated to by mcl_buckets and mcl_potions.
 	on_rightclick = function(place_pos, node, user, itemstack)
 		return mcl_cauldrons.Cauldron:apply_item(place_pos, node, user, itemstack)
 	end,
@@ -567,10 +566,12 @@ mcl_cauldrons.register_cauldron_node("mcl_cauldrons:cauldron_water_3",
 }), "mcl_cauldrons:cauldron")
 
 
+-- River Water Cauldron
 if minetest.get_modpath("mclx_core") then
 	dosubfile("x_river_water.lua")
 end
 
+-- Lava Cauldron
 if minetest.get_modpath("mclx_core") then
 	dosubfile("x_lava.lua")
 end

mods/ITEMS/mcl_cauldrons/spec/mcl_cauldron_spec.lua

@@ -467,6 +467,78 @@ describe("MineClone2 cauldrons test", function()
 		end)
 	end)
 
+	describe("apply river water bucket to cauldron #riverwaterbucket", function()
+		it(", implementation details", function()
+			assert.is_not_nil(minetest.registered_nodes["mcl_cauldrons:cauldron_river_water_1"])
+			assert.is_not_nil(minetest.registered_nodes["mcl_cauldrons:cauldron_river_water_2"])
+			assert.is_not_nil(minetest.registered_nodes["mcl_cauldrons:cauldron_river_water_3"])
+			----
+			-- test the implementation details (gets messy).
+			local itemstack0 = ItemStack(s_bucket1r)
+
+			-- river water bucket + empty cauldrons = full river water cauldron, empty bucket
+			local pos = CAULDRON_POS
+			local node = { name=s_cauldron0 }
+
+			local newnode, newitemstack = mcl_cauldrons.BucketProxy:apply_basic_rules(itemstack0, node)
+			assert.is_not_nil(newnode)
+			assert.is_not_nil(newitemstack)
+			assert.equals(s_cauldron3r, newnode.name)
+			assert.equals(s_bucket0, newitemstack:get_name())
+
+			-- river water bucket + 1/3 cauldron = full river water cauldron, empty bucket
+			node = { name=s_cauldron1r }
+			local newnode, newitemstack = mcl_cauldrons.BucketProxy:apply_basic_rules(itemstack0, node)
+			assert.equals(s_cauldron3r, newnode.name)
+			assert.equals(s_bucket0, newitemstack:get_name())
+
+			-- river water bucket + 2/3 cauldron = full river water cauldron, empty bucket
+			node = { name=s_cauldron2r }
+			local newnode, newitemstack = mcl_cauldrons.BucketProxy:apply_basic_rules(itemstack0, node)
+			assert.equals(s_cauldron3r, newnode.name)
+			assert.equals(s_bucket0, newitemstack:get_name())
+
+			-- river water bucket + full cauldron = full river water cauldron, empty bucket (wastes the river water bucket)
+			node = { name=s_cauldron3r }
+			local newnode, newitemstack = mcl_cauldrons.BucketProxy:apply_basic_rules(itemstack0, node)
+			assert.equals(s_cauldron3r, newnode.name)
+			assert.equals(s_bucket0, newitemstack:get_name())
+		end)
+
+		----
+		-- mimick world activity (validate for gameplay)
+
+		it(", mimick world activity", function()
+			-- apply river water bucket to full cauldron -> cauldron_water_3, empty bucket (bucket wasted)
+			force_cauldron(s_cauldron3r)
+			force_inv(s_bucket1r)
+			rclick_cauldron()
+			assert_cauldron(s_cauldron3r)
+			assert_inv(s_bucket0)
+
+			-- apply river water bucket to 2/3 cauldron -> cauldron_water_3, empty bucket
+			force_cauldron(s_cauldron2r)
+			force_inv(s_bucket1r)
+			rclick_cauldron()
+			assert_cauldron(s_cauldron3r)
+			assert_inv(s_bucket0)
+
+			-- apply river water bucket to 1/3 cauldron -> cauldron_water_3, empty bucket
+			force_cauldron(s_cauldron1r)
+			force_inv(s_bucket1r)
+			rclick_cauldron()
+			assert_cauldron(s_cauldron3r)
+			assert_inv(s_bucket0)
+
+			-- apply river water bucket to empty cauldron -> cauldron_water_3, empty bucket
+			force_cauldron(s_cauldron0)
+			force_inv(s_bucket1r)
+			rclick_cauldron()
+			assert_cauldron(s_cauldron3r)
+			assert_inv(s_bucket0)
+		end)
+	end)
+
 	describe("apply lava bucket to cauldron #lavabucket", function()
 		it(", implementation details", function()
 			assert.is_not_nil(minetest.registered_nodes["mcl_cauldrons:cauldron_lava_1"])

mods/ITEMS/mcl_cauldrons/x_river_water.lua

@@ -41,21 +41,22 @@ mcl_cauldrons.register_cauldron_node("mcl_cauldrons:cauldron_river_water_3",
 
 local BucketProxy = mcl_cauldrons.BucketProxy
 
-function BucketProxy.is_river_water(bucket_name, bucket_def)
-	if (bucket_name == "mcl_buckets:bucket_river_water") then return true end
+function BucketProxy.is_river_water(itemstack)
+	if (itemstack:get_name() == "mcl_buckets:bucket_river_water") then return true end
 	-- other tests.
 end
-function BucketProxy.make_river_water(bucket_name, cauldron_node)
+
+function BucketProxy.make_river_water(itemstack, cauldron_node)
 	for scan_name, scan_def in pairs(minetest.registered_items) do
 		-- TODO: expand mcl_buckets node definitions.
 	end
 end
 
--- empty bucket + full river water cauldron -> empty cauldron, river water bucket
+-- full river water cauldron + empty bucket -> empty cauldron, river water bucket
 BucketProxy:add_basic_rule("river_water", 3, BucketProxy.is_empty,  nil, 0, "mcl_buckets:bucket_river_water", BucketProxy.sound_water)
--- river water bucket + any river water cauldron -> full river water cauldron, empty bucket (may waste bucket)
+-- any river water cauldron + river water bucket -> full river water cauldron, empty bucket (may waste bucket)
 BucketProxy:add_basic_rule("river_water", nil, BucketProxy.is_river_water,  "river_water", true, "mcl_buckets:bucket_empty", BucketProxy.sound_water)
--- river water bucket + empty cauldron -> full river water cauldron, empty bucket
+-- empty cauldron + river water bucket -> full river water cauldron, empty bucket
 BucketProxy:add_basic_rule(nil, 0, BucketProxy.is_river_water,  "river_water", true, "mcl_buckets:bucket_empty", BucketProxy.sound_water)