Quest Log system improvement. (Hercules 6f55c00)

- Improved memory usage of the quest log system. (saves up to 75kB per online character).
- Fixed various issues with quest entries disappearing from characters without an apparent reason, or monster kill counters getting stuck - the issues were caused by a de-synchronization between the two parallel questlog arrays in map_session_data.
- Added some code documentation.
- Thanks to @shennetsind and @MishimaHaruna.

src/char/int_quest.c

@@ -18,44 +18,77 @@
 #include <string.h>
 #include <stdlib.h>
 
-//Load entire questlog for a character
-int mapif_quests_fromsql(int char_id, struct quest questlog[])
-{
-	int i;
+/**
+ * Loads the entire questlog for a character.
+ *
+ * @param char_id Character ID
+ * @param count   Pointer to return the number of found entries.
+ * @return Array of found entries. It has *count entries, and it is care of the
+ *         caller to aFree() it afterwards.
+ */
+struct quest *mapif_quests_fromsql(int char_id, int *count) {
+	struct quest *questlog = NULL;
 	struct quest tmp_quest;
-	SqlStmt * stmt;
+	SqlStmt *stmt;
+
+	if( !count )
+		return NULL;
 
 	stmt = SqlStmt_Malloc(sql_handle);
-	if( stmt == NULL )
-	{
+	if( stmt == NULL ) {
 		SqlStmt_ShowDebug(stmt);
-		return 0;
+		*count = 0;
+		return NULL;
 	}
 
 	memset(&tmp_quest, 0, sizeof(struct quest));
 
-	if( SQL_ERROR == SqlStmt_Prepare(stmt, "SELECT `quest_id`, `state`, `time`, `count1`, `count2`, `count3` FROM `%s` WHERE `char_id`=? LIMIT %d", quest_db, MAX_QUEST_DB)
+	if( SQL_ERROR == SqlStmt_Prepare(stmt, "SELECT `quest_id`, `state`, `time`, `count1`, `count2`, `count3` FROM `%s` WHERE `char_id`=?", quest_db)
 	||	SQL_ERROR == SqlStmt_BindParam(stmt, 0, SQLDT_INT, &char_id, 0)
 	||	SQL_ERROR == SqlStmt_Execute(stmt)
-	||	SQL_ERROR == SqlStmt_BindColumn(stmt, 0, SQLDT_INT,    &tmp_quest.quest_id, 0, NULL, NULL)
-	||	SQL_ERROR == SqlStmt_BindColumn(stmt, 1, SQLDT_INT,    &tmp_quest.state, 0, NULL, NULL)
-	||	SQL_ERROR == SqlStmt_BindColumn(stmt, 2, SQLDT_UINT,   &tmp_quest.time, 0, NULL, NULL)
-	||	SQL_ERROR == SqlStmt_BindColumn(stmt, 3, SQLDT_INT,    &tmp_quest.count[0], 0, NULL, NULL)
-	||	SQL_ERROR == SqlStmt_BindColumn(stmt, 4, SQLDT_INT,    &tmp_quest.count[1], 0, NULL, NULL)
-	||	SQL_ERROR == SqlStmt_BindColumn(stmt, 5, SQLDT_INT,    &tmp_quest.count[2], 0, NULL, NULL) )
+	||	SQL_ERROR == SqlStmt_BindColumn(stmt, 0, SQLDT_INT,  &tmp_quest.quest_id, 0, NULL, NULL)
+	||	SQL_ERROR == SqlStmt_BindColumn(stmt, 1, SQLDT_INT,  &tmp_quest.state,    0, NULL, NULL)
+	||	SQL_ERROR == SqlStmt_BindColumn(stmt, 2, SQLDT_UINT, &tmp_quest.time,     0, NULL, NULL)
+	||	SQL_ERROR == SqlStmt_BindColumn(stmt, 3, SQLDT_INT,  &tmp_quest.count[0], 0, NULL, NULL)
+	||	SQL_ERROR == SqlStmt_BindColumn(stmt, 4, SQLDT_INT,  &tmp_quest.count[1], 0, NULL, NULL)
+	||	SQL_ERROR == SqlStmt_BindColumn(stmt, 5, SQLDT_INT,  &tmp_quest.count[2], 0, NULL, NULL)
+	) {
 		SqlStmt_ShowDebug(stmt);
+		SqlStmt_Free(stmt);
+		*count = 0;
+		return NULL;
+	}
 
-	for( i = 0; i < MAX_QUEST_DB && SQL_SUCCESS == SqlStmt_NextRow(stmt); ++i )
-		memcpy(&questlog[i], &tmp_quest, sizeof(tmp_quest));
+	*count = (int)SqlStmt_NumRows(stmt);
+	if( *count > 0 ) {
+		int i = 0;
+
+		questlog = (struct quest *)aCalloc(*count, sizeof(struct quest));
+		while( SQL_SUCCESS == SqlStmt_NextRow(stmt) ) {
+			if( i >= *count ) //Sanity check, should never happen
+				break;
+			memcpy(&questlog[i++], &tmp_quest, sizeof(tmp_quest));
+		}
+		if( i < *count ) {
+			//Should never happen. Compact array
+			*count = i;
+			questlog = (struct quest *)aRealloc(questlog, sizeof(struct quest) * i);
+		}
+	}
 
 	SqlStmt_Free(stmt);
-	return i;
+	return questlog;
 }
 
-//Delete a quest
-bool mapif_quest_delete(int char_id, int quest_id)
-{
-	if ( SQL_ERROR == Sql_Query(sql_handle, "DELETE FROM `%s` WHERE `quest_id` = '%d' AND `char_id` = '%d'", quest_db, quest_id, char_id) )
+/**
+ * Deletes a quest from a character's questlog.
+ *
+ * @param char_id  Character ID
+ * @param quest_id Quest ID
+ * @return false in case of errors, true otherwise
+ */
+bool mapif_quest_delete(int char_id, int quest_id) {
+	if( SQL_ERROR == Sql_Query(sql_handle, "DELETE FROM `%s` WHERE `quest_id` = '%d' AND `char_id` = '%d'", quest_db, quest_id, char_id) )
 	{
 		Sql_ShowDebug(sql_handle);
 		return false;
@@ -64,10 +97,15 @@ bool mapif_quest_delete(int char_id, int quest_id)
 	return true;
 }
 
-//Add a quest to a questlog
-bool mapif_quest_add(int char_id, struct quest qd)
-{
-	if ( SQL_ERROR == Sql_Query(sql_handle, "INSERT INTO `%s`(`quest_id`, `char_id`, `state`, `time`, `count1`, `count2`, `count3`) VALUES ('%d', '%d', '%d','%d', '%d', '%d', '%d')", quest_db, qd.quest_id, char_id, qd.state, qd.time, qd.count[0], qd.count[1], qd.count[2]) ) 
+/**
+ * Adds a quest to a character's questlog.
+ *
+ * @param char_id Character ID
+ * @param qd      Quest data
+ * @return false in case of errors, true otherwise
+ */
+bool mapif_quest_add(int char_id, struct quest qd) {
+	if( SQL_ERROR == Sql_Query(sql_handle, "INSERT INTO `%s`(`quest_id`, `char_id`, `state`, `time`, `count1`, `count2`, `count3`) VALUES ('%d', '%d', '%d','%d', '%d', '%d', '%d')", quest_db, qd.quest_id, char_id, qd.state, qd.time, qd.count[0], qd.count[1], qd.count[2]) )
 	{
 		Sql_ShowDebug(sql_handle);
 		return false;
@@ -76,10 +114,15 @@ bool mapif_quest_add(int char_id, struct quest qd)
 	return true;
 }
 
-//Update a questlog
-bool mapif_quest_update(int char_id, struct quest qd)
-{
-	if ( SQL_ERROR == Sql_Query(sql_handle, "UPDATE `%s` SET `state`='%d', `count1`='%d', `count2`='%d', `count3`='%d' WHERE `quest_id` = '%d' AND `char_id` = '%d'", quest_db, qd.state, qd.count[0], qd.count[1], qd.count[2], qd.quest_id, char_id) ) 
+/**
+ * Updates a quest in a character's questlog.
+ *
+ * @param char_id Character ID
+ * @param qd      Quest data
+ * @return false in case of errors, true otherwise
+ */
+bool mapif_quest_update(int char_id, struct quest qd) {
+	if( SQL_ERROR == Sql_Query(sql_handle, "UPDATE `%s` SET `state`='%d', `count1`='%d', `count2`='%d', `count3`='%d' WHERE `quest_id` = '%d' AND `char_id` = '%d'", quest_db, qd.state, qd.count[0], qd.count[1], qd.count[2], qd.quest_id, char_id) ) 
 	{
 		Sql_ShowDebug(sql_handle);
 		return false;
@@ -88,93 +131,95 @@ bool mapif_quest_update(int char_id, struct quest qd)
 	return true;
 }
 
-//Save quests
-int mapif_parse_quest_save(int fd)
-{
-	int i, j, k, num2, num1 = (RFIFOW(fd,2)-8)/sizeof(struct quest);
+/**
+ * Handles the save request from mapserver for a character's questlog.
+ *
+ * Received quests are saved, and an ack is sent back to the map server.
+ *
+ * @see inter_parse_frommap
+ */
+int mapif_parse_quest_save(int fd) {
+	int i, j, k, old_n, new_n = (RFIFOW(fd,2) - 8) / sizeof(struct quest);
 	int char_id = RFIFOL(fd,4);
-	struct quest qd1[MAX_QUEST_DB],qd2[MAX_QUEST_DB];
+	struct quest *old_qd = NULL, *new_qd = NULL;
 	bool success = true;
 
-	memset(qd1, 0, sizeof(qd1));
-	memset(qd2, 0, sizeof(qd2));
-	if( num1 ) memcpy(&qd1, RFIFOP(fd,8), RFIFOW(fd,2)-8);
-	num2 = mapif_quests_fromsql(char_id, qd2);
-
-	for( i = 0; i < num1; i++ )
-	{
-		ARR_FIND( 0, num2, j, qd1[i].quest_id == qd2[j].quest_id );
-		if( j < num2 ) // Update existed quests
-		{	// Only states and counts are changable.
-			ARR_FIND( 0, MAX_QUEST_OBJECTIVES, k, qd1[i].count[k] != qd2[j].count[k] );
-			if( k != MAX_QUEST_OBJECTIVES || qd1[i].state != qd2[j].state )
-				success &= mapif_quest_update(char_id, qd1[i]);
-
-			if( j < (--num2) )
-			{
-				memmove(&qd2[j],&qd2[j+1],sizeof(struct quest)*(num2-j));
-				memset(&qd2[num2], 0, sizeof(struct quest));
+	if( new_n > 0 )
+		new_qd = (struct quest*)RFIFOP(fd,8);
+
+	old_qd = mapif_quests_fromsql(char_id, &old_n);
+	for( i = 0; i < new_n; i++ ) {
+		ARR_FIND(0, old_n, j, new_qd[i].quest_id == old_qd[j].quest_id);
+		if( j < old_n ) { //Update existing quests
+			//Only states and counts are changable.
+			ARR_FIND(0, MAX_QUEST_OBJECTIVES, k, new_qd[i].count[k] != old_qd[j].count[k]);
+			if( k != MAX_QUEST_OBJECTIVES || new_qd[i].state != old_qd[j].state )
+				success &= mapif_quest_update(char_id, new_qd[i]);
+
+			if( j < (--old_n) ) {
+				//Compact array
+				memmove(&old_qd[j], &old_qd[j + 1], sizeof(struct quest) * (old_n - j));
+				memset(&old_qd[old_n], 0, sizeof(struct quest));
 			}
-
-		}
-		else // Add new quests
-			success &= mapif_quest_add(char_id, qd1[i]);
+		} else //Add new quests
+			success &= mapif_quest_add(char_id, new_qd[i]);
 	}
 
-	for( i = 0; i < num2; i++ ) // Quests not in qd1 but in qd2 are to be erased.
-		success &= mapif_quest_delete(char_id, qd2[i].quest_id);
+	for( i = 0; i < old_n; i++ ) //Quests not in new_qd but in old_qd are to be erased.
+		success &= mapif_quest_delete(char_id, old_qd[i].quest_id);
 
+	if( old_qd )
+		aFree(old_qd);
+
+	//Send ack
 	WFIFOHEAD(fd,7);
 	WFIFOW(fd,0) = 0x3861;
 	WFIFOL(fd,2) = char_id;
-	WFIFOB(fd,6) = success?1:0;
+	WFIFOB(fd,6) = success ? 1 : 0;
 	WFIFOSET(fd,7);
 
 	return 0;
 }
 
-//Send questlog to map server
-int mapif_parse_quest_load(int fd)
-{
+/**
+ * Sends questlog to the map server
+ *
+ * NOTE: Completed quests (state == Q_COMPLETE) are guaranteed to be sent last
+ * and the map server relies on this behavior (once the first Q_COMPLETE quest,
+ * all of them are considered to be Q_COMPLETE)
+ *
+ * @see inter_parse_frommap
+ */
+int mapif_parse_quest_load(int fd) {
 	int char_id = RFIFOL(fd,2);
-	struct quest tmp_questlog[MAX_QUEST_DB];
-	int num_quests, i, num_complete = 0;
-	int complete[MAX_QUEST_DB];
-
-	memset(tmp_questlog, 0, sizeof(tmp_questlog));
-	memset(complete, 0, sizeof(complete));
+	struct quest *tmp_questlog = NULL;
+	int num_quests;
 
-	num_quests = mapif_quests_fromsql(char_id, tmp_questlog);
+	tmp_questlog = mapif_quests_fromsql(char_id, &num_quests);
 
-	WFIFOHEAD(fd,num_quests*sizeof(struct quest)+8);
+	WFIFOHEAD(fd,num_quests * sizeof(struct quest) + 8);
 	WFIFOW(fd,0) = 0x3860;
-	WFIFOW(fd,2) = num_quests*sizeof(struct quest)+8;
+	WFIFOW(fd,2) = num_quests * sizeof(struct quest) + 8;
 	WFIFOL(fd,4) = char_id;
 
-	//Active and inactive quests
-	for( i = 0; i < num_quests; i++ )
-	{
-		if( tmp_questlog[i].state == Q_COMPLETE )
-		{
-			complete[num_complete++] = i;
-			continue;
-		}
-		memcpy(WFIFOP(fd,(i-num_complete)*sizeof(struct quest)+8), &tmp_questlog[i], sizeof(struct quest));
-	}
+	if( num_quests > 0 )
+		memcpy(WFIFOP(fd,8), tmp_questlog, sizeof(struct quest) * num_quests);
 
-	// Completed quests
-	for( i = num_quests - num_complete; i < num_quests; i++ )
-		memcpy(WFIFOP(fd,i*sizeof(struct quest)+8), &tmp_questlog[complete[i-num_quests+num_complete]], sizeof(struct quest));
+	WFIFOSET(fd,num_quests * sizeof(struct quest) + 8);
 
-	WFIFOSET(fd,num_quests*sizeof(struct quest)+8);
+	if( tmp_questlog )
+		aFree(tmp_questlog);
 
 	return 0;
 }
 
-int inter_quest_parse_frommap(int fd)
-{
-	switch(RFIFOW(fd,0))
-	{
+/**
+ * Parses questlog related packets from the map server.
+ *
+ * @see inter_parse_frommap
+ */
+int inter_quest_parse_frommap(int fd) {
+	switch(RFIFOW(fd,0)) {
 		case 0x3060: mapif_parse_quest_load(fd); break;
 		case 0x3061: mapif_parse_quest_save(fd); break;
 		default:

src/char/int_quest.h

@@ -4,9 +4,6 @@
 #ifndef _QUEST_H_
 #define _QUEST_H_
 
-/*questlog system*/
-struct quest;
-
 int inter_quest_parse_frommap(int fd);
 
 #endif

src/common/mmo.h

@@ -72,7 +72,6 @@
 #define MAX_GUILDSKILL	15 ///Increased max guild skills because of new skills [Sara-chan]
 #define MAX_GUILDLEVEL 50 ///Max Guild level
 #define MAX_GUARDIANS 8	///Local max per castle. If this value is increased, need to add more fields on MySQL `guild_castle` table [Skotlex]
-#define MAX_QUEST_DB 2800 ///Max quests that the server will load
 #define MAX_QUEST_OBJECTIVES 3 ///Max quest objectives for a quest
 
 // for produce
@@ -157,15 +156,19 @@ enum item_types {
 	IT_MAX
 };
 
+// Questlog states
+enum quest_state {
+	Q_INACTIVE, ///< Inactive quest (the user can toggle between active and inactive quests)
+	Q_ACTIVE,   ///< Active quest
+	Q_COMPLETE, ///< Completed quest
+};
 
-//Questlog system [Kevin] [Inkfish]
-typedef enum quest_state { Q_INACTIVE, Q_ACTIVE, Q_COMPLETE } quest_state;
-
+/// Questlog entry
 struct quest {
-	int quest_id;
-	unsigned int time;
-	int count[MAX_QUEST_OBJECTIVES];
-	quest_state state;
+	int quest_id;                    ///< Quest ID
+	unsigned int time;               ///< Expiration time
+	int count[MAX_QUEST_OBJECTIVES]; ///< Kill counters of each quest objective
+	enum quest_state state;          ///< Current quest state
 };
 
 struct item {

src/map/clif.c

@@ -11042,19 +11042,19 @@ void clif_parse_GetItemFromCart(int fd,struct map_session_data *sd)
 }
 
 
-/// Request to remove cart/falcon/peco/dragon (CZ_REQ_CARTOFF).
+/// Request to remove cart/falcon/peco/dragon/mado (CZ_REQ_CARTOFF).
 /// 012a
 void clif_parse_RemoveOption(int fd,struct map_session_data *sd)
 {
 	if( !(sd->sc.option&(OPTION_RIDING|OPTION_FALCON|OPTION_DRAGON|OPTION_MADOGEAR))
 #ifdef NEW_CARTS
-	    && sd->sc.data[SC_PUSH_CART] ){
+		&& sd->sc.data[SC_PUSH_CART] )
 		pc_setcart(sd,0);
 #else
-	    ){
+		)
 		pc_setoption(sd,sd->sc.option&~OPTION_CART);
 #endif
-	} else  // priority to remove this option before we can clear cart
+	else  // priority to remove this option before we can clear cart
 		pc_setoption(sd,sd->sc.option&~(OPTION_RIDING|OPTION_FALCON|OPTION_DRAGON|OPTION_MADOGEAR));
 }
 
@@ -11095,7 +11095,7 @@ void clif_parse_ChangeCart(int fd,struct map_session_data *sd)
 /// status id:
 ///     SP_STR ~ SP_LUK
 /// amount:
-///     Old client send always 1 for this, even when using /str+ and the like.
+///     Old clients always send 1 for this, even when using /str+ and the like.
 ///     Newer clients (2013-12-23 and newer) send the correct amount.
 void clif_parse_StatusUp(int fd,struct map_session_data *sd)
 {
@@ -15225,16 +15225,18 @@ void clif_quest_send_mission(struct map_session_data * sd)
 	WFIFOL(fd, 4) = sd->avail_quests;
 
 	for( i = 0; i < sd->avail_quests; i++ ) {
+		struct quest_db *qi = quest_db(sd->quest_log[i].quest_id);
+
 		WFIFOL(fd, i*104+8) = sd->quest_log[i].quest_id;
-		WFIFOL(fd, i*104+12) = sd->quest_log[i].time - quest_db[sd->quest_index[i]].time;
+		WFIFOL(fd, i*104+12) = sd->quest_log[i].time - qi->time;
 		WFIFOL(fd, i*104+16) = sd->quest_log[i].time;
-		WFIFOW(fd, i*104+20) = quest_db[sd->quest_index[i]].num_objectives;
+		WFIFOW(fd, i*104+20) = qi->num_objectives;
 
-		for( j = 0 ; j < quest_db[sd->quest_index[i]].num_objectives; j++ ) {
-			WFIFOL(fd, i*104+22+j*30) = quest_db[sd->quest_index[i]].mob[j];
+		for( j = 0 ; j < qi->num_objectives; j++ ) {
+			WFIFOL(fd, i*104+22+j*30) = qi->mob[j];
 			WFIFOW(fd, i*104+26+j*30) = sd->quest_log[i].count[j];
-			mob = mob_db(quest_db[sd->quest_index[i]].mob[j]);
-			memcpy(WFIFOP(fd, i*104+28+j*30), mob?mob->jname:"NULL", NAME_LENGTH);
+			mob = mob_db(qi->mob[j]);
+			memcpy(WFIFOP(fd, i*104+28+j*30), mob->jname, NAME_LENGTH);
 		}
 	}
 
@@ -15244,25 +15246,26 @@ void clif_quest_send_mission(struct map_session_data * sd)
 
 /// Notification about a new quest (ZC_ADD_QUEST).
 /// 02b3 <quest id>.L <active>.B <start time>.L <expire time>.L <mobs>.W { <mob id>.L <mob count>.W <mob name>.24B }*3
-void clif_quest_add(struct map_session_data * sd, struct quest * qd, int index)
+void clif_quest_add(struct map_session_data * sd, struct quest * qd)
 {
 	int fd = sd->fd;
 	int i;
 	struct mob_db *mob;
+	struct quest_db *qi = quest_db(qd->quest_id);
 
 	WFIFOHEAD(fd, packet_len(0x2b3));
 	WFIFOW(fd, 0) = 0x2b3;
 	WFIFOL(fd, 2) = qd->quest_id;
 	WFIFOB(fd, 6) = qd->state;
-	WFIFOB(fd, 7) = qd->time - quest_db[index].time;
+	WFIFOB(fd, 7) = qi->time;
 	WFIFOL(fd, 11) = qd->time;
-	WFIFOW(fd, 15) = quest_db[index].num_objectives;
+	WFIFOW(fd, 15) = qi->num_objectives;
 
-	for( i = 0; i < quest_db[index].num_objectives; i++ ) {
-		WFIFOL(fd, i*30+17) = quest_db[index].mob[i];
+	for( i = 0; i < qi->num_objectives; i++ ) {
+		WFIFOL(fd, i*30+17) = qi->mob[i];
 		WFIFOW(fd, i*30+21) = qd->count[i];
-		mob = mob_db(quest_db[index].mob[i]);
-		memcpy(WFIFOP(fd, i*30+23), mob?mob->jname:"NULL", NAME_LENGTH);
+		mob = mob_db(qi->mob[i]);
+		memcpy(WFIFOP(fd, i*30+23), mob->jname, NAME_LENGTH);
 	}
 
 	WFIFOSET(fd, packet_len(0x2b3));
@@ -15284,21 +15287,22 @@ void clif_quest_delete(struct map_session_data * sd, int quest_id)
 
 /// Notification of an update to the hunting mission counter (ZC_UPDATE_MISSION_HUNT).
 /// 02b5 <packet len>.W <mobs>.W { <quest id>.L <mob id>.L <total count>.W <current count>.W }*3
-void clif_quest_update_objective(struct map_session_data * sd, struct quest * qd, int index)
+void clif_quest_update_objective(struct map_session_data * sd, struct quest * qd)
 {
 	int fd = sd->fd;
 	int i;
-	int len = quest_db[index].num_objectives*12+6;
+	struct quest_db *qi = quest_db(qd->quest_id);
+	int len = qi->num_objectives * 12 + 6;
 
 	WFIFOHEAD(fd, len);
 	WFIFOW(fd, 0) = 0x2b5;
 	WFIFOW(fd, 2) = len;
-	WFIFOW(fd, 4) = quest_db[index].num_objectives;
+	WFIFOW(fd, 4) = qi->num_objectives;
 
-	for( i = 0; i < quest_db[index].num_objectives; i++ ) {
+	for( i = 0; i < qi->num_objectives; i++ ) {
 		WFIFOL(fd, i*12+6) = qd->quest_id;
-		WFIFOL(fd, i*12+10) = quest_db[index].mob[i];
-		WFIFOW(fd, i*12+14) = quest_db[index].count[i];
+		WFIFOL(fd, i*12+10) = qi->mob[i];
+		WFIFOW(fd, i*12+14) = qi->count[i];
 		WFIFOW(fd, i*12+16) = qd->count[i];
 	}
 
@@ -15308,7 +15312,8 @@ void clif_quest_update_objective(struct map_session_data * sd, struct quest * qd
 
 /// Request to change the state of a quest (CZ_ACTIVE_QUEST).
 /// 02b6 <quest id>.L <active>.B
-void clif_parse_questStateAck(int fd, struct map_session_data * sd){
+void clif_parse_questStateAck(int fd, struct map_session_data * sd)
+{
 	struct s_packet_db* info = &packet_db[sd->packet_ver][RFIFOW(fd,0)];
 	quest_update_status(sd, RFIFOL(fd,info->pos[0]),
 	    RFIFOB(fd,info->pos[1])?Q_ACTIVE:Q_INACTIVE);

src/map/clif.h

@@ -47,7 +47,7 @@ enum e_packet_ack {
 	ZC_PERSONAL_INFOMATION,
 	ZC_PERSONAL_INFOMATION_CHN,
 	ZC_CLEAR_DIALOG,
-	//add otehr here
+	//add other here
 	MAX_ACK_FUNC //auto upd len
 };
 
@@ -692,10 +692,10 @@ void clif_msg_skill(struct map_session_data* sd, uint16 skill_id, int msg_id);
 //quest system [Kevin] [Inkfish]
 void clif_quest_send_list(struct map_session_data * sd);
 void clif_quest_send_mission(struct map_session_data * sd);
-void clif_quest_add(struct map_session_data * sd, struct quest * qd, int index);
+void clif_quest_add(struct map_session_data * sd, struct quest * qd);
 void clif_quest_delete(struct map_session_data * sd, int quest_id);
 void clif_quest_update_status(struct map_session_data * sd, int quest_id, bool active);
-void clif_quest_update_objective(struct map_session_data * sd, struct quest * qd, int index);
+void clif_quest_update_objective(struct map_session_data * sd, struct quest * qd);
 void clif_quest_show_event(struct map_session_data *sd, struct block_list *bl, short state, short color);
 void clif_displayexp(struct map_session_data *sd, unsigned int exp, char type, bool quest);
 

src/map/intif.c

@@ -1408,54 +1408,73 @@ QUESTLOG SYSTEM FUNCTIONS
 
 ***************************************/
 
-int intif_request_questlog(TBL_PC *sd)
+/**
+ * Requests a character's quest log entries to the inter server.
+ *
+ * @param sd Character's data
+ */
+void intif_request_questlog(TBL_PC *sd)
 {
 	WFIFOHEAD(inter_fd,6);
 	WFIFOW(inter_fd,0) = 0x3060;
 	WFIFOL(inter_fd,2) = sd->status.char_id;
 	WFIFOSET(inter_fd,6);
-	return 0;
 }
 
-int intif_parse_questlog(int fd)
+void intif_parse_questlog(int fd)
 {
-	int char_id = RFIFOL(fd, 4);
-	int i;
-	TBL_PC * sd = map_charid2sd(char_id);
-
-	//User not online anymore
-	if(!sd)
-		return -1;
-
-	sd->avail_quests = sd->num_quests = (RFIFOW(fd, 2)-8)/sizeof(struct quest);
+	int char_id = RFIFOL(fd,4), num_received = (RFIFOW(fd,2) - 8) / sizeof(struct quest);
+	TBL_PC *sd = map_charid2sd(char_id);
 
-	memset(&sd->quest_log, 0, sizeof(sd->quest_log));
-
-	for( i = 0; i < sd->num_quests; i++ )
-	{
-		memcpy(&sd->quest_log[i], RFIFOP(fd, i*sizeof(struct quest)+8), sizeof(struct quest));
+	if(!sd) // User not online anymore
+		return;
 
-		sd->quest_index[i] = quest_search_db(sd->quest_log[i].quest_id);
+	sd->num_quests = sd->avail_quests = 0;
 
-		if( sd->quest_index[i] < 0 )
-		{
-			ShowError("intif_parse_questlog: quest %d not found in DB.\n",sd->quest_log[i].quest_id);
-			sd->avail_quests--;
-			sd->num_quests--;
-			i--;
-			continue;
+	if(num_received == 0) {
+		if(sd->quest_log) {
+			aFree(sd->quest_log);
+			sd->quest_log = NULL;
 		}
+	} else {
+		struct quest *received = (struct quest *)RFIFOP(fd,8);
+		int i, k = num_received;
 
-		if( sd->quest_log[i].state == Q_COMPLETE )
-			sd->avail_quests--;
+		if(sd->quest_log)
+			RECREATE(sd->quest_log, struct quest, num_received);
+		else
+			CREATE(sd->quest_log, struct quest, num_received);
+
+		for(i = 0; i < num_received; i++) {
+			if(quest_db(received[i].quest_id) == &quest_dummy) {
+				ShowError("intif_parse_QuestLog: quest %d not found in DB.\n", received[i].quest_id);
+				continue;
+			}
+			if(received[i].state != Q_COMPLETE) // Insert at the beginning
+				memcpy(&sd->quest_log[sd->avail_quests++], &received[i], sizeof(struct quest));
+			else // Insert at the end
+				memcpy(&sd->quest_log[--k], &received[i], sizeof(struct quest));
+			sd->num_quests++;
+		}
+		if(sd->avail_quests < k) {
+			// sd->avail_quests and k didn't meet in the middle: some entries were skipped
+			if(k < num_received) // Move the entries at the end to fill the gap
+				memmove(&sd->quest_log[k], &sd->quest_log[sd->avail_quests], sizeof(struct quest) * (num_received - k));
+			sd->quest_log = aRealloc(sd->quest_log, sizeof(struct quest) * sd->num_quests);
+		}
 	}
 
 	quest_pc_login(sd);
-
-	return 0;
 }
 
-int intif_parse_questsave(int fd)
+/**
+ * Parses the quest log save ack for a character from the inter server.
+ *
+ * Received in reply to the requests made by intif_quest_save.
+ *
+ * @see intif_parse
+ */
+void intif_parse_questsave(int fd)
 {
 	int cid = RFIFOL(fd, 2);
 	TBL_PC *sd = map_id2sd(cid);
@@ -1464,25 +1483,27 @@ int intif_parse_questsave(int fd)
 		ShowError("intif_parse_questsave: Failed to save quest(s) for character %d!\n", cid);
 	else if( sd )
 		sd->save_quest = false;
-
-	return 0;
 }
 
+/**
+ * Requests to the inter server to save a character's quest log entries.
+ *
+ * @param sd Character's data
+ * @return 0 in case of success, nonzero otherwise
+ */
 int intif_quest_save(TBL_PC *sd)
 {
-	int len;
+	int len = sizeof(struct quest) * sd->num_quests + 8;
 
 	if(CheckForCharServer())
-		return 0;
-
-	len = sizeof(struct quest)*sd->num_quests + 8;
+		return 1;
 
 	WFIFOHEAD(inter_fd, len);
 	WFIFOW(inter_fd,0) = 0x3061;
 	WFIFOW(inter_fd,2) = len;
 	WFIFOL(inter_fd,4) = sd->status.char_id;
 	if( sd->num_quests )
-		memcpy(WFIFOP(inter_fd,8), &sd->quest_log, sizeof(struct quest)*sd->num_quests);
+		memcpy(WFIFOP(inter_fd,8), sd->quest_log, sizeof(struct quest)*sd->num_quests);
 	WFIFOSET(inter_fd,  len);
 
 	return 0;

src/map/intif.h

@@ -80,7 +80,7 @@ int intif_homunculus_requestsave(int account_id, struct s_homunculus* sh);
 int intif_homunculus_requestdelete(int homun_id);
 
 /******QUEST SYTEM*******/
-int intif_request_questlog(struct map_session_data * sd);
+void intif_request_questlog(struct map_session_data * sd);
 int intif_quest_save(struct map_session_data * sd);
 
 // MERCENARY SYSTEM

src/map/map.c

@@ -3832,6 +3832,7 @@ void do_final(void)
 	do_final_chrif();
 	do_final_clif();
 	do_final_npc();
+	do_final_quest();
 	do_final_script();
 	do_final_instance();
 	do_final_itemdb();

src/map/pc.c

@@ -1101,6 +1101,11 @@ bool pc_authok(struct map_session_data *sd, int login_id2, time_t expiration_tim
 	for( i = 0; i < 3; i++ )
 		sd->hate_mob[i] = -1;
 
+	sd->quest_log = NULL;
+	sd->num_quests = 0;
+	sd->avail_quests = 0;
+	sd->save_quest = false;
+
 	//warp player
 	if ((i=pc_setpos(sd,sd->status.last_point.map, sd->status.last_point.x, sd->status.last_point.y, CLR_OUTSIGHT)) != 0) {
 		ShowError ("Last_point_map %s - id %d not found (error code %d)\n", mapindex_id2name(sd->status.last_point.map), sd->status.last_point.map, i);
@@ -6275,7 +6280,7 @@ int pc_maxparameterincrease(struct map_session_data* sd, int type)
 	}
 	final_val--;
 
-	return final_val > base ? final_val-base : 0;
+	return (final_val > base ? final_val-base : 0);
 }
 
 /**
@@ -6309,7 +6314,7 @@ bool pc_statusup(struct map_session_data* sd, int type, int increase)
 		clif_statusupack(sd, type, 0, 0);
 		return false;
 	}
-	
+
 	// check status points
 	needed_points = pc_need_status_point(sd, type, increase);
 	if (needed_points < 0 || needed_points > sd->status.status_point) { // Sanity check

src/map/pc.h

@@ -507,12 +507,11 @@ struct map_session_data {
 		bool changed; // if true, should sync with charserver on next mailbox request
 	} mail;
 
-	//Quest log system [Kevin] [Inkfish]
-	int num_quests;
-	int avail_quests;
-	int quest_index[MAX_QUEST_DB];
-	struct quest quest_log[MAX_QUEST_DB];
-	bool save_quest;
+	//Quest log system
+	int num_quests;          ///< Number of entries in quest_log
+	int avail_quests;        ///< Number of Q_ACTIVE and Q_INACTIVE entries in quest log (index of the first Q_COMPLETE entry)
+	struct quest *quest_log; ///< Quest log entries (note: Q_COMPLETE quests follow the first <avail_quests>th enties
+	bool save_quest;         ///< Whether the quest_log entries were modified and are waitin to be saved
 
 	// temporary debug [flaviojs]
 	const char* debug_file;

src/map/quest.c

@@ -32,80 +32,84 @@
 #include <stdarg.h>
 #include <time.h>
 
-
-struct s_quest_db quest_db[MAX_QUEST_DB];
-
-
-int quest_search_db(int quest_id)
-{
-	int i;
-
-	ARR_FIND(0, MAX_QUEST_DB,i,quest_id == quest_db[i].id);
-	if( i == MAX_QUEST_DB )
-		return -1;
-
-	return i;
+/**
+ * Searches a quest by ID.
+ *
+ * @param quest_id ID to lookup
+ * @return Quest entry (equals to &quest_dummy if the ID is invalid)
+ */
+struct quest_db *quest_db(int quest_id) {
+  if( quest_id < 0 || quest_id > MAX_QUEST_DB || quest_db_data[quest_id] == NULL )
+    return &quest_dummy;
+  return quest_db_data[quest_id];
 }
 
-//Send quest info on login
-int quest_pc_login(TBL_PC * sd)
-{
+/**
+ * Sends quest info to the player on login.
+ *
+ * @param sd Player's data
+ * @return 0 in case of success, nonzero otherwise (i.e. the player has no quests)
+ */
+int quest_pc_login(TBL_PC *sd) {
 	int i;
 
-	if(sd->avail_quests == 0)
+	if( sd->avail_quests == 0 )
 		return 1;
 
 	clif_quest_send_list(sd);
 	clif_quest_send_mission(sd);
-	
-	for( i = 0; i < sd->avail_quests; i++ ){
-		clif_quest_update_objective(sd, &sd->quest_log[i], sd->quest_index[i]);
-	}
+
+	//@TODO[Haru]: Is this necessary? Does quest_send_mission not take care of this?
+	for( i = 0; i < sd->avail_quests; i++ )
+		clif_quest_update_objective(sd, &sd->quest_log[i]);
 
 	return 0;
 }
 
-int quest_add(TBL_PC * sd, int quest_id)
-{
-
-	int i, j;
-
-	if( sd->num_quests >= MAX_QUEST_DB )
-	{
-		ShowError("quest_add: Character %d has got all the quests.(max quests: %d)\n", sd->status.char_id, MAX_QUEST_DB);
-		return 1;
-	}
-
-	if( quest_check(sd, quest_id, HAVEQUEST) >= 0 )
-	{
-		ShowError("quest_add: Character %d already has quest %d.\n", sd->status.char_id, quest_id);
+/**
+ * Adds a quest to the player's list.
+ *
+ * New quest will be added as Q_ACTIVE.
+ *
+ * @param sd       Player's data
+ * @param quest_id ID of the quest to add.
+ * @return 0 in case of success, nonzero otherwise
+ */
+int quest_add(TBL_PC *sd, int quest_id) {
+	int n;
+	struct quest_db *qi = quest_db(quest_id);
+
+	if( qi == &quest_dummy ) {
+		ShowError("quest_add: quest %d not found in DB.\n", quest_id);
 		return -1;
 	}
 
-	if( (j = quest_search_db(quest_id)) < 0 )
-	{
-		ShowError("quest_add: quest %d not found in DB.\n", quest_id);
+	if( quest_check(sd, quest_id, HAVEQUEST) >= 0 ) {
+		ShowError("quest_add: Character %d already has quest %d.\n", sd->status.char_id, quest_id);
 		return -1;
 	}
 
-	i = sd->avail_quests;
-	memmove(&sd->quest_log[i+1], &sd->quest_log[i], sizeof(struct quest)*(sd->num_quests-sd->avail_quests));
-	memmove(sd->quest_index+i+1, sd->quest_index+i, sizeof(int)*(sd->num_quests-sd->avail_quests));
-
-	memset(&sd->quest_log[i], 0, sizeof(struct quest));
-	sd->quest_log[i].quest_id = quest_db[j].id;
-	if( quest_db[j].time )
-		sd->quest_log[i].time = (unsigned int)(time(NULL) + quest_db[j].time);
-	sd->quest_log[i].state = Q_ACTIVE;
+	n = sd->avail_quests; //Insertion point
 
-	sd->quest_index[i] = j;
 	sd->num_quests++;
 	sd->avail_quests++;
-	sd->save_quest = true;
+	RECREATE(sd->quest_log, struct quest, sd->num_quests);
+
+	//The character has some completed quests, make room before them so that they will stay at the end of the array
+	if( sd->avail_quests != sd->num_quests )
+		memmove(&sd->quest_log[n + 1], &sd->quest_log[n], sizeof(struct quest) * (sd->num_quests-sd->avail_quests));
 
-	clif_quest_add(sd, &sd->quest_log[i], sd->quest_index[i]);
+	memset(&sd->quest_log[n], 0, sizeof(struct quest));
+
+	sd->quest_log[n].quest_id = qi->id;
+	if( qi->time )
+		sd->quest_log[n].time = (unsigned int)(time(NULL) + qi->time);
+	sd->quest_log[n].state = Q_ACTIVE;
+
+	sd->save_quest = true;
 
-	clif_quest_update_objective(sd, &sd->quest_log[i], sd->quest_index[i]);
+	clif_quest_add(sd, &sd->quest_log[n]);
+	clif_quest_update_objective(sd, &sd->quest_log[n]);
 
 	if( save_settings&64 )
 		chrif_save(sd,0);
@@ -113,49 +117,50 @@ int quest_add(TBL_PC * sd, int quest_id)
 	return 0;
 }
 
-int quest_change(TBL_PC * sd, int qid1, int qid2)
-{
-
-	int i, j;
+/**
+ * Replaces a quest in a player's list with another one.
+ *
+ * @param sd   Player's data
+ * @param qid1 Current quest to replace
+ * @param qid2 New quest to add
+ * @return 0 in case of success, nonzero otherwise
+ */
+int quest_change(TBL_PC *sd, int qid1, int qid2) {
+	int i;
+	struct quest_db *qi = quest_db(qid2);
 
-	if( quest_check(sd, qid2, HAVEQUEST) >= 0 )
-	{
-		ShowError("quest_change: Character %d already has quest %d.\n", sd->status.char_id, qid2);
+	if( qi == &quest_dummy ) {
+		ShowError("quest_change: quest %d not found in DB.\n", qid2);
 		return -1;
 	}
 
-	if( quest_check(sd, qid1, HAVEQUEST) < 0 )
-	{
-		ShowError("quest_change: Character %d doesn't have quest %d.\n", sd->status.char_id, qid1);
+	if( quest_check(sd, qid2, HAVEQUEST) >= 0 ) {
+		ShowError("quest_change: Character %d already has quest %d.\n", sd->status.char_id, qid2);
 		return -1;
 	}
 
-	if( (j = quest_search_db(qid2)) < 0 )
-	{
-		ShowError("quest_change: quest %d not found in DB.\n",qid2);
+	if( quest_check(sd, qid1, HAVEQUEST) < 0 ) {
+		ShowError("quest_change: Character %d doesn't have quest %d.\n", sd->status.char_id, qid1);
 		return -1;
 	}
 
 	ARR_FIND(0, sd->avail_quests, i, sd->quest_log[i].quest_id == qid1);
-	if(i == sd->avail_quests)
-	{
-		ShowError("quest_change: Character %d has completed quests %d.\n", sd->status.char_id, qid1);
+	if( i == sd->avail_quests ) {
+		ShowError("quest_change: Character %d has completed quest %d.\n", sd->status.char_id, qid1);
 		return -1;
 	}
 
 	memset(&sd->quest_log[i], 0, sizeof(struct quest));
-	sd->quest_log[i].quest_id = quest_db[j].id;
-	if( quest_db[j].time )
-		sd->quest_log[i].time = (unsigned int)(time(NULL) + quest_db[j].time);
+	sd->quest_log[i].quest_id = qi->id;
+	if( qi->time )
+		sd->quest_log[i].time = (unsigned int)(time(NULL) + qi->time);
 	sd->quest_log[i].state = Q_ACTIVE;
 
-	sd->quest_index[i] = j;
 	sd->save_quest = true;
 
 	clif_quest_delete(sd, qid1);
-	clif_quest_add(sd, &sd->quest_log[i], sd->quest_index[i]);
-
-	clif_quest_update_objective(sd, &sd->quest_log[i], sd->quest_index[i]);
+	clif_quest_add(sd, &sd->quest_log[i]);
+	clif_quest_update_objective(sd, &sd->quest_log[i]);
 
 	if( save_settings&64 )
 		chrif_save(sd,0);
@@ -163,27 +168,31 @@ int quest_change(TBL_PC * sd, int qid1, int qid2)
 	return 0;
 }
 
-int quest_delete(TBL_PC * sd, int quest_id)
-{
+/**
+ * Removes a quest from a player's list
+ *
+ * @param sd       Player's data
+ * @param quest_id ID of the quest to remove
+ * @return 0 in case of success, nonzero otherwise
+ */
+int quest_delete(TBL_PC *sd, int quest_id) {
 	int i;
 
 	//Search for quest
 	ARR_FIND(0, sd->num_quests, i, sd->quest_log[i].quest_id == quest_id);
-	if(i == sd->num_quests)
-	{
+	if( i == sd->num_quests ) {
 		ShowError("quest_delete: Character %d doesn't have quest %d.\n", sd->status.char_id, quest_id);
 		return -1;
 	}
-
 	if( sd->quest_log[i].state != Q_COMPLETE )
 		sd->avail_quests--;
-	if( sd->num_quests-- < MAX_QUEST_DB && sd->quest_log[i+1].quest_id )
-	{
-		memmove(&sd->quest_log[i], &sd->quest_log[i+1], sizeof(struct quest)*(sd->num_quests-i));
-		memmove(sd->quest_index+i, sd->quest_index+i+1, sizeof(int)*(sd->num_quests-i));
-	}
-	memset(&sd->quest_log[sd->num_quests], 0, sizeof(struct quest));
-	sd->quest_index[sd->num_quests] = 0;
+	if( i < --sd->num_quests ) //Compact the array
+		memmove(&sd->quest_log[i], &sd->quest_log[i + 1], sizeof(struct quest) * (sd->num_quests - i));
+	if( sd->num_quests == 0 ) {
+		aFree(sd->quest_log);
+		sd->quest_log = NULL;
+	} else
+		RECREATE(sd->quest_log, struct quest, sd->num_quests);
 	sd->save_quest = true;
 
 	clif_quest_delete(sd, quest_id);
@@ -194,9 +203,16 @@ int quest_delete(TBL_PC * sd, int quest_id)
 	return 0;
 }
 
-int quest_update_objective_sub(struct block_list *bl, va_list ap)
-{
-	struct map_session_data * sd;
+/**
+ * Map iterator subroutine to update quest objectives for a party after killing a monster.
+ *
+ * @see map_foreachinrange
+ * @param ap Argument list, expecting:
+ *           int Party ID
+ *           int Mob ID
+ */
+int quest_update_objective_sub(struct block_list *bl, va_list ap) {
+	struct map_session_data *sd;
 	int mob, party;
 
 	nullpo_ret(bl);
@@ -215,29 +231,48 @@ int quest_update_objective_sub(struct block_list *bl, va_list ap)
 	return 1;
 }
 
-
-void quest_update_objective(TBL_PC * sd, int mob) {
-	int i,j;
+/**
+ * Updates the quest objectives for a character after killing a monster.
+ *
+ * @param sd     Character's data
+ * @param mob_id Monster ID
+ */
+void quest_update_objective(TBL_PC *sd, int mob) {
+	int i, j;
 
 	for( i = 0; i < sd->avail_quests; i++ ) {
-		if( sd->quest_log[i].state != Q_ACTIVE )
+		struct quest_db *qi = NULL;
+
+		if( sd->quest_log[i].state != Q_ACTIVE ) // Skip inactive quests
 			continue;
 
-		for( j = 0; j < MAX_QUEST_OBJECTIVES; j++ )
-			if( quest_db[sd->quest_index[i]].mob[j] == mob && sd->quest_log[i].count[j] < quest_db[sd->quest_index[i]].count[j] )  {
+		qi = quest_db(sd->quest_log[i].quest_id);
+
+		for( j = 0; j < qi->num_objectives; j++ ) {
+			if( qi->mob[j] == mob && sd->quest_log[i].count[j] < qi->count[j] )  {
 				sd->quest_log[i].count[j]++;
 				sd->save_quest = true;
-				clif_quest_update_objective(sd,&sd->quest_log[i],sd->quest_index[i]);
+				clif_quest_update_objective(sd, &sd->quest_log[i]);
 			}
+		}
 	}
 }
 
-int quest_update_status(TBL_PC * sd, int quest_id, quest_state status) {
+/**
+ * Updates a quest's state.
+ *
+ * Only status of active and inactive quests can be updated. Completed quests can't (for now). [Inkfish]
+ *
+ * @param sd       Character's data
+ * @param quest_id Quest ID to update
+ * @param qs       New quest state
+ * @return 0 in case of success, nonzero otherwise
+ */
+int quest_update_status(TBL_PC *sd, int quest_id, enum quest_state status) {
 	int i;
 
-	//Only status of active and inactive quests can be updated. Completed quests can't (for now). [Inkfish]
 	ARR_FIND(0, sd->avail_quests, i, sd->quest_log[i].quest_id == quest_id);
-	if(i == sd->avail_quests) {
+	if( i == sd->avail_quests ) {
 		ShowError("quest_update_status: Character %d doesn't have quest %d.\n", sd->status.char_id, quest_id);
 		return -1;
 	}
@@ -246,15 +281,17 @@ int quest_update_status(TBL_PC * sd, int quest_id, quest_state status) {
 	sd->save_quest = true;
 
 	if( status < Q_COMPLETE ) {
-		clif_quest_update_status(sd, quest_id, (bool)status);
+		clif_quest_update_status(sd, quest_id, status == Q_ACTIVE ? true : false);
 		return 0;
 	}
 
-	if( i != (--sd->avail_quests) ) {
+	// The quest is complete, so it needs to be moved to the completed quests block at the end of the array.
+	if( i < (--sd->avail_quests) ) {
 		struct quest tmp_quest;
-		memcpy(&tmp_quest, &sd->quest_log[i],sizeof(struct quest));
-		memcpy(&sd->quest_log[i], &sd->quest_log[sd->avail_quests],sizeof(struct quest));
-		memcpy(&sd->quest_log[sd->avail_quests], &tmp_quest,sizeof(struct quest));
+
+		memcpy(&tmp_quest, &sd->quest_log[i], sizeof(struct quest));
+		memcpy(&sd->quest_log[i], &sd->quest_log[sd->avail_quests], sizeof(struct quest));
+		memcpy(&sd->quest_log[sd->avail_quests], &tmp_quest, sizeof(struct quest));
 	}
 
 	clif_quest_delete(sd, quest_id);
@@ -265,7 +302,22 @@ int quest_update_status(TBL_PC * sd, int quest_id, quest_state status) {
 	return 0;
 }
 
-int quest_check(TBL_PC * sd, int quest_id, quest_check_type type) {
+/**
+ * Queries quest information for a character.
+ *
+ * @param sd       Character's data
+ * @param quest_id Quest ID
+ * @param type     Check type
+ * @return -1 if the quest was not found, otherwise it depends on the type:
+ *         HAVEQUEST: The quest's state
+ *         PLAYTIME:  2 if the quest's timeout has expired
+ *                    1 if the quest was completed
+ *                    0 otherwise
+ *         HUNTING:   2 if the quest has not been marked as completed yet, and its objectives have been fulfilled
+ *                    1 if the quest's timeout has expired
+ *                    0 otherwise
+ */
+int quest_check(TBL_PC *sd, int quest_id, enum quest_check_type type) {
 	int i;
 
 	ARR_FIND(0, sd->num_quests, i, sd->quest_log[i].quest_id == quest_id);
@@ -277,18 +329,18 @@ int quest_check(TBL_PC * sd, int quest_id, quest_check_type type) {
 			return sd->quest_log[i].state;
 		case PLAYTIME:
 			return (sd->quest_log[i].time < (unsigned int)time(NULL) ? 2 : sd->quest_log[i].state == Q_COMPLETE ? 1 : 0);
-		case HUNTING: {
-				if( sd->quest_log[i].state == 0 || sd->quest_log[i].state == 1 ) {
-					int j;
-					ARR_FIND(0, MAX_QUEST_OBJECTIVES, j, sd->quest_log[i].count[j] < quest_db[sd->quest_index[i]].count[j]);
-					if( j == MAX_QUEST_OBJECTIVES )
-						return 2;
-					if( sd->quest_log[i].time > 0 && sd->quest_log[i].time < (unsigned int)time(NULL) )
-						return 1;
-					return 0;
-				} else
-					return 0;
+		case HUNTING:
+			if( sd->quest_log[i].state == Q_INACTIVE || sd->quest_log[i].state == Q_ACTIVE ) {
+				int j;
+				struct quest_db *qi = quest_db(sd->quest_log[i].quest_id);
+
+				ARR_FIND(0, MAX_QUEST_OBJECTIVES, j, sd->quest_log[i].count[j] < qi->count[j]);
+				if( j == MAX_QUEST_OBJECTIVES )
+					return 2;
+				if( sd->quest_log[i].time < (unsigned int)time(NULL) )
+					return 1;
 			}
+			return 0;
 		default:
 			ShowError("quest_check_quest: Unknown parameter %d",type);
 			break;
@@ -297,79 +349,156 @@ int quest_check(TBL_PC * sd, int quest_id, quest_check_type type) {
 	return -1;
 }
 
+/**
+ * Loads quests from the quest db.
+ *
+ * @return Number of loaded quests, or -1 if the file couldn't be read.
+ */
 int quest_read_db(void) {
-	const char* dbsubpath[] = {
-		"",
-		DBIMPORT"/",
-	};
-	int f;
-
-	for(f=0; f<ARRAYLENGTH(dbsubpath); f++){
-		FILE *fp;
-		char line[1024];
-		int i,j,k = 0;
-		char *str[20],*p,*np;
-		char filename[256];
-	
-		sprintf(filename, "%s/%s%s", db_path,dbsubpath[f],"quest_db.txt");
-		if( (fp=fopen(filename,"r"))==NULL ){
-			if(f==0) ShowError("can't read %s\n", filename);
-			return -1;
-		}
+	//@TODO[Haru]: This duplicates some sv_readdb functionalities, and it would be
+	//nice if it could be replaced by it. The reason why it wasn't is probably
+	//because we need to accept commas (which is also used as delimiter) in the
+	//last field (quest name), and sv_readdb isn't capable of doing so.
+	FILE *fp;
+	char line[1024];
+	int i, count = 0;
+	char *str[20], *p, *np;
+	struct quest_db entry;
+
+	sprintf(line, "%s/quest_db.txt", db_path);
+	if( (fp = fopen(line, "r")) == NULL ) {
+		ShowError("can't read %s\n", line);
+		return -1;
+	}
 
-		while(fgets(line, sizeof(line), fp)) {
-			if (k == MAX_QUEST_DB) {
-				ShowError("quest_read_db: Too many entries specified in %s/quest_db.txt!\n", db_path);
+	while( fgets(line, sizeof(line), fp) ) {
+		if( line[0] == '/' && line[1] == '/' )
+			continue;
+		memset(str, 0, sizeof(str));
+
+		for( i = 0, p = line; i < 8; i++ ) {
+			if( (np = strchr(p, ',')) != NULL ) {
+				str[i] = p;
+				*np = 0;
+				p = np + 1;
+			} else if( str[0] == NULL )
 				break;
-			}
-			if(line[0]=='/' && line[1]=='/')
+			else {
+				ShowError("quest_read_db: insufficient columns in line %s\n", line);
 				continue;
-			memset(str,0,sizeof(str));
-
-			for( j = 0, p = line; j < 8; j++ ) {
-				if( ( np = strchr(p,',') ) != NULL ) {
-					str[j] = p;
-					*np = 0;
-					p = np + 1;
-				}
-				else if (str[0] == NULL)
-					continue;
-				else {
-					ShowError("quest_read_db: insufficient columns in line %s\n", line);
-					continue;
-				}
 			}
-			if(str[0]==NULL)
-				continue;
+		}
+		if( str[0] == NULL )
+			continue;
 
-			memset(&quest_db[k], 0, sizeof(quest_db[0]));
+		memset(&entry, 0, sizeof(entry));
 
-			quest_db[k].id = atoi(str[0]);
-			quest_db[k].time = atoi(str[1]);
+		entry.id = atoi(str[0]);
 
-			for( i = 0; i < MAX_QUEST_OBJECTIVES; i++ ) {
-				quest_db[k].mob[i] = atoi(str[2*i+2]);
-				quest_db[k].count[i] = atoi(str[2*i+3]);
+		if( entry.id < 0 || entry.id >= MAX_QUEST_DB ) {
+			ShowError("quest_read_db: Invalid quest ID '%d' in line '%s' (min: 0, max: %d.)\n", entry.id, line, MAX_QUEST_DB);
+			continue;
+		}
 
-				if( !quest_db[k].mob[i] || !quest_db[k].count[i] )
-					break;
-			}
+		entry.time = atoi(str[1]);
 
-			quest_db[k].num_objectives = i;
+		for( i = 0; i < MAX_QUEST_OBJECTIVES; i++ ) {
+			entry.mob[i] = atoi(str[2 * i + 2]);
+			entry.count[i] = atoi(str[2 * i + 3]);
 
-			k++;
+			if( !entry.mob[i] || !entry.count[i] )
+				break;
 		}
-		fclose(fp);
-		ShowStatus("Done reading '"CL_WHITE"%d"CL_RESET"' entries in '"CL_WHITE"%s"CL_RESET"'.\n", k, filename);
+
+		entry.num_objectives = i;
+
+		if( quest_db_data[entry.id] == NULL )
+			quest_db_data[entry.id] = aMalloc(sizeof(struct quest_db));
+
+		memcpy(quest_db_data[entry.id], &entry, sizeof(struct quest_db));
+		count++;
 	}
+
+	fclose(fp);
+	ShowStatus("Done reading '"CL_WHITE"%d"CL_RESET"' entries in '"CL_WHITE"%s"CL_RESET"'.\n", count, "quest_db.txt");
 	return 0;
 }
 
+/**
+ * Map iterator to ensures a player has no invalid quest log entries.
+ *
+ * Any entries that are no longer in the db are removed.
+ *
+ * @see map_foreachpc
+ * @param ap Ignored
+ */
+int quest_reload_check_sub(struct map_session_data *sd, va_list ap) {
+	int i, j;
+
+	nullpo_ret(sd);
+
+	j = 0;
+	for( i = 0; i < sd->num_quests; i++ ) {
+		struct quest_db *qi = quest_db(sd->quest_log[i].quest_id);
+
+		if( qi == &quest_dummy ) { //Remove no longer existing entries
+			if( sd->quest_log[i].state != Q_COMPLETE ) //And inform the client if necessary
+				clif_quest_delete(sd, sd->quest_log[i].quest_id);
+			continue;
+		}
+		if( i != j ) {
+			//Move entries if there's a gap to fill
+			memcpy(&sd->quest_log[j], &sd->quest_log[i], sizeof(struct quest));
+		}
+		j++;
+	}
+	sd->num_quests = j;
+	ARR_FIND(0, sd->num_quests, i, sd->quest_log[i].state == Q_COMPLETE);
+	sd->avail_quests = i;
+
+	return 1;
+}
+
+/**
+ * Clears the quest database for shutdown or reload.
+ */
+void quest_clear_db(void) {
+	int i;
+
+	for( i = 0; i < MAX_QUEST_DB; i++ ) {
+		if( quest_db_data[i] ) {
+			aFree(quest_db_data[i]);
+			quest_db_data[i] = NULL;
+		}
+	}
+}
+
+/**
+ * Initializes the quest interface.
+ */
 void do_init_quest(void) {
 	quest_read_db();
 }
 
+/**
+ * Finalizes the quest interface before shutdown.
+ */
+void do_final_quest(void) {
+	memset(&quest_dummy, 0, sizeof(quest_dummy));
+
+	quest_clear_db();
+}
+
+/**
+ * Reloads the quest database.
+ */
 void do_reload_quest(void) {
-	memset(&quest_db, 0, sizeof(quest_db));
+	memset(&quest_dummy, 0, sizeof(quest_dummy));
+
+	quest_clear_db();
+
 	quest_read_db();
+
+	//Update quest data for players, to ensure no entries about removed quests are left over.
+	map_foreachpc(&quest_reload_check_sub);
 }

src/map/quest.h

@@ -4,7 +4,9 @@
 #ifndef _QUEST_H_
 #define _QUEST_H_
 
-struct s_quest_db {
+#define MAX_QUEST_DB (62238 + 1) // Highest quest ID + 1
+
+struct quest_db {
 	int id;
 	unsigned int time;
 	int mob[MAX_QUEST_OBJECTIVES];
@@ -12,23 +14,32 @@ struct s_quest_db {
 	int num_objectives;
 	//char name[NAME_LENGTH];
 };
-extern struct s_quest_db quest_db[MAX_QUEST_DB];
 
-typedef enum quest_check_type { HAVEQUEST, PLAYTIME, HUNTING } quest_check_type;
+struct quest_db *quest_db_data[MAX_QUEST_DB];	///< Quest database
+struct quest_db quest_dummy;					///< Dummy entry for invalid quest lookups
+
+// Questlog check types
+enum quest_check_type {
+	HAVEQUEST, ///< Query the state of the given quest
+	PLAYTIME,  ///< Check if the given quest has been completed or has yet to expire
+	HUNTING,   ///< Check if the given hunting quest's requirements have been met
+};
 
-int quest_pc_login(TBL_PC * sd);
+int quest_pc_login(TBL_PC *sd);
 
 int quest_add(TBL_PC * sd, int quest_id);
 int quest_delete(TBL_PC * sd, int quest_id);
 int quest_change(TBL_PC * sd, int qid1, int qid2);
 int quest_update_objective_sub(struct block_list *bl, va_list ap);
 void quest_update_objective(TBL_PC * sd, int mob);
-int quest_update_status(TBL_PC * sd, int quest_id, quest_state status);
-int quest_check(TBL_PC * sd, int quest_id, quest_check_type type);
+int quest_update_status(TBL_PC * sd, int quest_id, enum quest_state status);
+int quest_check(TBL_PC * sd, int quest_id, enum quest_check_type type);
+void quest_clear(void);
 
-int quest_search_db(int quest_id);
+struct quest_db *quest_db(int quest_id);
 
-void do_init_quest();
+void do_init_quest(void);
+void do_final_quest(void);
 void do_reload_quest(void);
 
 #endif

src/map/script.c

@@ -16723,10 +16723,11 @@ BUILDIN_FUNC(questinfo)
 BUILDIN_FUNC(setquest)
 {
 	struct map_session_data *sd = script_rid2sd(st);
-	int i, quest_id;
+	unsigned short i;
+	int quest_id;
+
+	nullpo_retr(1, sd);
 
-	nullpo_ret(sd);
-	
 	quest_id = script_getnum(st, 2);
 
 	quest_add(sd, quest_id);
@@ -16775,12 +16776,12 @@ BUILDIN_FUNC(changequest)
 BUILDIN_FUNC(checkquest)
 {
 	struct map_session_data *sd = script_rid2sd(st);
-	quest_check_type type = HAVEQUEST;
+	enum quest_check_type type = HAVEQUEST;
 
 	nullpo_ret(sd);
 
 	if( script_hasdata(st, 3) )
-		type = (quest_check_type)script_getnum(st, 3);
+		type = (enum quest_check_type)script_getnum(st, 3);
 
 	script_pushint(st, quest_check(sd, script_getnum(st, 2), type));
 
@@ -16794,7 +16795,7 @@ BUILDIN_FUNC(isbegin_quest)
 
 	nullpo_ret(sd);
 
-	i = quest_check(sd, script_getnum(st, 2), (quest_check_type) HAVEQUEST);
+	i = quest_check(sd, script_getnum(st, 2), (enum quest_check_type) HAVEQUEST);
 	script_pushint(st, i + (i < 1));
 
 	return SCRIPT_CMD_SUCCESS;

src/map/unit.c

@@ -2659,6 +2659,11 @@ int unit_free(struct block_list *bl, clr_type clrtype)
 				aFree(sd->sc_display);
 				sd->sc_display = NULL;
 			}
+			if( sd->quest_log != NULL ) {
+				aFree(sd->quest_log);
+				sd->quest_log = NULL;
+				sd->num_quests = sd->avail_quests = 0;
+			}
 			break;
 		}
 		case BL_PET: