Skip to content

fix: on-sell commands not executing for COMMAND delivery items #67

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

Merged
ez-plugins merged 2 commits into from
May 18, 2026
Merged

fix: on-sell commands not executing for COMMAND delivery items #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d2b054
fix: on-sell commands not executing for COMMAND delivery items
ez-plugins May 18, 2026
93c12d2
chore: add Codecov, bump version to 2.5.5, update changelog
ez-plugins May 18, 2026
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
14 changes: 14 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ jobs:
- name: Run unit tests
run: mvn -B -Dpaper.version=${{ matrix.paper-version }} -Dmockbukkit.artifactId=${{ matrix.mockbukkit-artifactId }} -Dmockbukkit.version=${{ matrix.mockbukkit-version }} test

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v5
with:
files: target/site/jacoco/jacoco.xml
flags: unit-tests
fail_ci_if_error: false

feature-tests:
runs-on: ubuntu-latest
needs: unit-tests
Expand Down Expand Up @@ -65,3 +72,10 @@ jobs:

- name: Run feature tests
run: mvn -B -Dpaper.version=${{ matrix.paper-version }} -Dmockbukkit.artifactId=${{ matrix.mockbukkit-artifactId }} -Dmockbukkit.version=${{ matrix.mockbukkit-version }} -Pfeature-tests -Dtest=*FeatureTest test

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v5
with:
files: target/site/jacoco/jacoco.xml
flags: feature-tests
fail_ci_if_error: false
9 changes: 9 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [2.5.5] - 2026-05-18

### Fixed
- **`on-sell` commands not executing for `item-type: COMMAND` items** — `sell()` no longer checks or removes physical items from the player's inventory when the item's delivery type is `COMMAND`. Previously the transaction exited early with "insufficient items" because the player had no material to hand over, preventing sell commands from running at all.
- **`on-sell execute-as` overridden by `on-buy execute-as`** — `ShopPricingManager` now tracks `execute-as` independently for the `on-buy` and `on-sell` blocks. Previously a single shared flag meant that setting `on-buy: execute-as: player` would silently override `on-sell: execute-as: console`, causing sell commands to run as the player instead of the console.

### Added
- **Code coverage reporting** — JaCoCo is now configured in the Maven build (`jacoco-maven-plugin 0.8.12`). Coverage reports (`jacoco.xml`) are generated on every `mvn test` run and uploaded to Codecov by the CI workflow for both unit-test and feature-test jobs.

## [2.5.4] - 2026-05-14

### Fixed
Expand Down
24 changes: 22 additions & 2 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>com.skyblockexp</groupId>
<artifactId>ezshops</artifactId>
<version>2.5.4</version>
<version>2.5.5</version>
<name>EzShops Plugin</name>
<description>Standalone plugin providing the Skyblock shop command and sign shops.</description>
<packaging>jar</packaging>
Expand Down Expand Up @@ -219,9 +219,29 @@
<version>3.0.0-M8</version>
<configuration>
<useModulePath>false</useModulePath>
<argLine>-Dnet.bytebuddy.experimental=true</argLine>
<argLine>@{argLine} -Dnet.bytebuddy.experimental=true</argLine>
</configuration>
</plugin>
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>0.8.12</version>
<executions>
<execution>
<id>jacoco-prepare-agent</id>
<goals>
<goal>prepare-agent</goal>
</goals>
</execution>
<execution>
<id>jacoco-report</id>
<phase>test</phase>
<goals>
<goal>report</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>

Expand Down
14 changes: 14 additions & 0 deletions src/main/java/com/skyblockexp/ezshops/shop/ShopMenuLayout.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ public static final class Item {
private final java.util.List<String> buyCommands;
private final java.util.List<String> sellCommands;
private final Boolean commandsRunAsConsole;
private final Boolean sellCommandsRunAsConsole;
private final int requiredIslandLevel;
private final ShopPriceType priceType;
private final String priceId;
Expand All @@ -198,6 +199,14 @@ public Item(String id, Material material, ItemDecoration display, int slot, int
ShopPrice price, ItemType type, EntityType spawnerEntity,
Map<Enchantment, Integer> enchantments, int requiredIslandLevel, ShopPriceType priceType,
java.util.List<String> buyCommands, java.util.List<String> sellCommands, Boolean commandsRunAsConsole, String priceId, DeliveryType delivery) {
this(id, material, display, slot, page, amount, bulkAmount, price, type, spawnerEntity, enchantments, requiredIslandLevel, priceType, buyCommands, sellCommands, commandsRunAsConsole, null, priceId, delivery);
}

public Item(String id, Material material, ItemDecoration display, int slot, int page, int amount, int bulkAmount,
ShopPrice price, ItemType type, EntityType spawnerEntity,
Map<Enchantment, Integer> enchantments, int requiredIslandLevel, ShopPriceType priceType,
java.util.List<String> buyCommands, java.util.List<String> sellCommands, Boolean commandsRunAsConsole,
Boolean sellCommandsRunAsConsole, String priceId, DeliveryType delivery) {
this.id = Objects.requireNonNull(id, "id");
this.page = Math.max(0, page);
this.material = Objects.requireNonNull(material, "material");
Expand All @@ -212,6 +221,7 @@ public Item(String id, Material material, ItemDecoration display, int slot, int
this.buyCommands = buyCommands == null ? List.of() : List.copyOf(buyCommands);
this.sellCommands = sellCommands == null ? List.of() : List.copyOf(sellCommands);
this.commandsRunAsConsole = commandsRunAsConsole == null ? Boolean.TRUE : commandsRunAsConsole;
this.sellCommandsRunAsConsole = sellCommandsRunAsConsole;
this.requiredIslandLevel = Math.max(0, requiredIslandLevel);
this.priceType = priceType == null ? ShopPriceType.STATIC : priceType;
this.priceId = priceId == null ? material.name() : priceId;
Expand Down Expand Up @@ -286,6 +296,10 @@ public Boolean commandsRunAsConsole() {
return commandsRunAsConsole;
}

public Boolean sellCommandsRunAsConsole() {
return sellCommandsRunAsConsole != null ? sellCommandsRunAsConsole : commandsRunAsConsole;
}

public DeliveryType delivery() {
return delivery;
}
Expand Down
19 changes: 11 additions & 8 deletions src/main/java/com/skyblockexp/ezshops/shop/ShopPricingManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,8 @@ private ShopMenuLayout.Item parseItem(String contextPrefix, String itemId, Confi
// parse optional command hooks
java.util.List<String> buyCommands = section.getStringList("buy-commands");
java.util.List<String> sellCommands = section.getStringList("sell-commands");
Boolean commandsRunAsConsole = null;
Boolean buyCommandsRunAsConsole = null;
Boolean sellCommandsRunAsConsole = null;
// support 'on-buy'/'on-sell' blocks with execute-as and commands
if (section.isConfigurationSection("on-buy")) {
org.bukkit.configuration.ConfigurationSection onBuy = section.getConfigurationSection("on-buy");
Expand All @@ -751,26 +752,28 @@ private ShopMenuLayout.Item parseItem(String contextPrefix, String itemId, Confi
buyCommands = onBuy.getStringList("commands");
}
String exec = onBuy.getString("execute-as", null);
if (exec != null && exec.equalsIgnoreCase("player")) {
commandsRunAsConsole = Boolean.FALSE;
if (exec != null) {
buyCommandsRunAsConsole = !exec.equalsIgnoreCase("player");
}
}
}
if (section.isConfigurationSection("on-sell")) {
org.bukkit.configuration.ConfigurationSection onSell = section.getConfigurationSection("on-sell");
if (onSell != null && onSell.isSet("commands")) {
sellCommands = onSell.getStringList("commands");
if (onSell != null) {
if (onSell.isSet("commands")) {
sellCommands = onSell.getStringList("commands");
}
String exec = onSell.getString("execute-as", null);
if (exec != null && exec.equalsIgnoreCase("player")) {
commandsRunAsConsole = Boolean.FALSE;
if (exec != null) {
sellCommandsRunAsConsole = !exec.equalsIgnoreCase("player");
}
}
}

DeliveryType delivery = DeliveryType.fromConfig(section.getString("item-type"));
return new ShopMenuLayout.Item(itemId, material, decoration, slot, page, amount, bulkAmount, price, type,
spawnerEntity, enchantments, requiredIslandLevel, priceType, buyCommands, sellCommands,
commandsRunAsConsole, configuredPriceId, delivery);
buyCommandsRunAsConsole, sellCommandsRunAsConsole, configuredPriceId, delivery);
}

private Map<String, Map<String, Object>> readItemData(ConfigurationSection section) {
Expand Down
18 changes: 11 additions & 7 deletions src/main/java/com/skyblockexp/ezshops/shop/ShopTransactionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,16 +386,20 @@ public ShopTransactionResult sell(Player player, com.skyblockexp.ezshops.shop.Sh
return ShopTransactionResult.failure(errorMessages.invalidSellPrice());
}

int sellableAmount = countMaterial(player, item.material());
if (sellableAmount < amount) {
return ShopTransactionResult.failure(errorMessages.insufficientItems());
if (item.delivery() != DeliveryType.COMMAND) {
int sellableAmount = countMaterial(player, item.material());
if (sellableAmount < amount) {
return ShopTransactionResult.failure(errorMessages.insufficientItems());
}
removeItems(player, item.material(), amount);
}

removeItems(player, item.material(), amount);
EconomyResponse response = economy.depositPlayer(player, totalGain);
if (!response.transactionSuccess()) {
List<ItemStack> leftovers = giveItems(player, item.material(), amount);
handleLeftoverItems(player, leftovers);
if (item.delivery() != DeliveryType.COMMAND) {
List<ItemStack> leftovers = giveItems(player, item.material(), amount);
handleLeftoverItems(player, leftovers);
}
return ShopTransactionResult.failure(errorMessages.transactionFailed(response.errorMessage));
}

Expand All @@ -411,7 +415,7 @@ public ShopTransactionResult sell(Player player, com.skyblockexp.ezshops.shop.Sh
tokens.put("display", item.display() != null ? item.display().displayName() : "");
tokens.put("price", item.price() != null ? formatCurrency(item.price().sellPrice()) : "");
tokens.put("total", formatCurrency(totalGain));
hookService.executeHooks(player, item.sellCommands(), item.commandsRunAsConsole() == null ? true : item.commandsRunAsConsole(), tokens);
hookService.executeHooks(player, item.sellCommands(), item.sellCommandsRunAsConsole() == null ? true : item.sellCommandsRunAsConsole(), tokens);
org.bukkit.Bukkit.getPluginManager().callEvent(new com.skyblockexp.ezshops.event.ShopSaleEvent(player, new ItemStack(item.material(), Math.max(1, amount)), amount, totalGain));
}
return result;
Expand Down
50 changes: 50 additions & 0 deletions src/test/java/com/skyblockexp/ezshops/shop/ShopItemTypeFeatureTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,56 @@ void buy_item_type_command_runs_hooks_but_no_item_given() {
verify(hook).executeHooks(eq(player), eq(buyCommands), eq(false), tokensCaptor.capture());
}

@Test
void sell_command_delivery_succeeds_without_physical_items_and_runs_hooks() {
loadProviderPlugin(Mockito.mock(Economy.class));
var plugin = loadPlugin(com.skyblockexp.ezshops.EzShopsPlugin.class);

ShopPricingManager pricingManager = Mockito.mock(ShopPricingManager.class);
Economy econ = Mockito.mock(Economy.class);

ShopPrice price = new ShopPrice(10.0, 5.0);
when(pricingManager.getPrice(eq("DIAMOND"))).thenReturn(Optional.of(price));
when(pricingManager.estimateBulkTotal(eq("DIAMOND"), eq(1), any())).thenReturn(5.0);

when(econ.depositPlayer((org.bukkit.OfflinePlayer) any(), anyDouble()))
.thenReturn(new EconomyResponse(0.0, 5.0, EconomyResponse.ResponseType.SUCCESS, "ok"));

ShopTransactionService svc = new ShopTransactionService(pricingManager, econ,
com.skyblockexp.ezshops.config.ShopMessageConfiguration.load(plugin).transactions());

TransactionHookService hook = Mockito.mock(TransactionHookService.class);
svc.setTransactionHookService(hook);

// Player has NO DIAMOND in their inventory — a COMMAND sell should not require it
Player player = server.addPlayer("seller_cmd");
player.addAttachment(plugin, ShopTransactionService.PERMISSION_SELL, true);

ShopMenuLayout.ItemDecoration decoration =
new ShopMenuLayout.ItemDecoration(Material.DIAMOND, 1, "", List.of());
List<String> sellCommands = List.of("give {player} diamond 1");
ShopMenuLayout.Item item = new ShopMenuLayout.Item("diamond_command_sell", Material.DIAMOND, decoration,
0, 0, 1, 1, price, ShopMenuLayout.ItemType.MATERIAL, null, Map.of(), 0,
ShopPriceType.STATIC, List.of(), sellCommands, Boolean.TRUE, null, DeliveryType.COMMAND);

ShopTransactionResult result = svc.sell(player, item, 1);

assertTrue(result.success(), "COMMAND-delivery sell should succeed even without physical items: " + result.message());

// Economy should have deposited the sell price
verify(econ).depositPlayer((org.bukkit.OfflinePlayer) any(), eq(5.0));

// Inventory must be untouched
int remaining = player.getInventory().all(Material.DIAMOND).values().stream()
.mapToInt(ItemStack::getAmount).sum();
assertEquals(0, remaining, "COMMAND-delivery sell must not remove items from the player's inventory");

// Sell hooks must still run
ArgumentCaptor<Map> tokensCaptor = ArgumentCaptor.forClass(Map.class);
verify(hook).executeHooks(eq(player), eq(sellCommands), eq(Boolean.TRUE), tokensCaptor.capture());
assertEquals("1", tokensCaptor.getValue().get("amount"));
}

@Test
void buy_item_type_none_charges_but_no_item_and_no_hooks() {
loadProviderPlugin(Mockito.mock(Economy.class));
Expand Down
Loading
Loading