when player die, Hp is 1 ???

[AnnieRuru]

the exact same script

prontera,155,185,5  script  dsfjhsdkfj  1_F_MARIA,{
OnPCDieEvent:
    dispbottom Hp +" "+ readparam(Hp);
    end;
}

in rathena, when player alive, display 4138 4138 when player dead, display 0 0

in hercules when player alive, display 1000000 0 when player dead, display 1 0

there is currently no way to determine the player is dead in hercules unlike in the source, there is a pc_isdead() function, but no script command to do that I like the way how rathena just use readparam(Hp) to check . . if you need a screenshot, http://herc.ws/board/topic/10903-mvp-ladder-game-problem/#entry65045

[Reilaen]

Does that also happen when you use the Hp variable? It shouldn't be 1 for whatever reason you use it when your Hp should be 0. I am just curios if that happens with the variable too.

[AnnieRuru]

yes, both are broken in hercules dispbottom Hp +" "+ readparam(Hp);I tested with both hp and readparam(Hp)

in hercules, with Hp variable when alive, it shows your current hp <-- correct when dead, it shows 1 <-- wrong

with readparam(Hp) both alive and dead just shows 0

rathena got all of them right ... that's why ...

I got a feeling that with readparam script command, the Hp already is your current hp, becomes readparam(100000) <-- not confirm

[MishimaHaruna]

You can't use readparam(Hp), but you should instead use Hp directly. Documentation is incomplete and slightly misleading when it mentions "you can use a parameter name if it is defined in 'db/const.txt'". In fact, the following can't be used with readparam, but should be used directly (yes, it's just as you guessed, Hp will be replaced with its value - that's by design):

StatusPoint 9   1
BaseLevel   11  1
SkillPoint  12  1
Class   19  1
Upper   56  1
Zeny    20  1
Sex 21  1
Weight  24  1
MaxWeight   25  1
JobLevel    55  1
BaseExp 1   1
JobExp  2   1
Karma   3   1
Manner  4   1
NextBaseExp 22  1
NextJobExp  23  1
Hp  5   1
MaxHp   6   1
Sp  7   1
MaxSp   8   1
BaseJob 119 1
BaseClass   120 1
killerrid   121 1
killedrid   122 1
SlotChange  123 1
CharRename  124 1
ModExp  125 1
ModDrop 126 1
ModDeath    127 1

As for the reason why Hp is 1 instead of 0 when a player is dead, it's because you're checking it in the OnPCDieEvent, which is executed before the actual death (see the following code snippet from status_damage, keeping in mind that OnPCDieEvent is executed by pc->dead()):

    st->hp = 1; //To let the dead function cast skills and all that.
    //NOTE: These dead functions should return: [Skotlex]
    //0: Death canceled, auto-revived.
    //Non-zero: Standard death. Clear status, cancel move/attack, etc
    //&2: Also remove object from map.
    //&4: Also delete object from memory.
    switch (target->type) {
        case BL_PC:  flag = pc->dead((TBL_PC*)target,src); break;
        case BL_MOB: flag = mob->dead((TBL_MOB*)target, src, (flag&4) ? 3 : 0); break;
        case BL_HOM: flag = homun->dead((TBL_HOM*)target); break;
        case BL_MER: flag = mercenary->dead((TBL_MER*)target); break;
        case BL_ELEM: flag = elemental->dead((TBL_ELEM*)target); break;
        default: //Unhandled case, do nothing to object.
            flag = 0;
            break;
    }

    if(!flag) //Death canceled.
        return (int)(hp+sp);

    //Normal death
    st->hp = 0;

That's a part of code that we inherited from *Athena, and was never changed in Hercules (I haven't checked if it was changed in rAthena at a later time). If we ever change it, we have to keep in mind that it may break existing scripts that rely on it.

Is there a reason why you want to check the HP in the death event?

[AnnieRuru]

for the reason to do this, this script working fine in rathena, but broken in hercules https://github.com/rathena/rathena/blob/master/npc/custom/events/mvp_ladder.txt the idea is for the priest to resurrect their party members (which is usual when fighting MVP) so when the last party member died, loop through all the party members just check they are still alive only if ALL party members died, the game ends

break existing scripts ? LOL !!! I rather think this is a bug that passed down from eAthena, and only rathena fixed it now waiting for hercules to fix it

and for the readparam, I remember in rAthena, their bStr cannot read, but readparam(bStr) works in hercules however, bStr works, but readparam(bStr) doesn't work which is totally opposite you are welcome to remove the readparam command because I saw many hercules members just use the constant without readparam (since its broken?) which makes us different from rathena again ... yeah this is a good move /gg

now you mentioned it, yes I think rathena fixed it at a later date if my memory still serve me right, they fixed this bug before they move to GIT ... so should still in SVN

[Ancyker]

Using OnPCDieEvent for a single map is a waste of resources. Better to use a timer to check for wipes.

[Ancyker]

Also, for that script, once you find someone alive you might as well just break. There's no point looping through everyone if you treat >0 the same.

[Ancyker]

So like this:

    getpartymember .party_id, 1;
    getpartymember .party_id, 2;
    for ( .@i = 0; .@i < $@partymembercount; .@i++ ) {
        if ( isloggedin( $@partymemberaid[.@i], $@partymembercid[.@i] ) ) {
            attachrid $@partymemberaid[.@i];
            if ( strcharinfo(3) == .eventmap$ && hp > 0 )
                end;
        }
    }
    mapannounce .eventmap$, "Party wiped!", bc_map;
    sleep 10000;
    awake strnpcinfo(0);
    end;

[AnnieRuru]

@Ancyker I believe OnPCDieEvent+OnPCLogoutEvent should be a better choice than while(1){...sleep} I'll stick to this method

but good thing you point out the flaw of my script though

however this bug report still stand, Hp must be 0 when the player dead =/

[Ancyker]

I never said to use while(1){sleep}, I said to use a timer.

    initnpctimer;
...
OnTimer30000:
    getpartymember .party_id, 1;
    getpartymember .party_id, 2;
    for ( .@i = 0; .@i < $@partymembercount; .@i++ ) {
        if ( isloggedin( $@partymemberaid[.@i], $@partymembercid[.@i] ) ) {
            attachrid $@partymemberaid[.@i];
            if ( strcharinfo(3) == .eventmap$ && hp > 0 )
                end;
        }
    }
    mapannounce .eventmap$, "Party wiped!", bc_map;
    sleep 10000;
    awake strnpcinfo(0);
    end;

OnPC events are VERY taxing on the server. You should avoid them when you can.

OnPCDieEvent and OnPCLogoutEvent will ALWAYS be active, even when your script is not running. That means when everyone is off WoEing it's constantly tripping both of those (since people die a lot and many people relog to get out of castles). This adds additional load during WoE and other events and wastes resources. Worse yet, the larger the server the more resources wasted.

On the other hand, a timer consumes very limited resources in comparison. It's only running when someone is actually using your script, and it runs far less often. It uses much much less resources. It also doesn't use more resources with more population, unlike OnPC events which are ran more and more often as servers get larger.

[AnnieRuru]

curious, why not add more something like OnPCDieMapEvent: with diemap mapflag something like that its the same with OnPCLoadMapEvent with loadmap mapflag this way theOnPCDieMapEvent only trigger on certain map

forgot to mention @Ancyker

[Ancyker]

Why not use a timer instead of adding more overhead? All these things add clutter to the source code.

[AnnieRuru]

@Ancyker I just realize Hercules has queue related commands http://herc.ws/board/topic/928-memory-slasher-may-30-patch/?hl=iterator if I use those queueadd queueopt ... this will save much more memory I think ?

they are in script_commands.txt

[AnnieRuru]

again, this bug report also stands

even with hercules queue iterator script commands, the Hp is also shows as 1

prontera,155,185,5  script  kjhfksdjf   1_F_MARIA,{
    .qid = queue();
    queueadd .qid, getcharid(3);
    queueopt .qid, HQO_OnDeath, strnpcinfo(0)+"::OnDeath";
    dispbottom "Alive. Hp is "+ Hp; // shows 1000000
    end;
OnDeath:
    dispbottom "Dead. Hp is "+ Hp; // shows 1
    end;
}

[dastgirp]

Why do you need HP as 1? Only these events show HP as 1, but after player is dead, and you try attaching to him, it would show HP as 0. Reason been as haru said, we set HP as 1, call pc->dead and then set HP as 0

In pc_dead we have

    if( sd->bg_id ) {/* TODO: purge when bgqueue is deemed ok */
        struct battleground_data *bgd;
        if( (bgd = bg->team_search(sd->bg_id)) != NULL && bgd->die_event[0] )
            npc->event(sd, bgd->die_event, 0);
    }

    for( i = 0; i < sd->queues_count; i++ ) {
        struct hQueue *queue;
        if( (queue = script->queue(sd->queues[i])) && queue->onDeath[0] != '\0' )
            npc->event(sd, queue->onDeath, 0);
    }

    npc->script_event(sd,NPCE_DIE)

So all Die events will show HP as 1, but after dead, try attaching to them, it would show HP as 0...

[Ancyker]

No offense but, are you allergic to timers or something? You refuse to use them when they make sense and abuse the crap out of sleep.

[AnnieRuru]

I didn't say I need Hp as 1 I say I need the Hp to display as 0

here is the ugly, but quick fix

    sd->battle_status.hp = 0; // add this

    if( sd->bg_id ) {/* TODO: purge when bgqueue is deemed ok */
        struct battleground_data *bgd;
        if( (bgd = bg->team_search(sd->bg_id)) != NULL && bgd->die_event[0] )
            npc->event(sd, bgd->die_event, 0);
    }

    for( i = 0; i < sd->queues_count; i++ ) {
        struct hQueue *queue;
        if( (queue = script->queue(sd->queues[i])) && queue->onDeath[0] != '\0' )
            npc->event(sd, queue->onDeath, 0);
    }

    npc->script_event(sd,NPCE_DIE);

    sd->battle_status.hp = 1; // add this

@Ancyker , yes I'm don't like to use timer I found out hercules queue system works great as well Ima going to use hercules queue system from now on

from my understanding, the HQO_OnDeath is attach to the player (from the queue) if the player logout, this event label is detached from the player (it save memory) so it wont trigger on everyone, only those inside the queue

[MishimaHaruna]

Just a question. When you use those events, they run with the character that just died attached. Do you really need its HP to be 0 to know it's dead? If the event was triggered, the character is dead, there's no need to check anything else. You can certainly loop through its party to check if the others are also dead, but you know already that the invoker is dead.

Until a decision is taken on this matter (whether to change the place those events are invoked so that they're called after the HP is already 0), rather than your source workaround, wouldn't it be easier to work around it in the script directly?

[Emistry]

why not just delay the script execution ??

    OnPCDieEvent:
        dispbottom "readparam(Hp) = "+ readparam(Hp);
        dispbottom "HP = "+Hp;    
        sleep2 1;
        dispbottom "sleep1 readparam(Hp) = "+ readparam(Hp);
        dispbottom "sleep1 HP = "+Hp;    
        end;

[AnnieRuru]

@emistry your method is DEFINITELY a NO go it may work for single player stuff, but not in the case for team-based script because when inserting a sleep command, multi-thread will happen sleep2 1; doesn't necessary pause 1 mili-second, it can range from 20~100 mili-seconds depends on server load this multi-thread will create mili-seconds race scenario for example, what happen if player A die, and within that short time player B kill the monster ? the script will run both the OnPCDieEvent and OnMobDie: label and this will mess up the event totally

however for Acyker suggestion, that one can be counter this problem easily by adding killmonster <monster label>; during the party wipe check

@MishimaHaruna yes there is a work around method by adding another variable .@attachaid in the script so I have to do this extra check when I made a party based script in hercules

//  rathena
-   script  dfhdksjf    -1,{
OnPCDieEvent:
    getpartymember getcharid(1), 1;
    getpartymember getcharid(1), 2;
    for ( .@i = 0; .@i < $@partymembercount; .@i++ ) {
        if ( isloggedin( $@partymemberaid[.@i], $@partymembercid[.@i] ) ) {
            attachrid $@partymemberaid[.@i];
            if ( Hp )
                .@count++;
        }
    }
    announce .@count +" party members still alive", bc_all;
    end;
}

//  hercules
-   script  dfhdksjf    -1,{
OnPCDieEvent:
    .@attachaid = getcharid(3); // ADD this
    getpartymember getcharid(1), 1;
    getpartymember getcharid(1), 2;
    for ( .@i = 0; .@i < $@partymembercount; .@i++ ) {
        if ( isloggedin( $@partymemberaid[.@i], $@partymembercid[.@i] ) ) {
            attachrid $@partymemberaid[.@i];
            if ( $@partymemberaid[.@i] != .@attachaid && Hp ) // EXTRA check
                .@count++;
        }
    }
    announce .@count +" party members still alive", bc_all;
    end;
}

if you don't want to fix it, then I'll keep this in mind when I make this kind of event script

you may close this issue as you wish

oh and, remove the BUILDIN(readparam) from script.c while you are at it

[MishimaHaruna]

By all means, I'm interested in fixing this. I'm only concerned about side effects, so I want to read the related code carefully before this is fixed.

About readparam, right now it's still needed in some cases, since not all parameters are provided directly. An example are the character stats (bAgi, bStr, bInt, etc), but there are more (you can see them in pc_readparam)

[Historica]

@MishimaHaruna can you check this https://github.com/HerculesWS/Hercules/issues/659 while fixing that.

[AnnieRuru]

889

it seems the official behavior should show the amount of Hp as being re-spawn so the Hp should show 1, which is correct already

I think for this particular issue, we should create new script command isdead() to check player is dead

[MishimaHaruna]

Concerning #889, it just feels wron to display the HP you'll have at respawn (regardless, the value is different in case you're resurrected rather than respawned), I'd avoid doing that unless we have a reason to do it...

It kind of explains why aegis resurrects the character when it's moved while dead, likely an oversight on their part. I'm guessing their code can't handle a character's hp being equal to zero perhaps.

[AnnieRuru]

then .. you mean you'll do like rathena did ? their readparam(Hp) variable still is 0, but client show the Hp as 1 in clif.c file https://github.com/rathena/rathena/commit/c2f8dec83068296417743a5298403dfbbbba30ee

yes ... official has a lot of stuffs doing wrong ... but these bugs has exists since I started play RO ... 10 years ago, and still exist today perhaps

[gravity-ro]

Hello i have some question

This code use on rAthena but error this line

L_SpawnMob: .qid = queue(); <------- queueopt .qid, QUEUEOPT_LOGOUT, strnpcinfo(NPC_NAME)+"::OnQuit"; queueopt .qid, QUEUEOPT_MAPCHANGE, strnpcinfo(NPC_NAME)+"::OnMapChange";

How to fixit Thank you. ........................