Bucket with water can stack sometimes #745

New Issue

kay27 · 2020-06-13T15:45:04+02:00

kay27 commented

2020-06-13 15:45:04 +02:00

Usually when you stack several empty buckets and start filling them with water - they change their positions and place in the inventory without being stacked and you can't force them to stack - so I considered it was a correct behavior...

But sometimes when I play and empty some bucket and some refill again several times I find stacked several of them. But I'm still can't stack more then it is...

I captured a video just to confirm what I saw. As for me it's not a big problem for playing but looks like an error.

https://youtu.be/ZG2U1zXzeN8

No idea if it's related to Wuzzy/MineClone2#727

Regards,

Usually when you stack several empty buckets and start filling them with water - they change their positions and place in the inventory without being stacked and you can't force them to stack - so I considered it was a correct behavior... But sometimes when I play and empty some bucket and some refill again several times I find stacked several of them. But I'm still can't stack more then it is... I captured a video just to confirm what I saw. As for me it's not a big problem for playing but looks like an error. https://youtu.be/ZG2U1zXzeN8 No idea if it's related to https://git.minetest.land/Wuzzy/MineClone2/issues/727 Regards,

Wuzzy commented

2020-06-13 19:58:34 +02:00

First of all: Water buckets are supposed to be stackable.

Sadly, I can't reproduce this. I can stack all buckets just fine. Strange.

First of all: Water buckets are supposed to be stackable. Sadly, I can't reproduce this. I can stack all buckets just fine. Strange.

kneekoo commented

2020-06-13 20:32:06 +02:00

I remember this bug, although I think I was in the Minetest Game at the time, which is why I didn't report it here. On the other hand, I never played with several buckets of water at a time in MineClone 2, but I'll try this. :)

kneekoo commented

2020-06-13 22:10:14 +02:00

Ok, I found a way to replicate the stacking bug. :)

Have at least two empty slots in the toolbar.
Place a few empty buckets (stacked) in one slot.
Use an infinite water source to fill the empty buckets.

Here's what happens: https://youtu.be/8yiB2Wqhwgo

Ok, I found a way to replicate the stacking bug. :) 1. Have at least two empty slots in the toolbar. 2. Place a few empty buckets (stacked) in one slot. 3. Use an infinite water source to fill the empty buckets. Here's what happens: https://youtu.be/8yiB2Wqhwgo

kay27 commented

2020-06-15 00:19:29 +02:00

It's here, https://git.minetest.land/Wuzzy/MineClone2/src/branch/master/mods/ITEMS/mcl_buckets/init.lua#L250 :

		-- Add liquid bucket and put it into inventory, if possible.
		-- Drop new bucket otherwise.
		if new_bucket then
			if itemstack:get_count() == 1 then
				return new_bucket
			else
				local inv = user:get_inventory()
				if inv:room_for_item("main", new_bucket) then
					inv:add_item("main", new_bucket)
				else
					minetest.add_item(user:get_pos(), new_bucket)
				end
				if not minetest.settings:get_bool("creative_mode") then
					itemstack:take_item()
				end
				return itemstack

new_bucket with liquid just returns if it is alone, but if not and the mode is not creative - new_bucket adds but not takes and it's a question for me for the moment it adds to the stack or not, cause i'm still a newbie :)

But in the similar minetest mod this case is handled more carefully... IDK where "official" bucket mod's source published now, so it's the code from Windows version 5.3.0-dev:

		-- Check if pointing to a liquid source
		local node = minetest.get_node(pointed_thing.under)
		local liquiddef = bucket.liquids[node.name]
		local item_count = user:get_wielded_item():get_count()

		if liquiddef ~= nil
		and liquiddef.itemname ~= nil
		and node.name == liquiddef.source then
			if check_protection(pointed_thing.under,
					user:get_player_name(),
					"take ".. node.name) then
				return
			end

			-- default set to return filled bucket
			local giving_back = liquiddef.itemname

			-- check if holding more than 1 empty bucket
			if item_count > 1 then

				-- if space in inventory add filled bucked, otherwise drop as item
				local inv = user:get_inventory()
				if inv:room_for_item("main", {name=liquiddef.itemname}) then
					inv:add_item("main", liquiddef.itemname)
				else
					local pos = user:get_pos()
					pos.y = math.floor(pos.y + 0.5)
					minetest.add_item(pos, liquiddef.itemname)
				end

				-- set to return empty buckets minus 1
				giving_back = "bucket:bucket_empty "..tostring(item_count-1)

			end

			-- force_renew requires a source neighbour
			local source_neighbor = false
			if liquiddef.force_renew then
				source_neighbor =
					minetest.find_node_near(pointed_thing.under, 1, liquiddef.source)
			end
			if not (source_neighbor and liquiddef.force_renew) then
				minetest.add_node(pointed_thing.under, {name = "air"})
			end

			return ItemStack(giving_back)
		else
			-- non-liquid nodes will have their on_punch triggered

It's here, https://git.minetest.land/Wuzzy/MineClone2/src/branch/master/mods/ITEMS/mcl_buckets/init.lua#L250 : ``` -- Add liquid bucket and put it into inventory, if possible. -- Drop new bucket otherwise. if new_bucket then if itemstack:get_count() == 1 then return new_bucket else local inv = user:get_inventory() if inv:room_for_item("main", new_bucket) then inv:add_item("main", new_bucket) else minetest.add_item(user:get_pos(), new_bucket) end if not minetest.settings:get_bool("creative_mode") then itemstack:take_item() end return itemstack ``` new_bucket with liquid just returns if it is alone, but if not and the mode is not creative - new_bucket adds but not takes and it's a question for me for the moment it adds to the stack or not, cause i'm still a newbie :) But in the similar minetest mod this case is handled more carefully... IDK where "official" bucket mod's source published now, so it's the code from Windows version 5.3.0-dev: ``` -- Check if pointing to a liquid source local node = minetest.get_node(pointed_thing.under) local liquiddef = bucket.liquids[node.name] local item_count = user:get_wielded_item():get_count() if liquiddef ~= nil and liquiddef.itemname ~= nil and node.name == liquiddef.source then if check_protection(pointed_thing.under, user:get_player_name(), "take ".. node.name) then return end -- default set to return filled bucket local giving_back = liquiddef.itemname -- check if holding more than 1 empty bucket if item_count > 1 then -- if space in inventory add filled bucked, otherwise drop as item local inv = user:get_inventory() if inv:room_for_item("main", {name=liquiddef.itemname}) then inv:add_item("main", liquiddef.itemname) else local pos = user:get_pos() pos.y = math.floor(pos.y + 0.5) minetest.add_item(pos, liquiddef.itemname) end -- set to return empty buckets minus 1 giving_back = "bucket:bucket_empty "..tostring(item_count-1) end -- force_renew requires a source neighbour local source_neighbor = false if liquiddef.force_renew then source_neighbor = minetest.find_node_near(pointed_thing.under, 1, liquiddef.source) end if not (source_neighbor and liquiddef.force_renew) then minetest.add_node(pointed_thing.under, {name = "air"}) end return ItemStack(giving_back) else -- non-liquid nodes will have their on_punch triggered ```

Wuzzy added the

bug

items

labels 2020-06-16 04:00:00 +02:00

kay27 referenced this issue

2020-07-27 18:44:22 +02:00

Furnaces seem to stop cooking when players are far away #533

kay27 commented

2020-07-29 01:25:49 +02:00

First of all: Water buckets are supposed to be stackable.

@Wuzzy Would you mind telling me how strictly you suppose to follow https://minecraft.gamepedia.com/Water_Bucket ?

Water bucket is not stackable there in the article. Empty bucket has stack size 16 like in MCL2. Whether we like it or not, there are also some physical principles of that...

> First of all: Water buckets are supposed to be stackable. @Wuzzy Would you mind telling me how strictly you suppose to follow https://minecraft.gamepedia.com/Water_Bucket ? Water bucket is not stackable there in the article. Empty bucket has stack size 16 like in MCL2. Whether we like it or not, there are also some physical principles of that...

kneekoo commented

2020-07-29 08:31:40 +02:00

Physical principles? :)) An empty bucket has the same physical size as a full bucket. And stacking solid blocks up to 64 is far from reality, just as unreal it is stacking 64 chests inside a single chest slot. :)) Or getting apples from oak trees. :))

We should just settle on some game rules and make sure they are reliably implemented. The current implementation of the water bucket is buggy.

Physical principles? :)) An empty bucket has the same physical size as a full bucket. And stacking solid blocks up to 64 is far from reality, just as unreal it is stacking 64 chests inside a single chest slot. :)) Or getting apples from oak trees. :)) We should just settle on some game rules and make sure they are reliably implemented. The current implementation of the water bucket is buggy.

kay27 commented

2020-07-29 13:29:31 +02:00

You right about the size, and about the logic which can absent at all, but I meant the difference of empty bucket which allows to stack it the way like on the picture:

I'm trying to fix and have a strange sence that maybe I'm doing wrong thing so I decided to clarify.

You right about the size, and about the logic which can absent at all, but I meant the difference of empty bucket which allows to stack it the way like on the picture:![image](/attachments/0af12f03-011f-47f1-a2d6-ce1a62bd9d0d) I'm trying to fix and have a strange sence that maybe I'm doing wrong thing so I decided to clarify.

image.png

153 KiB

kneekoo commented

2020-07-29 14:33:05 +02:00

I had the same picture in mind when you first mentioned physical principle, but it's still funny because a lot of things are complete nonsense in Minecraft/Minetest/MineClone. :P

So it's ultimately a matter of decision on how it should be implemented. But as with all the stackable items that can hold something in them (buckets, bowls, bottles), normally the filled ones should not stack.

I had the same picture in mind when you first mentioned physical principle, but it's still funny because a lot of things are complete nonsense in Minecraft/Minetest/MineClone. :P So it's ultimately a matter of decision on how it should be implemented. But as with all the stackable items that can hold something in them (buckets, bowls, bottles), normally the filled ones should not stack.

kay27 commented

2020-07-29 15:15:38 +02:00

I feel myself so stupid just because of I new to Lua and Minetest and MineClone 2 :(
I don't know some very base things and waste a lot of time because of it...
When I read about the itemstack which returns after place action they say it must contain:

nil (if we don't change it)
leftover items.

There is no way to replace the stack with some new items, at least how it was documented, but mcl_bucket does it: replaces old bucket (unfilled) with filled one and back. So I firstly thought the problem is because of that and rewrote it to comply the rules, but... surprizingly it didn't help at all :)

Then I added some debug information, I don't know what's the reason to gather metadata from nodes and store them into the stack so I wanted to see them to start and what I saw is on the first screen.

When I removed this unclear (for me) meta gathering - bucket stacking became to work fine. Image number 2.

Now I may show you the fix but it still contain the first part when I tried to rewrite it to comply its own docs :) Seems now I need to start again from upstream version and try to just remove meta... but now I have no time for that and just want to document what I did.

now #contribution_inside but needs to be cleared... Dispense part is touched with no purpose and many other probably useless things...

7d6a9c6d66

I feel myself so stupid just because of I new to Lua and Minetest and MineClone 2 :( I don't know some very base things and waste a lot of time because of it... When I read about the ```itemstack``` which returns after ```place``` action they say it must contain: + ```nil``` (if we don't change it) + leftover items. There is no way to replace the stack with some new items, at least how it was documented, but ```mcl_bucket``` does it: replaces old bucket (unfilled) with filled one and back. So I firstly thought the problem is because of that and rewrote it to comply the rules, but... surprizingly it didn't help at all :) Then I added some debug information, I don't know what's the reason to gather metadata from nodes and store them into the stack so I wanted to see them to start and what I saw is on the first screen. When I removed this unclear (for me) meta gathering - bucket stacking became to work fine. Image number 2. Now I may show you the fix but it still contain the first part when I tried to rewrite it to comply its own docs :) Seems now I need to start again from upstream version and try to just remove meta... but now I have no time for that and just want to document what I did. now #contribution_inside but needs to be cleared... Dispense part is touched with no purpose and many other probably useless things... https://git.minetest.land/kay27/MineClone2/commit/7d6a9c6d66c4403cf12dcfb6ca934025ca44ee6b ![image](/attachments/d73e5611-249a-4847-a834-86d7fd240eee) ![image](/attachments/aa80d5ca-9362-4dff-9638-1d3f10faf887)

image.png

185 KiB

image.png

317 KiB

kay27 commented

2020-07-29 23:51:47 +02:00

I've reverted all my changes and removed only medadata gathering.

And it all works.

But there's a bevaioural difference from my way of itemstack usage: if you have 16 empty buckets and start to fill them all one by one... 16th of them wont stack to precious 15 filled already. You will get it instead of the empty - it transforms right in your hand.

Probably it's good......... My repo contains good version now, but I have no readable patch for it, will try to make it later to merge...

By the way, why somebody gathers metadata to itemstack?

I've reverted all my changes and removed only medadata gathering. And it all works. But there's a bevaioural difference from my way of ```itemstack``` usage: if you have 16 empty buckets and start to fill them all one by one... 16th of them wont stack to precious 15 filled already. You will get it instead of the empty - it transforms right in your hand. Probably it's good......... My repo contains good version now, but I have no readable patch for it, will try to make it later to merge... By the way, why somebody gathers metadata to itemstack?

kay27 commented

2020-07-30 00:03:32 +02:00

The patch for revising and picking it up if okay: 36074e6e3c

The patch for revising and picking it up if okay: https://git.minetest.land/kay27/MineClone2/commit/36074e6e3c8722c3956833acf3ca8614dfd2675b

kay27 commented

2020-07-30 02:37:23 +02:00

@kneekoo @Wuzzy By the way water bottles do not stack, speaking of the logic :(

@kneekoo @Wuzzy By the way water bottles do not stack, speaking of the logic :( ![image](/attachments/508b1d0c-3708-40ce-a829-d5994695eb6c)

image.png

249 KiB

kneekoo commented

2020-07-30 09:37:27 +02:00

I know, the water buckets were the exception. As I mentioned earlier, filled items shouldn't stack - buckets, bowls, bottles (are there others?)

Wuzzy commented

2020-07-31 09:45:08 +02:00

Potions should not stack because they give you effects. This basically means you can only carry a small number of potions with you, and you have to organize. This is by design.

kay27 commented

2020-08-02 17:49:34 +02:00

I tried to find old version of bucket with meta data usage and I found one of the year 2013 by @TenPlus1: https://forum.minetest.net/viewtopic.php?t=7633

This code is really rare now. Meta data was used in it to determine fullness on placing liquids:

                                -- Check if pointing to a buildable node
                                local node = minetest.get_node(pointed_thing.under)
                                local fullness = tonumber(itemstack:get_metadata())
                                if not fullness then fullness = LIQUID_MAX end

This is not used now, even in minetest_game ( https://github.com/minetest/minetest_game/blob/master/mods/bucket/init.lua ).

So I removed the usage of metadata for upstream too, 125840c9e4

It is now fixed.

I tried to find old version of ```bucket``` with meta data usage and I found one of the year 2013 by @TenPlus1: https://forum.minetest.net/viewtopic.php?t=7633 This code is really rare now. Meta data was used in it to determine ```fullness``` on placing liquids: ``` -- Check if pointing to a buildable node local node = minetest.get_node(pointed_thing.under) local fullness = tonumber(itemstack:get_metadata()) if not fullness then fullness = LIQUID_MAX end ``` This is not used now, even in minetest_game ( https://github.com/minetest/minetest_game/blob/master/mods/bucket/init.lua ). So I removed the usage of metadata for upstream too, https://git.minetest.land/Wuzzy/MineClone2/commit/125840c9e46ba62e9a77ab2dd51239e3bec36a0a It is now fixed.

👍 2

kay27 closed this issue

2020-08-02 17:49:36 +02:00

kay27 referenced this issue

2021-04-12 08:50:46 +02:00

Compose and document a way how we accept/unaccept features #1537

Sign in to join this conversation.