Updates Item Sell checks (#3491)

* Added config values for `allow_bound_sell`
  * 0x4 allows bound item to be sold at NPC Shops
  * 0x8 restrict only guild leader who can sell guild bound item at NPC Shops or Itemshops
* Added config to allow rental items can be sold at NPC `rental_transaction` (planned for more config values based on #3490). Default to `yes` allowed to be sold, still can be restricted by item_trade, item id based.
* Fixed itemshop which ignores another item stack in inventory even for same item currency
* Fixed exploit potential by using Itemshop & Pointshop
* Now equipped item cannot be sold at NPC
* Typo and grammar fixes. Thanks to @aleos89 and @secretdataz

conf/battle/items.conf

@@ -112,9 +112,12 @@ item_flooritem_check: yes
 default_bind_on_equip: 4
 
 // Allow selling of bound/sell restricted items as Itemshop currency? (Note 3)
-// 0x0 = Bound/sell restricted items are unable to be sold at Itemshops
-// 0x1 = Bound items are able to be sold at Itemshops
-// 0x2 = Sell restricted items are able to be sold at Itemshops
+// 0x0 = Bound/sell restricted items are unable to be sold to Itemshops/Shops
+// 0x1 = Bound items are able to be sold to Itemshops
+// 0x2 = Sell restricted items are able to be sold to Itemshops
+// 0x4 = Bound items are able to be sold to Shops,
+//       because most of trade restricted items are still able to be sold to Shops
+// 0x8 = Only Guild Leader can sell BOUND_GUILD items to Shops or Itemshops (if 0x1 or 0x4 set)
 allow_bound_sell: 0x0
 
 // Turn on event refine chance (see db/{pre-}re/refine_db.yml)
@@ -127,3 +130,6 @@ event_refine_chance: no
 // Note: Players with short names can be fully converted to asterisks if this
 // config value is set high.
 broadcast_hide_name: 2
+
+// Enable to sell rental item to NPC shop? (Note 1)
+rental_transaction: yes

src/map/battle.cpp

@@ -8517,7 +8517,7 @@ static const struct _battle_data {
 	{ "guild_leaderchange_woe",				&battle_config.guild_leaderchange_woe,			0,		0,		1,				},
 	{ "guild_alliance_onlygm",              &battle_config.guild_alliance_onlygm,           0,      0,      1, },
 	{ "feature.achievement",                &battle_config.feature_achievement,             1,      0,      1,              },
-	{ "allow_bound_sell",                   &battle_config.allow_bound_sell,                0,      0,      0x3,            },
+	{ "allow_bound_sell",                   &battle_config.allow_bound_sell,                0,      0,      0xF,            },
 	{ "event_refine_chance",                &battle_config.event_refine_chance,             0,      0,      1,              },
 	{ "autoloot_adjust",                    &battle_config.autoloot_adjust,                 0,      0,      1,              },
 	{ "broadcast_hide_name",                &battle_config.broadcast_hide_name,             2,      0,      NAME_LENGTH,    },
@@ -8528,6 +8528,7 @@ static const struct _battle_data {
 	{ "homunculus_autofeed_always",         &battle_config.homunculus_autofeed_always,      1,      0,      1,              },
 	{ "feature.attendance",                 &battle_config.feature_attendance,              1,      0,      1,              },
 	{ "feature.privateairship",             &battle_config.feature_privateairship,          1,      0,      1,              },
+	{ "rental_transaction",                 &battle_config.rental_transaction,              1,      0,      1,              },
 
 #include "../custom/battle_config_init.inc"
 };

src/map/battle.hpp

@@ -649,6 +649,7 @@ struct Battle_Config
 	int homunculus_autofeed_always;
 	int feature_attendance;
 	int feature_privateairship;
+	int rental_transaction;
 
 #include "../custom/battle_config_struct.inc"
 };

src/map/clif.cpp

@@ -1963,8 +1963,11 @@ void clif_buylist(struct map_session_data *sd, struct npc_data *nd)
 void clif_selllist(struct map_session_data *sd)
 {
 	int fd,i,c=0,val;
+	struct npc_data *nd;
 
 	nullpo_retv(sd);
+	if (!sd->npc_shopid || (nd = map_id2nd(sd->npc_shopid)) == NULL)
+		return;
 
 	fd=sd->fd;
 	WFIFOHEAD(fd, MAX_INVENTORY * 10 + 4);
@@ -1973,7 +1976,7 @@ void clif_selllist(struct map_session_data *sd)
 	{
 		if( sd->inventory.u.items_inventory[i].nameid > 0 && sd->inventory_data[i] )
 		{
-			if( !pc_can_sell_item(sd, &sd->inventory.u.items_inventory[i]))
+			if( !pc_can_sell_item(sd, &sd->inventory.u.items_inventory[i], nd->subtype))
 				continue;
 
 			val=sd->inventory_data[i]->value_sell;

src/map/itemdb.hpp

@@ -740,7 +740,8 @@ enum e_itemshop_restrictions {
 	ISR_NONE = 0x0,
 	ISR_BOUND = 0x1,
 	ISR_SELLABLE = 0x2,
-	ISR_ALL = 0x3,
+	ISR_BOUND_SELLABLE = 0x4,
+	ISR_BOUND_GUILDLEADER_ONLY = 0x8,
 };
 
 ///Item combo struct

src/map/npc.cpp

@@ -1407,7 +1407,7 @@ int npc_buysellsel(struct map_session_data* sd, int id, int type)
 /** Payment Process for NPCTYPE_CASHSHOP, NPCTYPE_ITEMSHOP, and NPCTYPE_POINTSHOP
  * @param nd NPC Shop data
  * @param price Price must be paid
- * @param points Total points that player has
+ * @param points Amount of secondary points that player requested
  * @param sd Player data
  * @return e_CASHSHOP_ACK
  **/
@@ -1425,12 +1425,13 @@ static enum e_CASHSHOP_ACK npc_cashshop_process_payment(struct npc_data *nd, int
 		case NPCTYPE_ITEMSHOP:
 			{
 				struct item_data *id = itemdb_exists(nd->u.shop.itemshop_nameid);
+				int delete_amount = price, i;
 
 				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_mapid2mapname(nd->bl.m), nd->bl.x, nd->bl.y);
 					return ERROR_TYPE_PURCHASE_FAIL;
 				}
-				if (cost[0] < (price - points)) {
+				if (cost[1] < points || cost[0] < (price - points)) {
 					char output[CHAT_SIZE_MAX];
 
 					memset(output, '\0', sizeof(output));
@@ -1439,8 +1440,28 @@ static enum e_CASHSHOP_ACK npc_cashshop_process_payment(struct npc_data *nd, int
 					clif_messagecolor(&sd->bl, color_table[COLOR_RED], output, false, SELF);
 					return ERROR_TYPE_PURCHASE_FAIL;
 				}
-				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_mapid2mapname(nd->bl.m), nd->bl.x, nd->bl.y);
+
+				for (i = 0; i < MAX_INVENTORY && delete_amount > 0; i++) {
+					struct item *it;
+					int amount = 0;
+
+					if (sd->inventory.u.items_inventory[i].nameid == 0 || sd->inventory_data[i] == NULL || !(it = &sd->inventory.u.items_inventory[i]) || it->nameid != nd->u.shop.itemshop_nameid)
+						continue;
+					if (!pc_can_sell_item(sd, it, nd->subtype))
+						continue;
+
+					amount = it->amount;
+					if (amount > delete_amount)
+						amount = delete_amount;
+
+					if (pc_delitem(sd, i, amount, 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_mapid2mapname(nd->bl.m), nd->bl.x, nd->bl.y);
+						return ERROR_TYPE_PURCHASE_FAIL;
+					}
+					delete_amount -= amount;
+				}
+				if (delete_amount > 0) {
+					ShowError("Item %hu is not enough as payment at itemshop NPC '%s' (%s, %d, %d, AID=%d, CID=%d)!\n", nd->u.shop.itemshop_nameid, nd->exname, map_mapid2mapname(nd->bl.m), nd->bl.x, nd->bl.y, sd->status.account_id, sd->status.char_id);
 					return ERROR_TYPE_PURCHASE_FAIL;
 				}
 			}
@@ -1451,7 +1472,7 @@ static enum e_CASHSHOP_ACK npc_cashshop_process_payment(struct npc_data *nd, int
 
 				memset(output, '\0', sizeof(output));
 
-				if (cost[0] < (price - points)) {
+				if (cost[1] < points || cost[0] < (price - points)) {
 					sprintf(output, msg_txt(sd, 713), nd->u.shop.pointshop_str); // You do not have enough '%s'.
 					clif_messagecolor(&sd->bl, color_table[COLOR_RED], output, false, SELF);
 					return ERROR_TYPE_PURCHASE_FAIL;
@@ -1594,7 +1615,7 @@ 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].amount > 0 && sd->inventory.u.items_inventory[i].nameid == id->nameid && pc_can_sell_item(sd, &sd->inventory.u.items_inventory[i]))
+					if (sd->inventory.u.items_inventory[i].amount > 0 && sd->inventory.u.items_inventory[i].nameid == id->nameid && pc_can_sell_item(sd, &sd->inventory.u.items_inventory[i], nd->subtype))
 						total += sd->inventory.u.items_inventory[i].amount;
 				}
 			}
@@ -2014,6 +2035,10 @@ uint8 npc_selllist(struct map_session_data* sd, int n, unsigned short *item_list
 			continue;
 		}
 
+		if (!pc_can_sell_item(sd, &sd->inventory.u.items_inventory[idx], nd->subtype)) {
+			return 1; // In official server, this illegal attempt the player will be disconnected
+		}
+
 		value = pc_modifysellvalue(sd, sd->inventory_data[idx]->value_sell);
 
 		z+= (double)value*amount;

src/map/pc.cpp

@@ -522,29 +522,44 @@ void pc_inventory_rental_add(struct map_session_data *sd, unsigned int seconds)
  * Check if the player can sell the current item
  * @param sd: map_session_data of the player
  * @param item: struct of the checking item
+ * @param shoptype: NPC's sub type see enum npc_subtype
  * @return bool 'true' is sellable, 'false' otherwise
  */
-bool pc_can_sell_item(struct map_session_data *sd, struct item *item) {
-	struct npc_data *nd;
-
+bool pc_can_sell_item(struct map_session_data *sd, struct item *item, enum npc_subtype shoptype) {
 	if (sd == NULL || item == NULL)
 		return false;
 
-	nd = map_id2nd(sd->npc_shopid);
+	if (item->equip > 0 || item->amount < 0)
+		return false;
 
 	if (battle_config.hide_fav_sell && item->favorite)
 		return false; //Cannot sell favs (optional config)
 
-	if (item->expire_time)
+	if (!battle_config.rental_transaction && item->expire_time)
 		return false; // Cannot Sell Rental Items
 
-	if (nd && nd->subtype == NPCTYPE_ITEMSHOP) {
-		struct item_data *itd;
-
-		if (item->bound && battle_config.allow_bound_sell&ISR_BOUND)
-			return true; // NPCTYPE_ITEMSHOP and bound item config is sellable
-		if ((itd = itemdb_search(item->nameid)) && itd->flag.trade_restriction&8 && battle_config.allow_bound_sell&ISR_SELLABLE)
-			return true; // NPCTYPE_ITEMSHOP and sell restricted item config is sellable
+	switch (shoptype) {
+		case NPCTYPE_SHOP:
+			if (item->bound && battle_config.allow_bound_sell&ISR_BOUND_SELLABLE && (
+				item->bound != BOUND_GUILD ||
+				(sd->guild && sd->status.char_id == sd->guild->member[0].char_id) ||
+				(item->bound == BOUND_GUILD && !(battle_config.allow_bound_sell&ISR_BOUND_GUILDLEADER_ONLY))
+				))
+				return true;
+			break;
+		case NPCTYPE_ITEMSHOP:
+			if (item->bound && battle_config.allow_bound_sell&ISR_BOUND && (
+				item->bound != BOUND_GUILD ||
+				(sd->guild && sd->status.char_id == sd->guild->member[0].char_id) ||
+				(item->bound == BOUND_GUILD && !(battle_config.allow_bound_sell&ISR_BOUND_GUILDLEADER_ONLY))
+				))
+				return true;
+			else if (!item->bound) {
+				struct item_data *itd = itemdb_search(item->nameid);
+				if (itd && itd->flag.trade_restriction&8 && battle_config.allow_bound_sell&ISR_SELLABLE)
+					return true;
+			}
+			break;
 	}
 
 	if (!itemdb_cansell(item, pc_get_group_level(sd)))

src/map/pc.hpp

@@ -998,7 +998,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_sell_item(struct map_session_data* sd, struct item * item, enum npc_subtype shoptype);
 bool pc_can_give_items(struct map_session_data *sd);
 bool pc_can_give_bounded_items(struct map_session_data *sd);