Skip to content

Let non-damage skills bypass player_invincible_time - #3430

Open
MrKeiKun wants to merge 2 commits into
HerculesWS:masterfrom
MrKeiKun:fix/966-invincible-time-nodamage-skills
Open

Let non-damage skills bypass player_invincible_time#3430
MrKeiKun wants to merge 2 commits into
HerculesWS:masterfrom
MrKeiKun:fix/966-invincible-time-nodamage-skills

Conversation

@MrKeiKun

@MrKeiKun MrKeiKun commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Pull Request Prelude

Changes Proposed

battle_check_target() blocked all skill targeting against a player under player_invincible_time (post-spawn/teleport grace period), except for a hardcoded two-skill allowlist (PR_SANCTUARY, PR_MAGNIFICAT) added in 2015 as an admittedly incomplete fix for bugreport:8397.

On official servers, invincibility only protects against damage and other hostile/offensive effects — non-damage skills such as SA_DISPELL, songs (BA_POEMBRAGI, DC_SERVICEFORYOU), and other buffs/debuffs should apply immediately regardless of the target's invincible state.

This replaces the two-skill allowlist with a general check against skill->get_nk(skill_id) & NK_NO_DAMAGE, so any non-damage skill bypasses the invincibility gate by default instead of needing to be special-cased individually. Equip-strip skills (RG_STRIPWEAPON/STRIPSHIELD/STRIPARMOR/STRIPHELM, ST_FULLSTRIP, SC_STRIPACCESSARY, GC_WEAPONCRUSH) are kept blocked despite being NK_NO_DAMAGE, since they're still an offensive debuff. Plain melee attacks (skill_id == 0) are unaffected and remain blocked.

Issues addressed: #966

Thanks @kyeme and @Playtester

Replace the two-skill allowlist with a general NK_NO_DAMAGE check so
buffs, debuffs, songs, and dispel apply immediately to newly spawned
or teleported targets, matching official behavior, while equip-strip
skills stay blocked as before.
@MrKeiKun MrKeiKun changed the title Let non-damage skills bypass player_invincible_time (#966) Let non-damage skills bypass player_invincible_time Aug 25, 2026
@kyeme

kyeme commented Sep 5, 2026

Copy link
Copy Markdown

@MrKeiKun
It seems that player_invincible_time against monsters is broken after these changes.

For example, if you log in while there is a monster nearby, the monster will attack you immediately instead of waiting for player_invincible_time to expire.

@MrKeiKun

MrKeiKun commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

@MrKeiKun It seems that player_invincible_time against monsters is broken after these changes.

For example, if you log in while there is a monster nearby, the monster will attack you immediately instead of waiting for player_invincible_time to expire.

try now @kyeme interestingly, battle_check_target() bypassed invincible_timer based on battle->get_current_skill(src), which returns unit_data::skill_id. i never noticed that earlier 😆

unit_data::skill_id reflects the last skill a unit *cast* and is never
reset afterwards, so battle_check_target() could misread a mob's stale
no-damage skill_id as the current action and let a plain attack bypass
a target's invincibility. unit->attack() now flags the call as a normal
attack via the new BCT_NORMAL_ATTACK bit so the check no longer trusts
leftover skill state for it.
@MrKeiKun
MrKeiKun force-pushed the fix/966-invincible-time-nodamage-skills branch from 06280d6 to 8e91d52 Compare September 6, 2026 01:50
@kyeme

kyeme commented Sep 6, 2026

Copy link
Copy Markdown

@MrKeiKun Tested the fix, and it's working correctly now. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants