Skip to content

ECS Party | The Bag#2989

Merged
IntegratedQuantum merged 15 commits into
PixelGuys:masterfrom
IntegratedQuantum:bag
Apr 30, 2026
Merged

ECS Party | The Bag#2989
IntegratedQuantum merged 15 commits into
PixelGuys:masterfrom
IntegratedQuantum:bag

Conversation

@IntegratedQuantum
Copy link
Copy Markdown
Member

@IntegratedQuantum IntegratedQuantum commented Apr 27, 2026

Since it exists now, I decided to put the relevant stuff into the ECS and following the pattern: Anything remotely related to the ECS gets the ECS label and number in the title :P

Screenshot at 2026-04-28 21-19-39

Remaining work:

  • make an issue for the missing inventory actions (right clicking, shift clicking)
  • display the capacity

fixes #73

@tillpp
Copy link
Copy Markdown
Contributor

tillpp commented Apr 27, 2026

Ahhh! i already have an ECS14 , not pushed tho

@tillpp
Copy link
Copy Markdown
Contributor

tillpp commented Apr 27, 2026

I decided to put the relevant stuff into the ECS and following the pattern: Anything remotely related to the ECS gets the ECS label and number in the title!

HEy! all ECS parts are related to the big chunky #2652 !
A complete misunderstanding what the label stands for!

notice for example how PlayerComponent is not part of the ECS series: #2889 even though its ECS related, because it's not 2652 related!

Or PVP #2890 is also not part of the series, even though it is ECS related! because its not 2652 related!

all ECS parts are intended to be related to 2652

If you could have handled a 4k PR i would have put all ECS parts into this one PR

so name change requested!

@tillpp
Copy link
Copy Markdown
Contributor

tillpp commented Apr 27, 2026

Also: please include screenshots when adding something visual

Comment thread src/server/world.zig Outdated

user.spawnPos = playerData.get(?Vec3d, "playerSpawnPos", null);

if (main.entity.components.@"cubyz:bag".server.get(user.id) == null) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you move this into initPlayer?
to make it more consistent with the other player related components (@"cubyz:model")
or do it the other way around, having the default model loaded where the bag is loaded,

Maybe this is a pattern that will occur so often, that it would make sense to have some kind of general main.entity.server.initNewPlayer functionality.

Copy link
Copy Markdown
Contributor

@Wunka Wunka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small thing so we don't need to rewrite this later.

Comment thread src/sync.zig Outdated
};

const TakeFromPlayerBag = struct { // MARK: TakeFromPlayerBag
dest: InventoryAndSlot,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you maybe use here the Inventories as the destinations, with a putItemsInto.
Currently this does not change really anything but would stop us from later needing to refactor this if needed.
I also thought maybe we want shift clicking in general to have the hand as a fallback?


const topItem = self.inventory.peek(0);
const shouldRenderStackSizeText = topItem.item.stackSize() > 1;
if (shouldRenderStackSizeText) {
Copy link
Copy Markdown
Contributor

@Wunka Wunka Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from here a lot is just copy from itemslot. I know they have key differences but maybe we want to generlize something here? Like a itemStackInformation we can render on top?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to move part of the rendering code to items.zig

@IntegratedQuantum IntegratedQuantum changed the title ECS Part 14 | The Bag ECS Party | The Bag Apr 28, 2026
@IntegratedQuantum
Copy link
Copy Markdown
Member Author

A complete misunderstanding what the label stands for!

Oh sorry, I renamed it to avoid the name conflicts :​P

@IntegratedQuantum
Copy link
Copy Markdown
Member Author

Also: please include screenshots when adding something visual

https://discord.com/channels/443805812390100992/574185221939789855/1498398538424651817

@tillpp
Copy link
Copy Markdown
Contributor

tillpp commented Apr 28, 2026

Also: please include screenshots when adding something visual

In the PR!!!

Oh sorry, I renamed it to avoid the name conflicts :​P

what does this have to do with a Party?

Comment thread src/sync.zig Outdated
Comment on lines +1357 to +1369
var remainingAmount = @min(self.amount, bag.peek(0).amount);
const item = bag.peek(0).item;
outer: for (self.destinations.inventories) |inv| {
for (inv._items, 0..) |itemStack, slot| {
if (remainingAmount == 0) break :outer;
if (itemStack.item != .null and !std.meta.eql(itemStack.item, item)) continue;

const amount = @min(remainingAmount, item.stackSize() - itemStack.amount);
remainingAmount -= amount;

ctx.execute(.{.takeFromBag = .{.source = bag, .dest = .{.inv = inv, .slot = @intCast(slot)}, .amount = amount}});
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you not just use the putItemsInto? you would just need to expand the Provider (in a fast done not complete tested version) this is how the Provider would look like:

	const Provider = union(enum) {
		move: InventoryAndSlot,
		create: Item,
		fromBag: *BagInventory,

		pub fn getBaseOperation(provider: Provider, dest: InventoryAndSlot, amount: u16) sync.Command.BaseOperation {
			return switch (provider) {
				.move => |slot| .{.move = .{
					.dest = dest,
					.amount = amount,
					.source = slot,
				}},
				.create => |item| .{.create = .{
					.dest = dest,
					.amount = amount,
					.item = item,
				}},
				.fromBag => |bag| .{.takeFromBag = .{
					.dest = dest,
					.source = bag,
					.amount = amount,
				}},
			};
		}

		pub fn getItem(provider: Provider) Item {
			return switch (provider) {
				.move => |slot| slot.ref().item,
				.create => |item| item,
				.fromBag => |bag| bag.peek(0).item,
			};
		}
	};

maybe I am not seeing a different problem?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my current code I stop depositing when the item changes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, ok. yeah makes sense

@IntegratedQuantum
Copy link
Copy Markdown
Member Author

what does this have to do with a Party?

It's a birthday party for the ECS: The first actual ECS feature that lands :​P

@tillpp
Copy link
Copy Markdown
Contributor

tillpp commented Apr 28, 2026

It's a birthday party for the ECS: The first actual ECS feature that lands :​P

NOOOO!
you are the Evil Snale
YOu always have been!

@Wunka
Copy link
Copy Markdown
Contributor

Wunka commented Apr 28, 2026

when dragging over the bag you take from it but also put things into the selected itemslots

2026-04-28.21-41-57.mp4

this is I think a more simple example to the general problem. It seems the taking is done on release even if you are not hovering over the bag anymore

2026-04-28.21-46-00.mp4

@Wunka
Copy link
Copy Markdown
Contributor

Wunka commented Apr 28, 2026

Another thing I noted. When having for example a tool procedural item at the top, there is no tooltip.

@IntegratedQuantum
Copy link
Copy Markdown
Member Author

Tooltip is blocked by #2527

@IntegratedQuantum IntegratedQuantum requested a review from Wunka April 29, 2026 05:27
Comment thread src/sync.zig Outdated
for (inv._items, 0..) |itemStack, slot| {
if (remainingAmount == 0) break :outer;
if (itemStack.item != .null and !std.meta.eql(itemStack.item, item)) continue;
if (!std.meta.eql(bag.peek(0).item, item)) continue;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you not break here? this will never change after it once becomes true

Comment thread src/sync.zig Outdated
Comment on lines +1359 to +1368
outer: for (self.destinations.inventories) |inv| {
for (inv._items, 0..) |itemStack, slot| {
if (remainingAmount == 0) break :outer;
if (itemStack.item != .null and !std.meta.eql(itemStack.item, item)) continue;
if (!std.meta.eql(bag.peek(0).item, item)) continue;

const amount = @min(remainingAmount, item.stackSize() - itemStack.amount);
remainingAmount -= amount;

ctx.execute(.{.takeFromBag = .{.source = bag, .dest = .{.inv = inv, .slot = @intCast(slot)}, .amount = amount}});
Copy link
Copy Markdown
Contributor

@Wunka Wunka Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in general this does not match the behavior we want for inventory -> inventories.
I would still suggest to just use putItemsInto and add there the if(!std.meta.eql(provider.getItem(), item)) break behavior

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to instead precalculate the amount here.

@ikabod-kee
Copy link
Copy Markdown
Collaborator

Wait, so the bag can hold 120 STACKS of items???

@IntegratedQuantum IntegratedQuantum requested a review from Wunka April 29, 2026 15:17
Comment thread src/sync.zig Outdated
amount +|= stack.amount;
}
amount = @min(amount, self.amount);
std.debug.assert(self.destinations.putItemsInto(ctx, amount, .{.bag = bag}) == 0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would mean, you crash the game if the amount doesn't fit into the destinations. I would say just ignore the return value

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, I wasn't thinking

@IntegratedQuantum IntegratedQuantum requested a review from Wunka April 29, 2026 15:22
Copy link
Copy Markdown
Contributor

@Wunka Wunka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like it!
The most that the amount which is shown is stack independent and instead item driven

Comment thread src/entityComponent/bag.zig
@IntegratedQuantum IntegratedQuantum merged commit 94fee0a into PixelGuys:master Apr 30, 2026
1 check passed
codemob-dev pushed a commit to codemob-dev/Cubyz that referenced this pull request May 1, 2026
Since it exists now, I decided to put the relevant stuff into the ECS
and following the pattern: Anything remotely related to the ECS gets the
ECS label and number in the title :P

<img width="2560" height="1413" alt="Screenshot at 2026-04-28 21-19-39"
src="https://github.com/user-attachments/assets/3f176c86-9408-4c3f-b49d-791be39c83a4"
/>

Remaining work:
- [x] make an issue for the missing inventory actions (right clicking,
shift clicking)
- [x] display the capacity

fixes PixelGuys#73
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inventory Enhancement: The Bag

4 participants