From fb81e2d7adf4f2def67c1fb4109fb43ef7f43d43 Mon Sep 17 00:00:00 2001 From: Lemongrass3110 Date: Thu, 20 Dec 2018 08:23:10 +0100 Subject: [PATCH] Fixed autobonus removal (#3767) Fixes #3739 Thanks to @whupdo for finding the broken removal. Fixes #3766 Since vector elements will be pushed around in the container when an element is removed, the original pointer might be invalid at the time the end of the timer has been reached. A big thank you to @teededung for being persistent and helping me to reproduce the issue. --- src/map/pc.cpp | 52 ++++++++++++++++++++++++++++++----------------- src/map/pc.hpp | 2 +- src/map/skill.cpp | 6 +++--- 3 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/map/pc.cpp b/src/map/pc.cpp index a0780f27d7..9a290ac350 100755 --- a/src/map/pc.cpp +++ b/src/map/pc.cpp @@ -2356,10 +2356,14 @@ void pc_delautobonus(struct map_session_data* sd, std::vector &bonu if (!sd) return; - for (uint8 i = 0; i < bonus.size(); i++) { - if (bonus[i].active != INVALID_TIMER) { - if (restore && (sd->state.autobonus&bonus[i].pos) == bonus[i].pos) { - if (bonus[i].bonus_script) { + std::vector::iterator it = bonus.begin(); + + while( it != bonus.end() ){ + s_autobonus b = *it; + + if (b.active != INVALID_TIMER) { + if (restore && (sd->state.autobonus&b.pos) == b.pos) { + if (b.bonus_script) { unsigned int equip_pos_idx = 0; // Create a list of all equipped positions to see if all items needed for the autobonus are still present [Playtester] @@ -2368,22 +2372,26 @@ void pc_delautobonus(struct map_session_data* sd, std::vector &bonu equip_pos_idx |= sd->inventory.u.items_inventory[sd->equip_index[j]].equip; } - if ((equip_pos_idx&bonus[i].pos) == bonus[i].pos) - script_run_autobonus(bonus[i].bonus_script, sd, bonus[i].pos); + if ((equip_pos_idx&b.pos) == b.pos) + script_run_autobonus(b.bonus_script, sd, b.pos); } + + it++; + continue; } else { // Logout / Unequipped an item with an activated bonus - delete_timer(bonus[i].active, pc_endautobonus); - bonus[i].active = INVALID_TIMER; + delete_timer(b.active, pc_endautobonus); + b.active = INVALID_TIMER; } } - if (bonus[i].bonus_script) - aFree(bonus[i].bonus_script); - if (bonus[i].other_script) - aFree(bonus[i].other_script); + if (b.bonus_script) + aFree(b.bonus_script); + if (b.other_script) + aFree(b.other_script); + + it = bonus.erase(it); } - bonus.clear(); } /** @@ -2391,7 +2399,7 @@ void pc_delautobonus(struct map_session_data* sd, std::vector &bonu * @param sd: Player data * @param autobonus: Autobonus to run */ -void pc_exeautobonus(struct map_session_data *sd, struct s_autobonus *autobonus) +void pc_exeautobonus(struct map_session_data *sd, std::vector *bonus, struct s_autobonus *autobonus) { if (!sd || !autobonus) return; @@ -2412,7 +2420,7 @@ void pc_exeautobonus(struct map_session_data *sd, struct s_autobonus *autobonus) script_run_autobonus(autobonus->other_script,sd,autobonus->pos); } - autobonus->active = add_timer(gettick()+autobonus->duration, pc_endautobonus, sd->bl.id, (intptr_t)autobonus); + autobonus->active = add_timer(gettick()+autobonus->duration, pc_endautobonus, sd->bl.id, (intptr_t)bonus); sd->state.autobonus |= autobonus->pos; status_calc_pc(sd,SCO_FORCE); } @@ -2422,13 +2430,19 @@ void pc_exeautobonus(struct map_session_data *sd, struct s_autobonus *autobonus) */ TIMER_FUNC(pc_endautobonus){ struct map_session_data *sd = map_id2sd(id); - struct s_autobonus *autobonus = (struct s_autobonus *)data; + std::vector *bonus = (std::vector *)data; nullpo_ret(sd); - nullpo_ret(autobonus); + nullpo_ret(bonus); - autobonus->active = INVALID_TIMER; - sd->state.autobonus &= ~autobonus->pos; + for( struct s_autobonus& autobonus : *bonus ){ + if( autobonus.active == tid ){ + autobonus.active = INVALID_TIMER; + sd->state.autobonus &= ~autobonus.pos; + break; + } + } + status_calc_pc(sd,SCO_FORCE); return 0; } diff --git a/src/map/pc.hpp b/src/map/pc.hpp index 3308b9fd2f..0b5a7802a8 100644 --- a/src/map/pc.hpp +++ b/src/map/pc.hpp @@ -1068,7 +1068,7 @@ bool pc_adoption(struct map_session_data *p1_sd, struct map_session_data *p2_sd, void pc_updateweightstatus(struct map_session_data *sd); bool pc_addautobonus(std::vector &bonus, const char *script, short rate, unsigned int dur, short atk_type, const char *o_script, unsigned int pos, bool onskill); -void pc_exeautobonus(struct map_session_data* sd, struct s_autobonus *bonus); +void pc_exeautobonus(struct map_session_data* sd, std::vector *bonus, struct s_autobonus *autobonus); TIMER_FUNC(pc_endautobonus); void pc_delautobonus(struct map_session_data* sd, std::vector &bonus, bool restore); diff --git a/src/map/skill.cpp b/src/map/skill.cpp index 0eb430ecbf..13ebe5f5b6 100755 --- a/src/map/skill.cpp +++ b/src/map/skill.cpp @@ -2179,7 +2179,7 @@ int skill_additional_effect(struct block_list* src, struct block_list *bl, uint1 ((it.atk_type)&attack_type)&BF_RANGEMASK && ((it.atk_type)&attack_type)&BF_SKILLMASK)) continue; // one or more trigger conditions were not fulfilled - pc_exeautobonus(sd, &it); + pc_exeautobonus(sd, &sd->autobonus, &it); } } @@ -2279,7 +2279,7 @@ int skill_onskillusage(struct map_session_data *sd, struct block_list *bl, uint1 continue; if (it.atk_type != skill_id) continue; - pc_exeautobonus(sd, &it); + pc_exeautobonus(sd, &sd->autobonus3, &it); } } @@ -2510,7 +2510,7 @@ int skill_counter_additional_effect (struct block_list* src, struct block_list * ((it.atk_type)&attack_type)&BF_RANGEMASK && ((it.atk_type)&attack_type)&BF_SKILLMASK)) continue; // one or more trigger conditions were not fulfilled - pc_exeautobonus(dstsd, &it); + pc_exeautobonus(dstsd, &dstsd->autobonus2, &it); } }