Sort button attempt 2

[Crepestrom]

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]

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]

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]

is ready for rereview

[Wunka]

Nice. Now the code looks much better

[IntegratedQuantum]

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

[Crepestrom]

testing finished

[IntegratedQuantum]

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

[Crepestrom]

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]

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

[Crepestrom]

sorts by id first now

[IntegratedQuantum]

Same here #3194 (comment)

[Crepestrom]

oops forgot to remove that line

[Wunka]

on the second line the first argument is already proven to be false (because of the first line). So either do:

Suggested change

	if (std.mem.lessThan(u8, itemA.id().?, itemB.id().?)) return true;
	if (!std.mem.lessThan(u8, itemA.id().?, itemB.id().?) and !std.mem.eql(u8, itemA.id().?, itemB.id().?)) return false;
	if (std.mem.lessThan(u8, itemA.id().?, itemB.id().?)) return true;
	if (!std.mem.eql(u8, itemA.id().?, itemB.id().?)) return false;

or

Suggested change

	if (std.mem.lessThan(u8, itemA.id().?, itemB.id().?)) return true;
	if (!std.mem.lessThan(u8, itemA.id().?, itemB.id().?) and !std.mem.eql(u8, itemA.id().?, itemB.id().?)) return false;
	if (!std.mem.eql(u8, itemA.id().?, itemB.id().?)) return std.mem.lessThan(u8, itemA.id().?, itemB.id().?):

[Crepestrom]

applied suggesion

[Wunka]

this is already checked a few lines above

[Crepestrom]

oops fixed

[Crepestrom]

thanks for your help wunka

[IntegratedQuantum]

This looks wrong, if there is no background, then it should not add any border around the supplied size.

[Crepestrom]

nope getting rid of the border just makes the button smaller or the icon smaller than it needs to be

[IntegratedQuantum]

I think durability should be the last property to consider (→after dps), otherwise items with the identical recipe will not be sorted next to each other.

[Crepestrom]

ok done