* Replaced jA's way of allocating npc shop data with a simple dynamic array that gets allocated during loading and freed on unload

- automatically fixes bugreport:404, which would otherwise require manipulating the npcname_db (the original author didn't, hence the bug)
- now a supporting variable 'count' is used for tracking the length instead of an extra dummy entry at the end of the shop list
- partially removed the MAX_SHOPITEM restriction (if this was written properly, the system could support an unlimited amount of entries)

git-svn-id: https://svn.code.sf.net/p/rathena/svn/trunk@11753 54d463be-8e91-2dee-dedb-b68131a5f0ec

Changelog-Trunk.txt

@@ -3,6 +3,15 @@ Date	Added
 AS OF SVN REV. 5091, WE ARE NOW USING TRUNK.  ALL UNTESTED BUGFIXES/FEATURES GO INTO TRUNK.
 IF YOU HAVE A WORKING AND TESTED BUGFIX PUT IT INTO STABLE AS WELL AS TRUNK.
 
+2007/11/18
+	* Replaced jA's way of allocating npc shop data with a simple dynamic
+	  array that gets allocated during loading and freed on unload
+	- automatically fixes bugreport:404, which would otherwise require
+	  manipulating the npcname_db (the original author didn't, hence the bug)
+	- now a supporting variable 'count' is used for tracking the length
+	  instead of an extra dummy entry at the end of the shop list
+	- partially removed the MAX_SHOPITEM restriction (if this was written
+	  properly, the system could support an unlimited amount of entries)
 2007/11/17
 	* Removed battle_config.error_log as console_silent already handles this
 	* Made OnTouch zone dimensions be stored as radius instead of diameter

src/map/clif.c

@@ -1700,29 +1700,26 @@ int clif_npcbuysell(struct map_session_data* sd, int id)
  *------------------------------------------*/
 int clif_buylist(struct map_session_data *sd, struct npc_data *nd)
 {
-	struct item_data *id;
-	int fd,i,val;
+	int fd,i;
 
 	nullpo_retr(0, sd);
 	nullpo_retr(0, nd);
 
-	fd=sd->fd;
+	fd = sd->fd;
  	WFIFOHEAD(fd, 200 * 11 + 4);
-	WFIFOW(fd,0)=0xc6;
-	for(i=0;nd->u.shop_item[i].nameid > 0;i++){
-		id = itemdb_search(nd->u.shop_item[i].nameid);
-		val=nd->u.shop_item[i].value;
-		WFIFOL(fd,4+i*11)=val;
+	WFIFOW(fd,0) = 0xc6;
+	WFIFOW(fd,2) = 4 + nd->u.shop.count*11;
+	for( i = 0; i < nd->u.shop.count; i++ )
+	{
+		struct item_data* id = itemdb_search(nd->u.shop.shop_item[i].nameid);
+		int val = nd->u.shop.shop_item[i].value;
+		WFIFOL(fd,4+i*11) = val;
 		if (!id->flag.value_notdc)
-			val=pc_modifybuyvalue(sd,val);
-		WFIFOL(fd,8+i*11)=val;
-		WFIFOB(fd,12+i*11)=itemtype(id->type);
-		if (id->view_id > 0)
-			WFIFOW(fd,13+i*11)=id->view_id;
-		else
-			WFIFOW(fd,13+i*11)=nd->u.shop_item[i].nameid;
+			val = pc_modifybuyvalue(sd,val);
+		WFIFOL(fd,8+i*11) = val;
+		WFIFOB(fd,12+i*11) = itemtype(id->type);
+		WFIFOW(fd,13+i*11) = ( id->view_id > 0 ) ? id->view_id : id->nameid;
 	}
-	WFIFOW(fd,2)=i*11+4;
 	WFIFOSET(fd,WFIFOW(fd,2));
 
 	return 0;

src/map/map.h

@@ -847,14 +847,16 @@ struct npc_data {
 			struct npc_label_list *label_list;
 			int src_id;
 		} scr;
-		struct npc_item_list shop_item[1];// dynamic array, allocated with extra entries (last one has nameid 0)
+		struct {
+			struct npc_item_list* shop_item;
+			int count;
+		} shop;
 		struct {
 			short xs,ys; // OnTouch area radius
 			short x,y; // destination coords
 			unsigned short mapindex; // destination map
 		} warp;
 	} u;
-	//Do NOT place anything afterwards... shop data NPC will override any variables from here and on! [Skotlex]
 };
 
 //For quick linking to a guardian's info. [Skotlex]

src/map/npc.c

@@ -1056,24 +1056,24 @@ int npc_buylist(struct map_session_data* sd, int n, unsigned short* item_list)
 		return 3;
 
 	for(i=0,w=0,z=0;i<n;i++) {
-		for(j=0;nd->u.shop_item[j].nameid;j++) {
-			if (nd->u.shop_item[j].nameid==item_list[i*2+1] || //Normal items
-				itemdb_viewid(nd->u.shop_item[j].nameid)==item_list[i*2+1]) //item_avail replacement
+		for(j=0;nd->u.shop.shop_item[j].nameid;j++) {
+			if (nd->u.shop.shop_item[j].nameid==item_list[i*2+1] || //Normal items
+				itemdb_viewid(nd->u.shop.shop_item[j].nameid)==item_list[i*2+1]) //item_avail replacement
 				break;
 		}
-		if (nd->u.shop_item[j].nameid==0)
+		if (nd->u.shop.shop_item[j].nameid==0)
 			return 3;
 		
-		if (!itemdb_isstackable(nd->u.shop_item[j].nameid) && item_list[i*2] > 1)
+		if (!itemdb_isstackable(nd->u.shop.shop_item[j].nameid) && item_list[i*2] > 1)
 		{	//Exploit? You can't buy more than 1 of equipment types o.O
 			ShowWarning("Player %s (%d:%d) sent a hexed packet trying to buy %d of nonstackable item %d!\n",
-				sd->status.name, sd->status.account_id, sd->status.char_id, item_list[i*2], nd->u.shop_item[j].nameid);
+				sd->status.name, sd->status.account_id, sd->status.char_id, item_list[i*2], nd->u.shop.shop_item[j].nameid);
 			item_list[i*2] = 1;
 		}
-		if (itemdb_value_notdc(nd->u.shop_item[j].nameid))
-			z+=(double)nd->u.shop_item[j].value * item_list[i*2];
+		if (itemdb_value_notdc(nd->u.shop.shop_item[j].nameid))
+			z+=(double)nd->u.shop.shop_item[j].value * item_list[i*2];
 		else
-			z+=(double)pc_modifybuyvalue(sd,nd->u.shop_item[j].value) * item_list[i*2];
+			z+=(double)pc_modifybuyvalue(sd,nd->u.shop.shop_item[j].value) * item_list[i*2];
 		itemamount+=item_list[i*2];
 
 		switch(pc_checkadditem(sd,item_list[i*2+1],item_list[i*2])) {
@@ -1299,7 +1299,11 @@ int npc_unload(struct npc_data* nd)
 	npc_chat_finalize(nd); // deallocate npc PCRE data structures
 #endif
 
-	if (nd->bl.subtype == SCRIPT) {
+	if( nd->bl.subtype == SHOP )
+		free(nd->u.shop.shop_item);
+	else
+	if( nd->bl.subtype == SCRIPT )
+	{
 		ev_db->foreach(ev_db,npc_unload_ev,nd->exname); //Clean up all events related.
 		if (nd->u.scr.timerid != -1) {
 			struct TimerData *td = NULL;
@@ -1322,7 +1326,9 @@ int npc_unload(struct npc_data* nd)
 			}
 		}
 	}
+
 	script_stop_sleeptimers(nd->bl.id);
+
 	aFree(nd);
 
 	return 0;
@@ -1577,6 +1583,7 @@ static const char* npc_parse_warp(char* w1, char* w2, char* w3, char* w4, const
 /// Parses a shop npc.
 static const char* npc_parse_shop(char* w1, char* w2, char* w3, char* w4, const char* start, const char* buffer, const char* filepath)
 {
+	//TODO: could be rewritten to NOT need this temp array [ultramage] 
 	#define MAX_SHOPITEM 100
 	struct npc_item_list items[MAX_SHOPITEM];
 	char *p;
@@ -1631,9 +1638,10 @@ static const char* npc_parse_shop(char* w1, char* w2, char* w3, char* w4, const
 		return strchr(start,'\n');// continue
 	}
 
-	nd = (struct npc_data *) aCalloc (1, sizeof(struct npc_data) + sizeof(nd->u.shop_item[0])*(i));
-	memcpy(&nd->u.shop_item, items, sizeof(struct npc_item_list)*i);
-	nd->u.shop_item[i].nameid = 0;
+	CREATE(nd, struct npc_data, 1);
+	CREATE(nd->u.shop.shop_item, struct npc_item_list, i);
+	memcpy(nd->u.shop.shop_item, items, sizeof(struct npc_item_list)*i);
+	nd->u.shop.count = i;
 	nd->bl.prev = nd->bl.next = NULL;
 	nd->bl.m = m;
 	nd->bl.x = x;

src/map/script.c

@@ -11713,86 +11713,58 @@ BUILDIN_FUNC(callshop)
 	return 0;
 }
 
-#ifndef MAX_SHOPITEM
-	#define MAX_SHOPITEM 100
-#endif
 BUILDIN_FUNC(npcshopitem)
 {
-	struct npc_data *nd= NULL;
-	int n = 0;
-	int i = 3;
-	int amount;
-
 	const char* npcname = script_getstr(st, 2);
-	nd = npc_name2id(npcname);
+	struct npc_data* nd = npc_name2id(npcname);
+	int n, i;
+	int amount;
 
-	if(!nd || nd->bl.subtype!=SHOP)
+	if( !nd || nd->bl.subtype != SHOP )
 	{	//Not found.
 		script_pushint(st,0);
 		return 0;
 	}
 
-	// st->end - 2 = nameid + value # ... / 2 = number of items ... + 1 to end it
-	amount = ((st->end-2)/2)+1;
-
-	//Reinsert as realloc could change the pointer address.
-	map_deliddb(&nd->bl);
-	nd = (struct npc_data *)aRealloc(nd,sizeof(struct npc_data) +
-		sizeof(nd->u.shop_item[0]) * amount);
-	map_addiddb(&nd->bl);
-
-	// Reset sell list.
-	memset(nd->u.shop_item, 0, sizeof(nd->u.shop_item[0]) * amount);
-
-	n = 0;
+	// st->end - 2 = nameid + value # ... / 2 = number of items
+	amount = (st->end-2)/2;
 
-	while (script_hasdata(st,i)) {
-		nd->u.shop_item[n].nameid = script_getnum(st,i);
-		i++;
-		nd->u.shop_item[n].value = script_getnum(st,i);
-		i++;
-		n++;
+	// generate new shop item list
+	RECREATE(nd->u.shop.shop_item, struct npc_item_list, amount);
+	for( n = 0, i = 3; n < amount; n++, i+=2 )
+	{
+		nd->u.shop.shop_item[n].nameid = script_getnum(st,i);
+		nd->u.shop.shop_item[n].value = script_getnum(st,i+1);
 	}
+	nd->u.shop.count = n;
+
+	script_pushint(st,1);
 	return 0;
 }
 
 BUILDIN_FUNC(npcshopadditem)
 {
-	struct npc_data *nd=NULL;
-	int n = 0;
-	int i = 3;
+	const char* npcname = script_getstr(st,2);
+	struct npc_data* nd = npc_name2id(npcname);
+	int n, i;
 	int amount;
 
-	const char* npcname = script_getstr(st, 2);
-	nd = npc_name2id(npcname);
-
-	if (!nd || nd->bl.subtype!=SHOP)
+	if( !nd || nd->bl.subtype != SHOP )
 	{	//Not found.
 		script_pushint(st,0);
 		return 0;
 	}
-	amount = ((st->end-2)/2)+1;
-	while (nd->u.shop_item[n].nameid && n < MAX_SHOPITEM)
-		n++;
 
+	amount = (st->end-2)/2;
 
-	//Reinsert as realloc could change the pointer address.
-	map_deliddb(&nd->bl);
-	nd = (struct npc_data *)aRealloc(nd,sizeof(struct npc_data) +
-		sizeof(nd->u.shop_item[0]) * (amount+n));
-	map_addiddb(&nd->bl);
-
-	while(script_hasdata(st,i)){
-		nd->u.shop_item[n].nameid = script_getnum(st,i);
-		i++;
-		nd->u.shop_item[n].value = script_getnum(st,i);
-		i++;
-		n++;
+	// append new items to existing shop item list
+	RECREATE(nd->u.shop.shop_item, struct npc_item_list, nd->u.shop.count+amount);
+	for( n = nd->u.shop.count, i = 3; n < nd->u.shop.count+amount; n++, i+=2 )
+	{
+		nd->u.shop.shop_item[n].nameid = script_getnum(st,i);
+		nd->u.shop.shop_item[n].value = script_getnum(st,i+1);
 	}
-
-	// Marks the last of our stuff..
-	nd->u.shop_item[n].value = 0;
-	nd->u.shop_item[n].nameid = 0;
+	nd->u.shop.count = n;
 
 	script_pushint(st,1);
 	return 0;
@@ -11800,61 +11772,50 @@ BUILDIN_FUNC(npcshopadditem)
 
 BUILDIN_FUNC(npcshopdelitem)
 {
-	struct npc_data *nd=NULL;
-	int n=0;
-	int i=3;
-	int size = 0;
-
-	const char* npcname = script_getstr(st, 2);
-	nd = npc_name2id(npcname);
+	const char* npcname = script_getstr(st,2);
+	struct npc_data* nd = npc_name2id(npcname);
+	int n, i;
+	int amount;
+	int size;
 
-	if (!nd || nd->bl.subtype!=SHOP)
+	if( !nd || nd->bl.subtype != SHOP )
 	{	//Not found.
 		script_pushint(st,0);
 		return 0;
 	}
 
-	while (nd->u.shop_item[size].nameid)
-		size++;
+	amount = (st->end-2)/2;
+	size = nd->u.shop.count;
 
-	while (script_hasdata(st,i)) {
-		for(n=0;nd->u.shop_item[n].nameid && n < MAX_SHOPITEM;n++) {
-			if (nd->u.shop_item[n].nameid == script_getnum(st,i)) {
-				// We're moving 1 extra empty block. Junk data is eliminated later.
-				memmove(&nd->u.shop_item[n], &nd->u.shop_item[n+1], sizeof(nd->u.shop_item[0])*(size-n));
-			}
+	// remove specified items from the shop item list
+	for( i = 3; i < 3 + amount; i++ )
+	{
+		ARR_FIND( 0, size, n, nd->u.shop.shop_item[n].nameid == script_getnum(st,i) );
+		if( n < size )
+		{
+			memmove(&nd->u.shop.shop_item[n], &nd->u.shop.shop_item[n+1], sizeof(nd->u.shop.shop_item[0])*(size-n));
+			size--;
 		}
-		i++;
 	}
 
-	size = 0;
-
-	while (nd->u.shop_item[size].nameid)
-		size++;
-
-	//Reinsert as realloc could change the pointer address.
-	map_deliddb(&nd->bl);
-	nd = (struct npc_data *)aRealloc(nd,sizeof(struct npc_data) +
-		sizeof(nd->u.shop_item[0]) * (size+1));
-	map_addiddb(&nd->bl);
+	RECREATE(nd->u.shop.shop_item, struct npc_item_list, size);
+	nd->u.shop.count = size;
 
 	script_pushint(st,1);
 	return 0;
 }
 
-//Sets a script to attach to an npc.
+//Sets a script to attach to a shop npc.
 BUILDIN_FUNC(npcshopattach)
 {
-	struct npc_data *nd=NULL;
-	const char* npcname = script_getstr(st, 2);
+	const char* npcname = script_getstr(st,2);
+	struct npc_data* nd = npc_name2id(npcname);
 	int flag = 1;
 
 	if( script_hasdata(st,3) )
 		flag = script_getnum(st,3);
 
-	nd = npc_name2id(npcname);
-
-	if (!nd || nd->bl.subtype!=SHOP)
+	if( !nd || nd->bl.subtype != SHOP )
 	{	//Not found.
 		script_pushint(st,0);
 		return 0;
@@ -11864,6 +11825,7 @@ BUILDIN_FUNC(npcshopattach)
 		nd->master_nd = ((struct npc_data *)map_id2bl(st->oid));
 	else
 		nd->master_nd = NULL;
+
 	script_pushint(st,1);
 	return 0;
 }