Cleaned up Item Shop checks (#2373)

* Fixes #2352.
* Removed an extra bound check when selling items.
* Added a check to return failure when unable to delete an item.
* Added checks when counting sellable items in an Item Shop.
- Items that can't be sold, favorite items, rental items, and bound items will now be ignored unless the GM has required permissions.
Thanks to @chadn4u, @Jeybla, @Lemongrass3110!

src/map/clif.c

@@ -1962,18 +1962,9 @@ void clif_selllist(struct map_session_data *sd)
 	{
 		if( sd->inventory.u.items_inventory[i].nameid > 0 && sd->inventory_data[i] )
 		{
-			if( !itemdb_cansell(&sd->inventory.u.items_inventory[i], pc_get_group_level(sd)) )
+			if( !pc_can_sell_item(sd, &sd->inventory.u.items_inventory[i]))
 				continue;
 
-			if( battle_config.hide_fav_sell && sd->inventory.u.items_inventory[i].favorite )
-				continue; //Cannot sell favs [Jey]
-
-			if( sd->inventory.u.items_inventory[i].expire_time || (sd->inventory.u.items_inventory[i].bound && !pc_can_give_bounded_items(sd)) )
-				continue; // Cannot Sell Rental Items or Account Bounded Items
-
-			if( sd->inventory.u.items_inventory[i].bound && !pc_can_give_bounded_items(sd))
-				continue; // Don't allow sale of bound items
-
 			val=sd->inventory_data[i]->value_sell;
 			if( val < 0 )
 				continue;

src/map/npc.c

@@ -1412,19 +1412,23 @@ static enum e_CASHSHOP_ACK npc_cashshop_process_payment(struct npc_data *nd, int
 			{
 				struct item_data *id = itemdb_exists(nd->u.shop.itemshop_nameid);
 
+				if (!id) { // Item Data is checked at script parsing but in case of item_db reload, check again.
+					ShowWarning("Failed to find sellitem %hu for itemshop NPC '%s' (%s, %d, %d)!\n", nd->u.shop.itemshop_nameid, nd->exname, map[nd->bl.m].name, nd->bl.x, nd->bl.y);
+					return ERROR_TYPE_PURCHASE_FAIL;
+				}
 				if (cost[0] < (price - points)) {
 					char output[CHAT_SIZE_MAX];
 
 					memset(output, '\0', sizeof(output));
 
-					sprintf(output, msg_txt(sd, 712), (id) ? id->jname : "NULL", (id) ? id->nameid : 0); // You do not have enough %s (%hu).
+					sprintf(output, msg_txt(sd, 712), id->jname, id->nameid); // You do not have enough %s (%hu).
 					clif_messagecolor(&sd->bl, color_table[COLOR_RED], output, false, SELF);
 					return ERROR_TYPE_PURCHASE_FAIL;
 				}
-				if (id)
-					pc_delitem(sd, pc_search_inventory(sd, nd->u.shop.itemshop_nameid), price - points, 0, 0, LOG_TYPE_NPC);
-				else
-					ShowWarning("Failed to delete item %hu from itemshop NPC '%s' (%s, %d, %d)!\n", nd->u.shop.itemshop_nameid, nd->exname, map[nd->bl.m].name, nd->bl.x, nd->bl.y);
+				if (pc_delitem(sd, pc_search_inventory(sd, nd->u.shop.itemshop_nameid), price - points, 0, 0, LOG_TYPE_NPC)) {
+					ShowWarning("Failed to delete item %hu from '%s' at itemshop NPC '%s' (%s, %d, %d)!\n", nd->u.shop.itemshop_nameid, sd->status.name, nd->exname, map[nd->bl.m].name, nd->bl.x, nd->bl.y);
+					return ERROR_TYPE_PURCHASE_FAIL;
+				}
 			}
 			break;
 		case NPCTYPE_POINTSHOP:
@@ -1576,8 +1580,10 @@ void npc_shop_currency_type(struct map_session_data *sd, struct npc_data *nd, in
 				}
 
 				for (i = 0; i < MAX_INVENTORY; i++) {
-					if (sd->inventory.u.items_inventory[i].nameid == id->nameid)
+					if (sd->inventory.u.items_inventory[i].nameid == id->nameid &&
+					    pc_can_sell_item(sd, &sd->inventory.u.items_inventory[i])) {
 						total += sd->inventory.u.items_inventory[i].amount;
+					}
 				}
 			}
 
@@ -4795,3 +4801,4 @@ void do_init_npc(void){
 	map_addiddb(&fake_nd->bl);
 	// End of initialization
 }
+

src/map/pc.c

@@ -580,6 +580,30 @@ void pc_inventory_rental_add(struct map_session_data *sd, unsigned int seconds)
 		sd->rental_timer = add_timer(gettick() + umin(tick,3600000), pc_inventory_rental_end, sd->bl.id, 0);
 }
 
+/**
+* Check if the player can sell the current item
+* @param sd map_session_data of the player
+* @param item struct of the checking item.
+* @return bool 'true' is sellable, 'false' otherwise
+*/
+bool pc_can_sell_item(struct map_session_data * sd, struct item * item) {
+	if (sd == NULL || item == NULL)
+		return false;
+
+	if (!itemdb_cansell(item, pc_get_group_level(sd)))
+		return false;
+
+	if (battle_config.hide_fav_sell && item->favorite)
+		return false; //Cannot sell favs (optional config)
+
+	if (item->expire_time)
+		return false; // Cannot Sell Rental Items
+
+	if (item->bound && !pc_can_give_bounded_items(sd))
+		return false; // Don't allow sale of bound items
+	return true;
+}
+
 /**
  * Determines if player can give / drop / trade / vend items
  */

src/map/pc.h

@@ -989,6 +989,7 @@ int pc_split_atoi(char* str, int* val, char sep, int max);
 int pc_class2idx(int class_);
 int pc_get_group_level(struct map_session_data *sd);
 int pc_get_group_id(struct map_session_data *sd);
+bool pc_can_sell_item(struct map_session_data* sd, struct item * item);
 bool pc_can_give_items(struct map_session_data *sd);
 bool pc_can_give_bounded_items(struct map_session_data *sd);