Song Refresh

Unresolved Issue about the "Song Refresh bug"
http://rathena.org/b..._4951#entry4951


Originally posted by Valo
http://www.eathena.w...er&showbug=4951

Tested the following scenario:

Clown A activated Bragi, Clown B walked into the area of effect, then quickly exited it which then results in 20 seconds of bragi effect (as intended).
But if Clown B then activates bragi (then deactivates by e.g. switching weapon) himself within 20 seconds he resets the timer on that effect to 20 again, which results, if done repeatedly, in infinite bragi effect for Clown B.

The clown/bard should only be able to bragi himself when soullinked.

This might work for all songs because of the way they are handled by the server.

I found this bug to be present in 14600+ (didn't test any earlier revisions though).

Edit:
I've somehow figured out the problem.

case DC_SERVICEFORYOU:
if (sce)
{
delete_timer(sce->timer, status_change_timer);
//NOTE: It'd be nice if we could get the skill_lv for a more accurate extra time, but alas...
//not possible on our current implementation.
sce->val4 = 1; //Store the fact that this is a "reduced" duration effect.
sce->timer = add_timer(tick+skill_get_time2(skill_id,1), status_change_timer, bl->id, type);
}
break;

This code is responsible for adding the timer after leaving a song. But since the parameters are seriously crippled one cannot find out whether the bragi left was cast by the owner or another clown. (I'm not entirely sure because I don't get the structure completely yet, Notepad++ isn't that informative.. ) Since one doesn't want the timer to start for someone who hasn't had bragi before (meaning: the caster of the bragi) it is checked whether the effect was there before. If the caster already has the effect (gotten from another clown / bard for example) it is of course refreshed.

static int skill_unit_onleft (int skill_id, struct block_list *bl, unsigned int tick)

Now I've come up with a quick & dirty fix which still leaves another bug which isn't that severe though.
I've added another state for val4 (which apparently is some variable not used for the bragi sc, val3 is also free though I think ).

case DC_SERVICEFORYOU:
if (sce)
{
if ( sce->val4 != 2 ) {
delete_timer(sce->timer, status_change_timer);
//NOTE: It'd be nice if we could get the skill_lv for a more accurate extra time, but alas...
//not possible on our current implementation.
sce->val4 = 1; //Store the fact that this is a "reduced" duration effect.

sce->timer = add_timer(tick+skill_get_time2(skill_id,1), status_change_timer, bl->id, type);
}
}
break;

And changed

case UNT_SERVICEFORYOU:
if (sg->src_id==bl->id && !(sc && sc->data[SC_SPIRIT] && sc->data[SC_SPIRIT]->val2 == SL_BARDDANCER))
return 0;
if (!sc) return 0;
if (!sce)
sc_start4(bl,type,100,sg->skill_lv,sg->val1,sg->val2,0,5);
else if (sce->val4 == 1) {
//Readjust timers since the effect will not last long.
sce->val4 = 0;
delete_timer(sce->timer, status_change_timer);
sce->timer = add_timer(tick+sg->limit, status_change_timer, bl->id, type);
}
break;

to

case UNT_SERVICEFORYOU:
if (sg->src_id==bl->id && !(sc && sc->data[SC_SPIRIT] && sc->data[SC_SPIRIT]->val2 == SL_BARDDANCER)) {
if ( sce ) {
sce->val4 = 2;
}
return 0;
}
if (!sc) return 0;
if (!sce)
sc_start4(bl,type,100,sg->skill_lv,sg->val1,sg->val2,0,sg->limit);
else if (sce->val4 >= 1) {
//Readjust timers since the effect will not last long.
sce->val4 = 0;
delete_timer(sce->timer, status_change_timer);
sce->timer = add_timer(tick+sg->limit, status_change_timer, bl->id, type);
}
break;

This fixes the above described problem but another bug arises. If one tries the above scenario which gave Clown B infinite bragi he instead becomes "immune" to Clown A's bragi until Clown A recasts a new bragi. Aside from that everything works as intended then. Maybe some dev comes up with some cleaner, working solution but this is as far as I can go with what little I figured out.