Questinfo correction (#5523)

* Corrected questinfo bubble not always being displayed
* Docs updated
* Changed qi_display to std::vector

Co-authored-by: Lemongrass3110 <lemongrass@kstp.at>

Thanks to @lighta @aleos89 @Lemongrass3110 !

doc/script_commands.txt

@@ -9260,7 +9260,7 @@ Examples:
 
 *questinfo <Icon>{,<Map Mark Color>{,"<condition>"}};
 
-This command should only be used in an OnInit label.
+This command should only be used in OnInit/OnInstanceInit labels.
 Show an emotion on top of a NPC, and optionally, a colored mark in the mini-map like "viewpoint".
 When a user is doing some action, each NPC is checked for questinfo that has been set on the map.
 If questinfo is present, it will check if the player fulfill the condition.

src/map/map.cpp

@@ -3691,10 +3691,6 @@ void map_data_copy(struct map_data *dst_map, struct map_data *src_map) {
 	dst_map->skill_duration.insert(src_map->skill_duration.begin(), src_map->skill_duration.end());
 
 	dst_map->zone = src_map->zone;
-
-	// Mimic questinfo
-	if (!src_map->qi_data.empty())
-		src_map->qi_data = dst_map->qi_data;
 }
 
 /**
@@ -4343,29 +4339,27 @@ int log_sql_init(void)
 
 void map_remove_questinfo(int m, struct npc_data *nd) {
 	struct map_data *mapdata = map_getmapdata(m);
-	struct s_questinfo *qi;
 
 	nullpo_retv(nd);
 	nullpo_retv(mapdata);
 
-	for (int i = 0; i < mapdata->qi_data.size(); i++) {
+	util::vector_erase_if_exists(mapdata->qi_npc, nd->bl.id);
-		qi = &mapdata->qi_data[i];
+	nd->qi_data.clear();
-		if (qi && qi->nd == nd) {
-			script_free_code(qi->condition);
-			mapdata->qi_data.erase(mapdata->qi_data.begin() + i);
-		}
-	}
 }
 
 static void map_free_questinfo(struct map_data *mapdata) {
 	nullpo_retv(mapdata);
 
-	for (const auto &it : mapdata->qi_data) {
+	for (const auto &it : mapdata->qi_npc) {
-		if (it.condition)
+		struct npc_data *nd = map_id2nd(it);
-			script_free_code(it.condition);
+
+		if (!nd || nd->qi_data.empty())
+			continue;
+
+		nd->qi_data.clear();
 	}
 
-	mapdata->qi_data.clear();
+	mapdata->qi_npc.clear();
 }
 
 /**
@@ -4876,6 +4870,10 @@ void do_final(void){
 		map_quit(sd);
 	mapit_free(iter);
 
+	for (int i = 0; i < map_num; i++) {
+		map_free_questinfo(map_getmapdata(i));
+	}
+
 	/* prepares npcs for a faster shutdown process */
 	do_clear_npc();
 
@@ -4939,7 +4937,6 @@ void do_final(void){
 			for (int j=0; j<MAX_MOB_LIST_PER_MAP; j++)
 				if (mapdata->moblist[j]) aFree(mapdata->moblist[j]);
 		}
-		map_free_questinfo(mapdata);
 		mapdata->damage_adjust = {};
 	}
 

src/map/map.hpp

@@ -722,13 +722,6 @@ struct iwall_data {
 	bool shootable;
 };
 
-struct s_questinfo {
-	struct npc_data *nd;
-	e_questinfo_types icon;
-	e_questinfo_markcolor color;
-	struct script_code* condition;
-};
-
 struct map_data {
 	char name[MAP_NAME_LENGTH];
 	uint16 index; // The map index used by the mapindex* functions.
@@ -766,7 +759,7 @@ struct map_data {
 	struct Channel *channel;
 
 	/* ShowEvent Data Cache */
-	std::vector<s_questinfo> qi_data;
+	std::vector<int> qi_npc;
 
 	/* speeds up clif_updatestatus processing by causing hpmeter to run only when someone with the permission can view it */
 	unsigned short hpmeter_visible;

src/map/npc.cpp

@@ -2483,6 +2483,8 @@ int npc_unload(struct npc_data* nd, bool single) {
 		}
 	}
 
+	nd->qi_data.clear();
+
 	script_stop_sleeptimers(nd->bl.id);
 	aFree(nd);
 

src/map/npc.hpp

@@ -51,6 +51,18 @@ struct s_npc_buy_list {
 #pragma pack(pop)
 #endif // not NetBSD < 6 / Solaris
 
+struct s_questinfo {
+	e_questinfo_types icon;
+	e_questinfo_markcolor color;
+	struct script_code* condition;
+
+	~s_questinfo(){
+		if( this->condition != nullptr ){
+			script_free_code( this->condition );
+		}
+	}
+};
+
 struct npc_data {
 	struct block_list bl;
 	struct unit_data ud; //Because they need to be able to move....
@@ -112,6 +124,8 @@ struct npc_data {
 	struct sc_display_entry **sc_display;
 	unsigned char sc_display_count;
 
+	std::vector<std::shared_ptr<s_questinfo>> qi_data;
+
 	struct {
 		t_tick timeout;
 		unsigned long color;

src/map/pc.cpp

@@ -1690,9 +1690,6 @@ bool pc_authok(struct map_session_data *sd, uint32 login_id2, time_t expiration_
 	sd->vars_ok = false;
 	sd->vars_received = 0x0;
 
-	sd->qi_display = nullptr;
-	sd->qi_count = 0;
-
 	//warp player
 	if ((i=pc_setpos(sd,sd->status.last_point.map, sd->status.last_point.x, sd->status.last_point.y, CLR_OUTSIGHT)) != SETPOS_OK) {
 		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);
@@ -13319,33 +13316,6 @@ void pc_validate_skill(struct map_session_data *sd) {
 	}
 }
 
-/**
- * Toggle to remember if the questinfo is displayed yet or not.
- * @param qi_display Display flag
- * @param show If show is true and qi_display is 0, set qi_display to 1 and show the event bubble.
- *             If show is false and qi_display is 1, set qi_display to 0 and hide the event bubble.
- **/
-static void pc_show_questinfo_sub(struct map_session_data *sd, bool *qi_display, struct s_questinfo *qi, bool show) {
-	if (show) {
-		// Check if need to be displayed
-		if ((*qi_display) != 1) {
-			(*qi_display) = 1;
-			clif_quest_show_event(sd, &qi->nd->bl, qi->icon, qi->color);
-		}
-	}
-	else {
-		// Check if need to be hide
-		if ((*qi_display) != 0) {
-			(*qi_display) = 0;
-#if PACKETVER >= 20120410
-			clif_quest_show_event(sd, &qi->nd->bl, QTYPE_NONE, QMARK_NONE);
-#else
-			clif_quest_show_event(sd, &qi->nd->bl, QTYPE_QUEST, QMARK_NONE);
-#endif
-		}
-	}
-}
-
 /**
  * Show available NPC Quest / Event Icon Check [Kisuka]
  * @param sd Player
@@ -13360,28 +13330,45 @@ void pc_show_questinfo(struct map_session_data *sd) {
 	struct map_data *mapdata = map_getmapdata(sd->bl.m);
 	nullpo_retv(mapdata);
 
-	if (mapdata->qi_data.empty())
+	if (mapdata->qi_npc.empty())
 		return;
-	if (mapdata->qi_data.size() != sd->qi_count)
+	if (mapdata->qi_npc.size() != sd->qi_display.size())
 		return; // init was not called yet
 
-	struct s_questinfo *qi = nullptr;
+	for (int i = 0; i < mapdata->qi_npc.size(); i++) {
-	bool show;
+		struct npc_data *nd = map_id2nd(mapdata->qi_npc[i]);
 
-	for (int i = 0; i < mapdata->qi_data.size(); i++) {
+		if (!nd || nd->qi_data.empty())
-		qi = &mapdata->qi_data[i];
+			continue;
- 		if (!qi)
- 			continue;
 
-		if (!qi->condition)
+		bool show = false;
-			show = true;
+
-		else {
+		for (auto &qi : nd->qi_data) {
-			if (achievement_check_condition(qi->condition, sd))
+			if (!qi->condition || achievement_check_condition(qi->condition, sd)) {
 				show = true;
-			else
+				// Check if need to be displayed
-				show = false;
+				if (!sd->qi_display[i].is_active || qi->icon != sd->qi_display[i].icon || qi->color != sd->qi_display[i].color) {
+					sd->qi_display[i].is_active = true;
+					sd->qi_display[i].icon = qi->icon;
+					sd->qi_display[i].color = qi->color;
+					clif_quest_show_event(sd, &nd->bl, qi->icon, qi->color);
+				}
+				break;
+			}
+		}
+		if (show == false) {
+			// Check if need to be hide
+			if (sd->qi_display[i].is_active) {
+				sd->qi_display[i].is_active = false;
+				sd->qi_display[i].icon = QTYPE_NONE;
+				sd->qi_display[i].color = QMARK_NONE;
+#if PACKETVER >= 20120410
+				clif_quest_show_event(sd, &nd->bl, QTYPE_NONE, QMARK_NONE);
+#else
+				clif_quest_show_event(sd, &nd->bl, QTYPE_QUEST, QMARK_NONE);
+#endif
+			}
 		}
-		pc_show_questinfo_sub(sd, &sd->qi_display[i], qi, show);
 	}
 #endif
 }
@@ -13394,11 +13381,7 @@ void pc_show_questinfo_reinit(struct map_session_data *sd) {
 #if PACKETVER >= 20090218
 	nullpo_retv(sd);
 
-	if (sd->qi_display) {
+	sd->qi_display.clear();
-		aFree(sd->qi_display);
-		sd->qi_display = nullptr;
-	}
-	sd->qi_count = 0;
 
 	if (sd->bl.m < 0 || sd->bl.m >= MAX_MAPINDEX)
 		return;
@@ -13406,10 +13389,14 @@ void pc_show_questinfo_reinit(struct map_session_data *sd) {
 	struct map_data *mapdata = map_getmapdata(sd->bl.m);
 	nullpo_retv(mapdata);
 
-	if (mapdata->qi_data.empty())
+	if (mapdata->qi_npc.empty())
 		return;
 
-	CREATE(sd->qi_display, bool, (sd->qi_count = mapdata->qi_data.size()));
+	sd->qi_display.reserve( mapdata->qi_npc.size() );
+
+	for( int i = 0; i < mapdata->qi_npc.size(); i++ ){
+		sd->qi_display.push_back( s_qi_display() );
+	}
 #endif
 }
 

src/map/pc.hpp

@@ -267,6 +267,12 @@ struct s_combos {
 	uint32 pos;
 };
 
+struct s_qi_display {
+	bool is_active;
+	e_questinfo_types icon;
+	e_questinfo_markcolor color;
+};
+
 struct map_session_data {
 	struct block_list bl;
 	struct unit_data ud;
@@ -678,8 +684,7 @@ struct map_session_data {
 	std::vector<int> cloaked_npc;
 
 	/* ShowEvent Data Cache flags from map */
-	bool *qi_display;
+	std::vector<s_qi_display> qi_display;
-	int qi_count;
 
 	// temporary debug [flaviojs]
 	const char* debug_file;

src/map/script.cpp

@@ -19733,9 +19733,6 @@ BUILDIN_FUNC(questinfo)
 		return SCRIPT_CMD_FAILURE;
 	}
 
-	struct s_questinfo qi;
-	struct script_code *script = nullptr;
-	int color = QMARK_NONE;
 	int icon = script_getnum(st, 2);
 
 #if PACKETVER >= 20120410
@@ -19772,6 +19769,8 @@ BUILDIN_FUNC(questinfo)
 		icon = icon + 1;
 #endif
 
+	int color = QMARK_NONE;
+
 	if (script_hasdata(st, 3)) {
 		color = script_getnum(st, 3);
 		if (color < QMARK_NONE || color >= QMARK_MAX) {
@@ -19781,6 +19780,8 @@ BUILDIN_FUNC(questinfo)
 		}
 	}
 
+	struct script_code *script = nullptr;
+
 	if (script_hasdata(st, 4)) {
 		const char *str = script_getstr(st, 4);
 
@@ -19798,13 +19799,18 @@ BUILDIN_FUNC(questinfo)
 		}
 	}
 
-	qi.nd = nd;
+	std::shared_ptr<s_questinfo> qi = std::make_shared<s_questinfo>();
-	qi.icon = static_cast<e_questinfo_types>(icon);
+
-	qi.color = static_cast<e_questinfo_markcolor>(color);
+	qi->icon = static_cast<e_questinfo_types>(icon);
-	qi.condition = script;
+	qi->color = static_cast<e_questinfo_markcolor>(color);
+	qi->condition = script;
+
+	nd->qi_data.push_back(qi);
 
 	struct map_data *mapdata = map_getmapdata(nd->bl.m);
-	mapdata->qi_data.push_back(qi);
+
+	if (mapdata && !util::vector_exists(mapdata->qi_npc, nd->bl.id))
+		mapdata->qi_npc.push_back(nd->bl.id);
 
 	return SCRIPT_CMD_SUCCESS;
 }

src/map/unit.cpp

@@ -3397,11 +3397,7 @@ int unit_free(struct block_list *bl, clr_type clrtype)
 				sd->num_quests = sd->avail_quests = 0;
 			}
 
-			if (sd->qi_display) {
+			sd->qi_display.clear();
-				aFree(sd->qi_display);
-				sd->qi_display = NULL;
-			}
-			sd->qi_count = 0;
 
 #if PACKETVER_MAIN_NUM >= 20150507 || PACKETVER_RE_NUM >= 20150429 || defined(PACKETVER_ZERO)
 			sd->hatEffects.clear();