draconisPW / PWMAngband

A free, multi-player roguelike dungeon exploration game based on Angband
35 stars 11 forks source link

player_apply_damage_reduction() made take_hit() clumsy #628

Closed igroglaz closed 2 months ago

igroglaz commented 2 months ago

last PWMA commit removed bool to phys dmg in image

it's not cool as eg I used this func for

    // instead of DAM_RED (raw reducement), hc % reducement
    // 1) to ALL dmg
    if (streq(p->race->name, "Gargoyle"))
        damage = (damage * 17) / 18;
    // 2) to physical dmg
    else if (streq(p->race->name, "Half-Giant") && !non_physical)
        damage = (damage * 12) / 13;
    // 3) magic dmg
    else if (streq(p->race->name, "Golem") && non_physical)
        damage = (damage * 9) / 10;

so it reduces code customizability 😦 please could you try not to remove such flag in future as it break T stuff

I've reworked new player_apply_damage_reduction() to T means:

int player_apply_damage_reduction(struct player *p, int dam, bool non_physical)
{
    // in T this applies ONLY for _pure_ physical damage, with the only exception - GoI

    /* Globe of invulnerability protects against non-physical attacks only OR safelogin */
    if (non_physical || p->timed[TMD_SAFELOGIN]) {
        if (p->timed[TMD_INVULN]) dam -= dam * p->lev / 100;
    }
    // in all other case - ONLY for _pure_ physical damage
    else {
        dam -= p->state.dam_red; // Apply damage reduction
    }

    return ((dam < 0)? 0: dam);

} but in general old code was more easier to customize

now I dont know wjhat to do with it. without bool non_physical in take_hit T looses it's features. please could you reconsider to add bool non_physical back

for now I'll revert PWMA past commit. I preserved it in new branch: https://github.com/igroglaz/Tangaria/tree/24042024-before-reverting-PWMA-comm

draconisPW commented 2 months ago

I don't understand...

Every damage reduction should go into player_apply_damage_reduction(). take_hit() should apply final damage at player hp. The code you added for T goes into player_apply_damage_reduction() fine...

igroglaz commented 2 months ago

take hit atm: `bool take_hit(struct player p, int damage, const char hit_from, bool non_physical, const char died_flavor) { int old_chp = p->chp; int warning = (p->mhp p->opts.hitpoint_warn / 10); int old_num = get_player_num(p);

/* Undisturbed rest */
bool nodisturb = ((p->upkeep->resting == REST_COMPLETE_NODISTURB) && (p->chp > warning));

/* Paranoia */
if (p->is_dead) return true;

/* Become aware of player's presence */
if (p->k_idx) aware_player(p, p);

/* Hack -- apply "invulnerability" */
if ((p->timed[TMD_INVULN] == -1) || p->timed[TMD_SAFELOGIN])
{
    /* Permanent invulnerability */
    damage = 0;
}
else if (p->timed[TMD_INVULN] && non_physical)
{
    /* Globe of invulnerability protects against non-physical attacks only */
    damage -= damage * p->lev / 100;
}

// Apply damage reduction: ONLY for _pure_ physical damage
if (p->state.dam_red != 0 && !non_physical && strcmp(hit_from, "fading") &&
    strcmp(hit_from, "hypoxia") && strcmp(hit_from, "poison") &&
    strcmp(hit_from, "a fatal wound") && strcmp(hit_from, "starvation") &&
    strcmp(hit_from, "an earthquake") && strcmp(hit_from, "adrenaline poisoning") &&
    strcmp(hit_from, "over-exertion") && strcmp(hit_from, "drowning"))
        damage -= p->state.dam_red;
if (damage <= 0)
{
    p->died_flavor[0] = '\0';
    return false;
}

// instead of DAM_RED (raw reducement), hc % reducement
// 1) to ALL dmg
if (streq(p->race->name, "Gargoyle"))
    damage = (damage * 17) / 18;
// 2) to physical dmg
else if (streq(p->race->name, "Half-Giant") && !non_physical)
    damage = (damage * 12) / 13;
// 3) magic dmg
else if (streq(p->race->name, "Golem") && non_physical)
    damage = (damage * 9) / 10;

`

so in the end there is a check for races and different races has different resistances to phys/non-phys

igroglaz commented 2 months ago

note: (there is a bug on T side as race's dmg reducing should go before this if (damage <= 0) { p->died_flavor[0] = '\0'; return false; } will fix this :)

draconisPW commented 2 months ago

Still all the code should go into player_apply_damage_reduction(). If you have to make adjustments, they should be there.

igroglaz commented 2 months ago

thanks, but there are still problem. previously it was possible to understand source of dmg and appy reducing only at dmg which needed: // in all other case - ONLY for _pure_ physical damage if (p->state.dam_red != 0 && !non_physical && strcmp(hit_from, "fading") && strcmp(hit_from, "hypoxia") && strcmp(hit_from, "poison") && strcmp(hit_from, "a fatal wound") && strcmp(hit_from, "starvation") && strcmp(hit_from, "an earthquake") && strcmp(hit_from, "adrenaline poisoning") && strcmp(hit_from, "over-exertion") && strcmp(hit_from, "drowning")) damage -= p->state.dam_red;

so it was kinda list of dmg which should be affected by dmg red. Now it's all scattered around the code.

It's was better before - when we had dmg reducing in 1 place (in take_hit()) and had all info about damage there. Now code is much harder to maintain

igroglaz commented 2 months ago

so the only solution is to pass hit_from to player_apply_damage_reduction() I suppose... and it makes code a bit ugly as we pass it to take_hit() too. previously it was better IMO

igroglaz commented 2 months ago

I've cherry-picked fbe312a3a315f66ccd647c7ff35276f74921ad6f and 75126493406e4042b266b007fa9705e21792c5d0 commits... but this big update 18bbe8a0996133dd94bfc1a8fcbc08c8bf3da78b is not fun, sorry :( I had to pass it. Sad that it include a lot of things :(

draconisPW commented 2 months ago

Previous code didn't allow displaying correct messages as the message was displayed before take_hit was called. Now the proper sequence is applied: damage reduction -> message -> take hit. I had to review 50-60 cases of take_hit for this update. You probably have to do the same, it shouldn't take too much if the only thing you have to do is add hit_from to damage reduction.