Skip to content

Sort button 2#3194

Open
Crepestrom wants to merge 14 commits into
PixelGuys:masterfrom
Crepestrom:Sort-Button-2
Open

Sort button 2#3194
Crepestrom wants to merge 14 commits into
PixelGuys:masterfrom
Crepestrom:Sort-Button-2

Conversation

@Crepestrom

@Crepestrom Crepestrom commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

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

image

@Crepestrom Crepestrom mentioned this pull request Jun 8, 2026

@Wunka Wunka left a comment

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.

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 Crepestrom marked this pull request as ready for review June 9, 2026 21:16
@Crepestrom

Copy link
Copy Markdown
Contributor Author

also fixes #3195

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

@IntegratedQuantum IntegratedQuantum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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:

Image

Comment thread src/Inventory.zig

pub fn compressItems(source: ClientInventory) void {
for (source.super._items, 0..) |invStack, slot| {
for (source.super._items, 0..) |checkedInvStack, checkedSlot| {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the item is filled with mostly the same items, this will spawn O(n²) inventory commands. That is excessive.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok i made it a bit more effecient

Comment thread src/Inventory.zig Outdated
Comment thread src/Inventory.zig Outdated
Comment thread src/Inventory.zig Outdated
@IntegratedQuantum IntegratedQuantum moved this from Low Priority to In review in PRs to review Jun 11, 2026
@Crepestrom

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author

is ready for rereview

@Wunka Wunka left a comment

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.

Nice. Now the code looks much better

Comment thread src/Inventory.zig
Comment on lines +584 to +595
pub fn getTagsFromItem(givenItem: Item) []const Tag {
if (givenItem == .null) {
return &[_]Tag{};
} else if (givenItem == .proceduralItem) {
return givenItem.proceduralItem.type.tags();
} else if (givenItem == .baseItem) {
return givenItem.baseItem.tags();
} else {
std.log.err("getSortingTag: Could not find item class {}", .{givenItem});
return &[_]Tag{};
}
}

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 should be in the Item struct and use a switch(self) so you don't need the else at the end

Comment thread src/Inventory.zig
Comment on lines +567 to +571
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;

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 is probably not the best thing (I know it is from me but it was at that time made up on the spot). We probably need to enforce that the tags are sorted so that for example when ItemA has a,b tags and ItemB has c,a,b It gets compared with a,b and a,b,c. Either by sorting them when we load the tags or here. Or some other way.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Definitely on load.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If I do this I think it would cause read order issues if I make it check if the first tag of ItemA exists in ItemB because they could have different orders
I could fix this with forcing them to read based on the enum int value but I like what I have now because it gives control to developers on how items are catogorized and sorted

you could create tags that are used in nothing but sorting so that all flowers are sorted together under the category of cuttable

@Wunka Wunka Jun 12, 2026

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.

If I do this I think it would cause read order issues if I make it check if the first tag of ItemA exists in ItemB because they could have different orders I could fix this with forcing them to read based on the enum int value but I like what I have now because it gives control to developers on how items are catogorized and sorted

you could create tags that are used in nothing but sorting so that all flowers are sorted together under the category of cuttable

We didn't mean that. We meant that the tags should be already sorted so what you at the start said doesn't need to be done. This sorting algorithm could then also be changed by a user if they want to.
You just need to add in src/tag.zig

pub fn loadTagsFromZon(_allocator: main.heap.NeverFailingAllocator, zon: main.ZonElement) []Tag {
	const result = _allocator.alloc(Tag, zon.toSlice().len);
	for (zon.toSlice(), 0..) |tagZon, i| {
		result[i] = Tag.find(tagZon.as([]const u8) orelse blk: {
			std.log.err("Tag array field {s} has incorrect type, expected string", .{@tagName(tagZon)});
			break :blk "incorrect";
		});
	}
+	// some kind of sorting before returning the result
	return result;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don’t understand what you mean by sorting the tags

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh I see
You mean so that
C A B
B A C
C B A

all get sorted together A B C
Right?

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.

Oh I see You mean so that C A B B A C C B A

all get sorted together A B C Right?

yes exactly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No
Id rather give the developer control on sorting rather than enforce is based on the order tags were read into memory
Even if that means mistakes can make sorting look weird

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.

But the sorting of the tags is also clientside. So it can also be changed.

Comment thread src/Inventory.zig
}
if (itemATags.len != itemBTags.len) return itemATags.len < itemBTags.len;

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

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.

I think it would be nice to add here that proceduralItems are sorted by durability, when even the id is the same.

@IntegratedQuantum IntegratedQuantum moved this from In review to WIP/not ready for review in PRs to review Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: WIP/not ready for review

Development

Successfully merging this pull request may close these issues.

4 participants