Sort button attempt 2 by Crepestrom · Pull Request #3194 · PixelGuys/Cubyz

Crepestrom · 2026-06-08T19:45:27Z

Sort button now functions as normal

Before sorting it will compress all items into their max stacks and push them all to the lowest index getting rid of any holes

Then the sort button will first seperate items into procederal items and nonprocedural items
then they will be sorted by tag
items that share the same first tag will be internally sorted by their second tag and so on and so forth
procedural items are sorted by durability

Also allows buttons to be hidden
if a button is hidden the icon within it is extended to the size of the whole button (its scaled up).

Wunka

I still think that std.sort is probably way nicer and easier. Here is an example (not tested) based on your current code (but of course not completly mapped and I with an added getTags method for the Item struct):

pub fn sortItems(source: ClientInventory, ignoredSlotCount: usize) void {
	compressItems(source);
	const ctx: SortContext = .{.inv = source};
	std.sort.insertionContext(ignoredSlotCount, source.super._items.len, ctx);
}

const SortContext = struct {
	inv: ClientInventory,

	pub fn lessThan(ctx: @This(), a: usize, b: usize) bool {
		const itemA = ctx.inv.getItem(a);
		const itemB = ctx.inv.getItem(b);

		if(itemA == .null) return false;
		if(itemB == .null) return true;

		const itemATags = itemA.getTags().?;
		const itemBTags = itemB.getTags().?;

		for(0..@min(itemATags.len, itemBTags.len)) |i| {
			if(itemATags[i] == itemBTags[i]) continue;
			return std.mem.lessThan(u8, itemATags[i].getName(), itemBTags[i].getName());
		}
		if(itemATags.len != itemBTags.len) return itemATags.len < itemBTags.len;

		return std.mem.lessThan(u8, itemA.id().?, itemB.id().?);
	}

	pub fn swap(ctx: @This(), a: usize, b: usize) void {
		main.sync.client.executeCommand(.{.depositOrSwap = .{
			.dest = .{.inv = ctx.inv.super, .slot = @intCast(a)}, 
			.source = .{.inv = ctx.inv.super, .slot = @intCast(b)},
		}});
	}
};

Crepestrom · 2026-06-09T21:16:48Z

also fixes #3195

IntegratedQuantum

About the UI: An icon like this is hard to understand, there is no such thing as a universal sorting icon, and a funnel is often used for filtering.
I think Terraria has a simple and intuitive UI for this:

Crepestrom · 2026-06-11T16:04:24Z

I still think that std.sort is probably way nicer and easier. Here is an example (not tested) based on your current code (but of course not completly mapped and I with an added getTags method for the Item struct):

pub fn sortItems(source: ClientInventory, ignoredSlotCount: usize) void {
	compressItems(source);
	const ctx: SortContext = .{.inv = source};
	std.sort.insertionContext(ignoredSlotCount, source.super._items.len, ctx);
}

const SortContext = struct {
	inv: ClientInventory,

	pub fn lessThan(ctx: @This(), a: usize, b: usize) bool {
		const itemA = ctx.inv.getItem(a);
		const itemB = ctx.inv.getItem(b);

		if(itemA == .null) return false;
		if(itemB == .null) return true;

		const itemATags = itemA.getTags().?;
		const itemBTags = itemB.getTags().?;

		for(0..@min(itemATags.len, itemBTags.len)) |i| {
			if(itemATags[i] == itemBTags[i]) continue;
			return std.mem.lessThan(u8, itemATags[i].getName(), itemBTags[i].getName());
		}
		if(itemATags.len != itemBTags.len) return itemATags.len < itemBTags.len;

		return std.mem.lessThan(u8, itemA.id().?, itemB.id().?);
	}

	pub fn swap(ctx: @This(), a: usize, b: usize) void {
		main.sync.client.executeCommand(.{.depositOrSwap = .{
			.dest = .{.inv = ctx.inv.super, .slot = @intCast(a)}, 
			.source = .{.inv = ctx.inv.super, .slot = @intCast(b)},
		}});
	}
};

I realized how to turn tags into numbers so I’m gonna try this

Crepestrom · 2026-06-11T21:57:30Z

is ready for rereview

Wunka

Nice. Now the code looks much better

IntegratedQuantum

Before my next review please figure out how to get leak checking working. It still leaks.
To use leak checking you need to build in debug mode, test your feature, and then you need to exit the game normally (do not kill it through Ctrl+C or whatever), then you will see the leaks in the terminal output.
Please ask someone for help if you can't figure it out on your own. But we cannot have you as a contributor if every time I review I need to run the leak check for you.

Crepestrom · 2026-07-22T17:14:31Z

ok what allocator is leaking?
ill see if i can fix that check

du82 · 2026-07-23T18:16:39Z

I don't think the sort button should be that big. The UI / UX is pretty bad on it. Maybe it should live in the toolbar as a small filter icon

Wunka · 2026-07-24T01:10:41Z

I don't think the sort button should be that big. The UI / UX is pretty bad on it. Maybe it should live in the toolbar as a small filter icon

Are you looking at the old pictures?

IntegratedQuantum

Not sure how you missed it, but it's still leaking.

Also stack merging doesn't work properly:

when clicking the button becomes

IntegratedQuantum · 2026-07-25T17:18:26Z

 	const index = items.BaseItemIndex.fromId(stringId).?;
 	const item = &items.itemList[@intFromEnum(index)];
 	item.block = block;
+	const combinedTags = std.mem.concat(main.worldArena.allocator, Tag, &.{blocks.parseBlock(stringId).tags(), item.tags}) catch unreachable;


I'm still not sure if we want this.
Also it's not needed now that sorting no longer uses tags

IntegratedQuantum · 2026-07-25T17:18:32Z

 		}
 	}
+
+	pub fn getTags(self: Item) []const Tag {


IntegratedQuantum · 2026-07-25T17:19:47Z

+			return std.mem.lessThan(u8, itemA.id().?, itemB.id().?);
+		}
+
+		pub fn swap(ctx: *@This(), a: usize, b: usize) void {


IntegratedQuantum · 2026-07-25T17:20:40Z

+		var SortList = main.ListManaged(usize).init(main.stackAllocator);
+		var intermediaryList = main.ListManaged(usize).init(main.stackAllocator);
+		defer SortList.deinit();
+		defer intermediaryList.deinit();


defer should always be directly after the line that does the init

IntegratedQuantum · 2026-07-25T17:20:48Z

+	pub fn sortItems(source: ClientInventory, options: SortOptions) void {
+		compressItems(source, options);
+		const InventorySize: usize = source.super.size() - options.ignoredSlotCount;
+		var SortList = main.ListManaged(usize).init(main.stackAllocator);


Suggested change

var SortList = main.ListManaged(usize).init(main.stackAllocator);

var sortList = main.ListManaged(usize).init(main.stackAllocator);

IntegratedQuantum · 2026-08-02T17:25:26Z

I don't see any changes since my last review. Did you forget to push?

Crepestrom · 2026-08-04T18:26:37Z

testing finished

IntegratedQuantum

Full stacks should always be placed left of partial stacks of the same type, currently it behaves like this instead:

Crepestrom · 2026-08-14T04:07:08Z

funny ennough that happens because items were not being sorted by their ammounts

Full stacks should always be placed left of partial stacks of the same type, currently it behaves like this instead:

IntegratedQuantum · 2026-08-16T16:16:44Z

+			}
+			if ((ctx.inv.getAmount(a) > ctx.inv.getAmount(b)) and std.mem.eql(u8, itemA.id().?, itemB.id().?)) return true;
+
+			return std.mem.lessThan(u8, itemA.id().?, itemB.id().?);


They should still sort by id first (you did accound for that with items, but not tools).

IntegratedQuantum · 2026-08-16T16:17:26Z

 		.disabled = options.disabled,
+		.hideBackground = options.hideBackground,
 	};
+	if (self.hideBackground) self.child.mutSize().* = self.size;


Same here #3194 (comment)

IntegratedQuantum · 2026-08-16T16:19:07Z

+			if ((itemA == .proceduralItem) and (itemB != .proceduralItem)) return true;
+			if ((itemA == .proceduralItem) and (itemB == .proceduralItem)) {
+				const itemADurabilityPercent: f32 = @as(f32, @floatFromInt(itemA.proceduralItem.durability))/itemA.proceduralItem.getProperty(.maxDurability);
+				const itemBDurabilityPercent: f32 = @as(f32, @floatFromInt(itemB.proceduralItem.durability))/itemB.proceduralItem.getProperty(.maxDurability);


I'm not sure if this is reliable.
Does the floating point spec guarantee that x/x is always exactly 1?

does it matter?
at the point that happens the durabilty will be close enough anyway

If it doesn't matter, then why do you have a separate sort criterion afterwards?

Also I think it does matter at tools with full durability (which is why I'm asking whether they are guaranteed to be exactly 1).

Crepestrom added 3 commits June 8, 2026 08:58

added button

7a91563

Update Inventory.zig

1bc6472

hmm baseitems dont work

bb2ff56

Crepestrom mentioned this pull request Jun 8, 2026

Sort Button #3184

Closed

Basic Sorting works

96e1ccd

Wunka suggested changes Jun 9, 2026

View reviewed changes

Crepestrom added 3 commits June 9, 2026 13:50

Sorting Finish

9cc3310

formatting

befe81c

sort icon

cc53269

Crepestrom marked this pull request as ready for review June 9, 2026 21:16

Update sort_icon.png

88510ea

Wunka added this to PRs to review Jun 10, 2026

Wunka moved this to Low Priority in PRs to review Jun 10, 2026

IntegratedQuantum requested changes Jun 11, 2026

View reviewed changes

Comment thread src/Inventory.zig

Comment thread src/Inventory.zig Outdated

Comment thread src/Inventory.zig Outdated

Comment thread src/Inventory.zig Outdated

IntegratedQuantum moved this from Low Priority to In review in PRs to review Jun 11, 2026

Crepestrom added 4 commits June 11, 2026 14:38

updated

2c6e489

formatting

d505cba

formatting 2

4721852

formatting 3

d5f8e9a

Crepestrom requested a review from IntegratedQuantum June 11, 2026 21:57

Wunka suggested changes Jun 12, 2026

View reviewed changes

Comment thread src/Inventory.zig Outdated

Wunka suggested changes Jun 12, 2026

View reviewed changes

Comment thread src/Inventory.zig Outdated

Comment thread src/Inventory.zig Outdated

Crepestrom added 2 commits June 12, 2026 12:48

durability procedural tool check

1fa83ee

formatting

8e5d0e1

IntegratedQuantum moved this from In review to WIP/not ready for review in PRs to review Jun 12, 2026

Update sort_icon.png

fe8e1bb

Crepestrom changed the title ~~Sort button 2~~ Sort button attempt 2 Jun 13, 2026

Update Inventory.zig

71250e0

IntegratedQuantum requested changes Jul 22, 2026

View reviewed changes

Comment thread src/gui/windows/chest.zig

Crepestrom added 3 commits July 22, 2026 10:16

chest sorts chest now

f7eb5b5

Merge remote-tracking branch 'upstream/master' into Sort-Button-2

4393805

oops this isnt seen

0811db1

IntegratedQuantum requested changes Jul 25, 2026

View reviewed changes

Comment thread src/gui/components/Button.zig Outdated

Crepestrom added 3 commits July 25, 2026 09:48

fixed the leak and the item compression

7dd1ff0

use stack size instead of magic number

822fed3

changes size during intialization rather than other

7dcd839

IntegratedQuantum requested changes Jul 25, 2026

View reviewed changes

IntegratedQuantum moved this from Low Priority to In review in PRs to review Jul 30, 2026

Crepestrom requested a review from IntegratedQuantum August 2, 2026 17:09

simple changes

a2dec0f

Crepestrom added 2 commits August 4, 2026 11:29

Update Inventory.zig

1375702

Merge remote-tracking branch 'upstream/master' into Sort-Button-2

6dbdb36

IntegratedQuantum requested changes Aug 8, 2026

View reviewed changes

Comment thread src/gui/components/Button.zig Outdated

Comment thread src/assets.zig Outdated

Comment thread src/Inventory.zig Outdated

Comment thread src/Inventory.zig Outdated

Crepestrom added 4 commits August 13, 2026 19:46

remove unused things

8fa984c

private that

ebe7a89

hidden icons are slightly less jank

4ad5862

altered sorting

cd5485a

Crepestrom requested a review from IntegratedQuantum August 14, 2026 03:34

IntegratedQuantum requested changes Aug 16, 2026

View reviewed changes

Crepestrom added 2 commits August 18, 2026 13:22

moved around this stuff

2cdf0f6

fixed issues

1cd73ab

	var SortList = main.ListManaged(usize).init(main.stackAllocator);
	var sortList = main.ListManaged(usize).init(main.stackAllocator);

Uh oh!

Uh oh!

Conversation

Crepestrom commented Jun 8, 2026 • edited by IntegratedQuantum Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Wunka left a comment

Choose a reason for hiding this comment

Uh oh!

Crepestrom commented Jun 9, 2026

Uh oh!

IntegratedQuantum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Crepestrom commented Jun 11, 2026

Uh oh!

Crepestrom commented Jun 11, 2026

Uh oh!

Wunka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

IntegratedQuantum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Crepestrom commented Jul 22, 2026

Uh oh!

du82 commented Jul 23, 2026

Uh oh!

Wunka commented Jul 24, 2026

Uh oh!

IntegratedQuantum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IntegratedQuantum commented Aug 2, 2026

Uh oh!

Crepestrom commented Aug 4, 2026

Uh oh!

IntegratedQuantum left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Crepestrom commented Aug 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Crepestrom commented Jun 8, 2026 •

edited by IntegratedQuantum

Loading