Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix shop issues with category filtering #1482

Merged
merged 1 commit into from
Jan 20, 2025
Merged

Fix shop issues with category filtering #1482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions core/src/main/java/tc/oc/pgm/menu/InventoryMenu.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ public void open() {
inventory.open(getBukkit());
}

public void close() {
inventory.close(getBukkit());
}

public Player getBukkit() {
return viewer.getBukkit();
}
Expand Down
3 changes: 1 addition & 2 deletions core/src/main/java/tc/oc/pgm/shops/Shop.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import com.google.common.collect.ImmutableList;
import java.util.List;
import java.util.stream.Collectors;
import tc.oc.pgm.api.player.MatchPlayer;
import tc.oc.pgm.features.SelfIdentifyingFeatureDefinition;
import tc.oc.pgm.shops.menu.Category;
Expand Down Expand Up @@ -33,7 +32,7 @@ public List<Category> getCategories() {
public List<Category> getVisibleCategories(MatchPlayer player) {
return categories.stream()
.filter(c -> c.getFilter().query(player).isAllowed())
.collect(Collectors.toList());
.toList();
}

public void purchase(Icon icon, MatchPlayer buyer) {
Expand Down
197 changes: 112 additions & 85 deletions core/src/main/java/tc/oc/pgm/shops/menu/ShopMenu.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import fr.minuskube.inv.content.InventoryContents;
import fr.minuskube.inv.content.Pagination;
import fr.minuskube.inv.content.SlotIterator;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
import net.kyori.adventure.text.Component;
Expand Down Expand Up @@ -37,37 +38,34 @@ public class ShopMenu extends InventoryMenu {
private static final String SYMBOL_SUFFICIENT = "\u2714 "; // ✔
private static final String SYMBOL_INSUFFICIENT = "\u2715 "; // ✕

private Shop shop;
private final Shop shop;
private final List<CategoryItem> allCategories;
private Category category;
private ClickableItem[] categories;
private int highlight;

public ShopMenu(Shop shop, MatchPlayer viewer) {
super(text(colorize(shop.getName())), 6, viewer, null);
this.shop = shop;
this.allCategories = shop.getCategories().stream().map(CategoryItem::new).toList();

if (shop.getVisibleCategories(viewer).isEmpty()) {
getViewer().sendWarning(translatable("shop.category.empty"));
return;
}

this.category = shop.getVisibleCategories(viewer).get(0);
this.categories = getCategoryItems();
updateCategories();
if (category == null) return;
this.highlight = getStartingIndex();
open();
}

@Override
public void init(Player player, InventoryContents contents) {
render(player, contents);
render(contents);
}

@Override
public void update(Player player, InventoryContents contents) {
render(player, contents);
render(contents);
}

private void render(Player player, InventoryContents contents) {
private void render(InventoryContents contents) {
this.renderHeader(contents);
this.renderIcons(contents);
}
Expand All @@ -81,6 +79,10 @@ private boolean isPaginated() {
}

private void renderHeader(InventoryContents contents) {
if (this.updateCategories()) {
contents.fillRow(0, null);
}

if (isPaginated()) {
Pagination page = contents.pagination();
page.setItems(categories);
Expand All @@ -106,7 +108,7 @@ private void renderHeader(InventoryContents contents) {

// Menu divider & highlight
contents.fillRow(1, getMenuSeperator(DyeColor.GRAY));
contents.set(1, highlight, getMenuSeperator(DyeColor.GREEN));
contents.set(1, Math.max(0, Math.min(8, highlight)), getMenuSeperator(DyeColor.GREEN));
}

private void renderIcons(InventoryContents contents) {
Expand All @@ -123,7 +125,7 @@ private void renderIcons(InventoryContents contents) {

int row = 2;
int col = 1;
for (Icon icon : getCategory().getVisibleIcons(getViewer())) {
for (Icon icon : icons) {
contents.set(row, col, getPurchasableItem(icon));
col++;
if (col > 7) {
Expand All @@ -142,35 +144,53 @@ private void setCategory(Category category, int slot) {
this.highlight = slot;
}

private ClickableItem[] getCategoryItems() {
ClickableItem[] items = new ClickableItem[shop.getVisibleCategories(getViewer()).size()];
for (int i = 0; i < shop.getCategories().size(); i++) {
items[i] = getCategoryItem(shop.getCategories().get(i));
private boolean updateCategories() {
boolean anyChanged = false;
for (CategoryItem category : allCategories) {
if (category.changed()) anyChanged = true;
}
if (categories != null && !anyChanged) return false;
Comment on lines +148 to +152
Copy link
Member

Choose a reason for hiding this comment

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

nit: Assuming we don't need to run category.changed() for every category every time:

Suggested change
boolean anyChanged = false;
for (CategoryItem category : allCategories) {
if (category.changed()) anyChanged = true;
}
if (categories != null && !anyChanged) return false;
if (categories != null && allCategories.stream().noneMatch((CategoryItem::changed)))
return false;

Copy link
Member Author

@Pablete1234 Pablete1234 Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We do need to check all of them, since that will update the active flag on the category.

If we do a noneMatch as soon as one is changed, all following ones will not get their filter updated.

For most cases, you'll check changed on all categories and none will have changed, in which case that's where it ends. In the odd case that one or more have changed, you will need to re-build the lists etc


List<ClickableItem> newCategories = new ArrayList<>(allCategories.size());
Category first = null;
int curr = 0, active = -1;
for (CategoryItem category : allCategories) {
if (!category.isActive()) continue;
if (first == null) first = category.category;
if (category.category == this.category) active = curr;

newCategories.add(category.item);
curr++;
}
this.categories = newCategories.toArray(ClickableItem[]::new);

if (newCategories.isEmpty()) {
getViewer().sendWarning(translatable("shop.category.empty"));
close();
return true;
}
return items;
}

private ClickableItem getCategoryItem(Category category) {
return ClickableItem.of(category.getCategoryIcon(), c -> setCategory(category, c.getSlot()));
// Update selected category if it became inactive
if (active != -1) setCategory(category, getStartingIndex() + active);
else setCategory(first, getStartingIndex());
return true;
}

private ClickableItem getMenuSeperator(DyeColor color) {
return ClickableItem.empty(
new ItemBuilder()
.material(Materials.STAINED_GLASS_PANE)
.color(color)
.name(" ")
.flags(ItemFlag.values())
.build());
return ClickableItem.empty(new ItemBuilder()
.material(Materials.STAINED_GLASS_PANE)
.color(color)
.name(" ")
.flags(ItemFlag.values())
.build());
}

private ClickableItem getNoItemsItem() {
return ClickableItem.empty(
new ItemBuilder()
.material(Material.BARRIER)
.name(ChatColor.RED + TextTranslations.translate("shop.item.empty", getBukkit()))
.flags(ItemFlag.values())
.build());
return ClickableItem.empty(new ItemBuilder()
.material(Material.BARRIER)
.name(getBukkit(), translatable("shop.item.empty", NamedTextColor.RED))
.flags(ItemFlag.values())
.build());
}

private ClickableItem getPurchasableItem(Icon icon) {
Expand All @@ -183,48 +203,39 @@ private ClickableItem getPurchasableItem(Icon icon) {
// Free item
price.add(translatable("shop.lore.free", NamedTextColor.GREEN));
} else {
price =
icon.getPayments().stream()
.map(
p -> {
boolean hasPayment = p.hasPayment(getViewer().getInventory());

Component currencyName =
p.getItem() != null
? text(p.getItem().getItemMeta().getDisplayName())
: text(getMaterial(p.getCurrency()))
.color(TextFormatter.convert(p.getColor()));

Component prefix =
icon.getPayments().size() == 1
? null
: text(
hasPayment ? SYMBOL_SUFFICIENT : SYMBOL_INSUFFICIENT,
hasPayment ? NamedTextColor.GREEN : NamedTextColor.DARK_RED);

TextComponent.Builder priceComponent = text();

if (prefix != null) {
priceComponent.append(prefix);
}

priceComponent
.append(
text(
p.getPrice(),
hasPayment ? NamedTextColor.GREEN : NamedTextColor.RED))
.append(space())
.append(currencyName);

return priceComponent.build();
})
.collect(Collectors.toList());
price = icon.getPayments().stream()
.map(p -> {
boolean hasPayment = p.hasPayment(getViewer().getInventory());

Component currencyName = p.getItem() != null
? text(p.getItem().getItemMeta().getDisplayName())
: text(getMaterial(p.getCurrency())).color(TextFormatter.convert(p.getColor()));

Component prefix = icon.getPayments().size() == 1
? null
: text(
hasPayment ? SYMBOL_SUFFICIENT : SYMBOL_INSUFFICIENT,
hasPayment ? NamedTextColor.GREEN : NamedTextColor.DARK_RED);

TextComponent.Builder priceComponent = text();

if (prefix != null) {
priceComponent.append(prefix);
}

priceComponent
.append(text(p.getPrice(), hasPayment ? NamedTextColor.GREEN : NamedTextColor.RED))
.append(space())
.append(currencyName);

return priceComponent.build();
})
.collect(Collectors.toList());
}

TextComponent.Builder cost =
text()
.append(translatable("shop.lore.cost", NamedTextColor.GRAY))
.append(text(": ", NamedTextColor.DARK_GRAY));
TextComponent.Builder cost = text()
.append(translatable("shop.lore.cost", NamedTextColor.GRAY))
.append(text(": ", NamedTextColor.DARK_GRAY));

// Display free or single item price on the same line as cost
if (price.size() == 1) {
Expand Down Expand Up @@ -260,27 +271,43 @@ private ClickableItem getPurchasableItem(Icon icon) {
}

private ClickableItem getPageItem(Player player, int page, boolean next) {
return ClickableItem.of(
getPageIcon(page, next),
c -> {
getInventory().open(player, page);
this.highlight = next ? 0 : 8;
});
return ClickableItem.of(getPageIcon(page, next), c -> {
getInventory().open(player, page);
this.highlight += next ? -9 : 9;
});
}

private final ItemStack getPageIcon(int page, boolean next) {
private ItemStack getPageIcon(int page, boolean next) {
return new ItemBuilder()
.material(Material.ARROW)
.amount(page)
.name(
ChatColor.YELLOW
+ TextTranslations.translate(
"menu.page." + (next ? "next" : "previous"), getBukkit()))
.name(ChatColor.YELLOW
+ TextTranslations.translate("menu.page." + (next ? "next" : "previous"), getBukkit()))
.flags(ItemFlag.values())
.build();
}

private String getMaterial(Material material) {
return WordUtils.capitalizeFully(material.name().toLowerCase().replace("_", " "));
}

private class CategoryItem {
private final Category category;
private final ClickableItem item;
private boolean active = false;

public CategoryItem(Category category) {
this.category = category;
this.item =
ClickableItem.of(category.getCategoryIcon(), c -> setCategory(category, c.getSlot()));
}

boolean isActive() {
return active;
}

boolean changed() {
return active != (active = category.getFilter().query(getViewer()).isAllowed());
}
}
}
Loading