Missing Inventory Space Validation Leading to Item Loss #2

New Issue

saulteafarmer · 2025-05-26T16:48:04Z

saulteafarmer commented

2025-05-26 16:48:04 +00:00

The Orangemart plugin fails to check available inventory space before giving currency items to players, potentially causing item loss when player inventories are full. Items are given using basePlayer.GiveItem() without validation, which can fail silently.

Root Cause Analysis

Affected Methods

1. RewardPlayer() Method (Lines ~440-465)

private void RewardPlayer(IPlayer player, int amount)
{
    player.Reply($"You have successfully purchased {amount} {currencyName}!");

    var basePlayer = player.Object as BasePlayer;
    if (basePlayer != null)
    {
        var currencyItem = ItemManager.CreateByItemID(currencyItemID, amount);
        if (currencyItem != null)
        {
            if (currencySkinID > 0)
            {
                currencyItem.skin = currencySkinID;
            }
            basePlayer.GiveItem(currencyItem); // ← NO INVENTORY CHECK
            Puts($"Gave {amount} {currencyName} (skinID: {currencySkinID}) to player {basePlayer.UserIDString}.");
        }
    }
}

2. ReturnCurrency() Method (Lines ~480-495)

private void ReturnCurrency(BasePlayer player, int amount)
{
    var returnedCurrency = ItemManager.CreateByItemID(currencyItemID, amount);
    if (returnedCurrency != null)
    {
        if (currencySkinID > 0)
        {
            returnedCurrency.skin = currencySkinID;
        }
        returnedCurrency.MoveToContainer(player.inventory.containerMain); // ← NO SPACE CHECK
    }
}

Issue Details

Problem 1: Silent Item Loss

basePlayer.GiveItem() returns false if inventory is full
Return value is ignored, causing silent failures
Player pays for items but doesn't receive them
Money is lost, items are destroyed

Problem 2: Partial Stack Issues

Large currency amounts may only partially fit in inventory
No handling for cases where only some items can be given
Remaining items are lost without notification

Problem 3: No Fallback Mechanism

No attempt to drop items on ground if inventory full
No notification to player about inventory space issues
No retry mechanism or delayed delivery

Impact

💸 Financial Loss: Players pay real Bitcoin but lose items
😠 Player Frustration: Silent failures with no explanation
🐛 Support Issues: Players reporting "I paid but got nothing"
💰 Refund Complications: Difficult to track and resolve lost items
🎮 Poor UX: No guidance about inventory requirements

Steps to Reproduce

Fill player inventory completely (all slots occupied)
Player uses /buyblood 1000 command (large amount)
Player pays Lightning invoice successfully
Expected: Player receives currency items
Actual: Payment succeeds, items are lost, player gets nothing

Proposed Fix

Solution 1: Pre-Purchase Inventory Validation

Add inventory checks before creating invoices:

private bool HasInventorySpace(BasePlayer player, int itemID, int amount)
{
    var testItem = ItemManager.CreateByItemID(itemID, amount);
    if (testItem == null) return false;
    
    // Check if player can fit the item
    bool canFit = player.inventory.CanAcceptItem(testItem, -1) != null;
    testItem.Remove(); // Clean up test item
    
    return canFit;
}

private void CmdBuyCurrency(IPlayer player, string command, string[] args)
{
    // ... existing validation ...
    
    var basePlayer = player.Object as BasePlayer;
    if (basePlayer == null)
    {
        player.Reply(Lang("FailedToFindBasePlayer", player.Id));
        return;
    }
    
    // NEW: Check inventory space before creating invoice
    if (!HasInventorySpace(basePlayer, currencyItemID, amount))
    {
        player.Reply(Lang("InsufficientInventorySpace", player.Id, amount, currencyName));
        return;
    }
    
    // ... continue with invoice creation ...
}

Solution 2: Smart Item Distribution with Fallback

Improve item giving with ground drop fallback:

private void RewardPlayer(IPlayer player, int amount)
{
    var basePlayer = player.Object as BasePlayer;
    if (basePlayer == null)
    {
        PrintError($"Failed to find base player object for player {player.Id}.");
        return;
    }

    var currencyItem = ItemManager.CreateByItemID(currencyItemID, amount);
    if (currencyItem == null)
    {
        PrintError($"Failed to create {currencyName} item for player {basePlayer.UserIDString}.");
        return;
    }

    if (currencySkinID > 0)
    {
        currencyItem.skin = currencySkinID;
    }

    // Try to give item to inventory first
    if (basePlayer.GiveItem(currencyItem))
    {
        player.Reply(Lang("PurchaseSuccess", player.Id, amount, currencyName));
        Puts($"Gave {amount} {currencyName} to player {basePlayer.UserIDString}.");
    }
    else
    {
        // Fallback: Drop item on ground near player
        currencyItem.Drop(basePlayer.transform.position + Vector3.up, Vector3.zero);
        player.Reply(Lang("PurchaseSuccessDropped", player.Id, amount, currencyName));
        Puts($"Dropped {amount} {currencyName} for player {basePlayer.UserIDString} (inventory full).");
    }
}

Solution 3: Partial Stack Handling

Handle cases where only some items fit:

private void GiveItemsSafely(BasePlayer player, int itemID, int amount, ulong skinID = 0)
{
    int remaining = amount;
    int maxStackSize = ItemManager.FindItemDefinition(itemID).stackable;
    
    while (remaining > 0)
    {
        int stackAmount = Math.Min(remaining, maxStackSize);
        var item = ItemManager.CreateByItemID(itemID, stackAmount);
        
        if (item == null) break;
        
        if (skinID > 0) item.skin = skinID;
        
        if (player.GiveItem(item))
        {
            remaining -= stackAmount;
        }
        else
        {
            // Drop remaining items on ground
            item.Drop(player.transform.position + Vector3.up, Vector3.zero);
            break;
        }
    }
    
    if (remaining < amount)
    {
        int given = amount - remaining;
        player.ChatMessage($"Received {given}/{amount} {currencyName}. Check ground for remaining items.");
    }
}

Required Language Additions

Add these messages to LoadDefaultMessages():

["InsufficientInventorySpace"] = "You need at least {0} free inventory slots to purchase {1} {2}. Please make space and try again.",
["PurchaseSuccessDropped"] = "You have successfully purchased {0} {1}! Items dropped on ground (inventory full).",
["PartialItemDelivery"] = "Received {0}/{1} {2} in inventory. Remaining {3} items dropped on ground.",

Environment

Rust Inventory System: Limited container slots
High-Value Purchases: Large currency amounts
Full Inventory Scenarios: Common in active gameplay

Severity

🔴 Critical - Players lose real money when items are destroyed

Additional Notes

This issue affects player trust and can lead to chargebacks or disputes when players pay but don't receive items. The fix should prioritize never losing paid-for items even if it means dropping them on the ground.

The Orangemart plugin **fails to check available inventory space** before giving currency items to players, potentially causing **item loss** when player inventories are full. Items are given using `basePlayer.GiveItem()` without validation, which can fail silently. ## Root Cause Analysis ### **Affected Methods** #### **1. RewardPlayer() Method** (Lines ~440-465) ```csharp private void RewardPlayer(IPlayer player, int amount) { player.Reply($"You have successfully purchased {amount} {currencyName}!"); var basePlayer = player.Object as BasePlayer; if (basePlayer != null) { var currencyItem = ItemManager.CreateByItemID(currencyItemID, amount); if (currencyItem != null) { if (currencySkinID > 0) { currencyItem.skin = currencySkinID; } basePlayer.GiveItem(currencyItem); // ← NO INVENTORY CHECK Puts($"Gave {amount} {currencyName} (skinID: {currencySkinID}) to player {basePlayer.UserIDString}."); } } } ``` #### **2. ReturnCurrency() Method** (Lines ~480-495) ```csharp private void ReturnCurrency(BasePlayer player, int amount) { var returnedCurrency = ItemManager.CreateByItemID(currencyItemID, amount); if (returnedCurrency != null) { if (currencySkinID > 0) { returnedCurrency.skin = currencySkinID; } returnedCurrency.MoveToContainer(player.inventory.containerMain); // ← NO SPACE CHECK } } ``` ### **Issue Details** #### **Problem 1: Silent Item Loss** - `basePlayer.GiveItem()` returns `false` if inventory is full - Return value is **ignored**, causing silent failures - Player pays for items but doesn't receive them - **Money is lost, items are destroyed** #### **Problem 2: Partial Stack Issues** - Large currency amounts may only partially fit in inventory - No handling for cases where only some items can be given - Remaining items are lost without notification #### **Problem 3: No Fallback Mechanism** - No attempt to drop items on ground if inventory full - No notification to player about inventory space issues - No retry mechanism or delayed delivery ## Impact - 💸 **Financial Loss**: Players pay real Bitcoin but lose items - 😠 **Player Frustration**: Silent failures with no explanation - 🐛 **Support Issues**: Players reporting "I paid but got nothing" - 💰 **Refund Complications**: Difficult to track and resolve lost items - 🎮 **Poor UX**: No guidance about inventory requirements ## Steps to Reproduce 1. Fill player inventory completely (all slots occupied) 2. Player uses `/buyblood 1000` command (large amount) 3. Player pays Lightning invoice successfully 4. **Expected**: Player receives currency items 5. **Actual**: Payment succeeds, items are lost, player gets nothing ## Proposed Fix ### **Solution 1: Pre-Purchase Inventory Validation** Add inventory checks before creating invoices: ```csharp private bool HasInventorySpace(BasePlayer player, int itemID, int amount) { var testItem = ItemManager.CreateByItemID(itemID, amount); if (testItem == null) return false; // Check if player can fit the item bool canFit = player.inventory.CanAcceptItem(testItem, -1) != null; testItem.Remove(); // Clean up test item return canFit; } private void CmdBuyCurrency(IPlayer player, string command, string[] args) { // ... existing validation ... var basePlayer = player.Object as BasePlayer; if (basePlayer == null) { player.Reply(Lang("FailedToFindBasePlayer", player.Id)); return; } // NEW: Check inventory space before creating invoice if (!HasInventorySpace(basePlayer, currencyItemID, amount)) { player.Reply(Lang("InsufficientInventorySpace", player.Id, amount, currencyName)); return; } // ... continue with invoice creation ... } ``` ### **Solution 2: Smart Item Distribution with Fallback** Improve item giving with ground drop fallback: ```csharp private void RewardPlayer(IPlayer player, int amount) { var basePlayer = player.Object as BasePlayer; if (basePlayer == null) { PrintError($"Failed to find base player object for player {player.Id}."); return; } var currencyItem = ItemManager.CreateByItemID(currencyItemID, amount); if (currencyItem == null) { PrintError($"Failed to create {currencyName} item for player {basePlayer.UserIDString}."); return; } if (currencySkinID > 0) { currencyItem.skin = currencySkinID; } // Try to give item to inventory first if (basePlayer.GiveItem(currencyItem)) { player.Reply(Lang("PurchaseSuccess", player.Id, amount, currencyName)); Puts($"Gave {amount} {currencyName} to player {basePlayer.UserIDString}."); } else { // Fallback: Drop item on ground near player currencyItem.Drop(basePlayer.transform.position + Vector3.up, Vector3.zero); player.Reply(Lang("PurchaseSuccessDropped", player.Id, amount, currencyName)); Puts($"Dropped {amount} {currencyName} for player {basePlayer.UserIDString} (inventory full)."); } } ``` ### **Solution 3: Partial Stack Handling** Handle cases where only some items fit: ```csharp private void GiveItemsSafely(BasePlayer player, int itemID, int amount, ulong skinID = 0) { int remaining = amount; int maxStackSize = ItemManager.FindItemDefinition(itemID).stackable; while (remaining > 0) { int stackAmount = Math.Min(remaining, maxStackSize); var item = ItemManager.CreateByItemID(itemID, stackAmount); if (item == null) break; if (skinID > 0) item.skin = skinID; if (player.GiveItem(item)) { remaining -= stackAmount; } else { // Drop remaining items on ground item.Drop(player.transform.position + Vector3.up, Vector3.zero); break; } } if (remaining < amount) { int given = amount - remaining; player.ChatMessage($"Received {given}/{amount} {currencyName}. Check ground for remaining items."); } } ``` ## Required Language Additions Add these messages to `LoadDefaultMessages()`: ```csharp ["InsufficientInventorySpace"] = "You need at least {0} free inventory slots to purchase {1} {2}. Please make space and try again.", ["PurchaseSuccessDropped"] = "You have successfully purchased {0} {1}! Items dropped on ground (inventory full).", ["PartialItemDelivery"] = "Received {0}/{1} {2} in inventory. Remaining {3} items dropped on ground.", ``` ## Environment - **Rust Inventory System**: Limited container slots - **High-Value Purchases**: Large currency amounts - **Full Inventory Scenarios**: Common in active gameplay ## Severity 🔴 **Critical** - Players lose real money when items are destroyed ## Additional Notes This issue affects player trust and can lead to chargebacks or disputes when players pay but don't receive items. The fix should prioritize **never losing paid-for items** even if it means dropping them on the ground.

saulteafarmer commented

2025-06-20 18:48:55 +00:00

Fixed on 0.4.0

saulteafarmer closed this issue

2025-06-20 18:48:55 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: rustysats/orangemart.cs#2