Fix ECS entity id vs player_id confusion across magic, combat, effects

[barrelin-oss]

Summary

Full src/ sweep for the recurring bug class where an ECS entity id (from the entity_manager shared with NPCs) was cast to player_id — or the reverse. The two id spaces are not interchangeable: get_player(player_id{entity.id}) returns nullptr (or the wrong player) and if (auto* p = ...)-guarded blocks are silently skipped. Standard fix: player_system::get_player_by_entity(entity) (O(1), const overload available).

Fixes

• magic_system.cpp (21 sites): mana/HP/SP deduction, silenced/level/stat checks, range checks, safe-zone PvP checks, damage/heal scaling, SP drain, resurrection targets, debuff resist rolls. find_aoe_targets/find_line_targets no longer fabricate entity{player_id.value, 0} for hit players — they push the real p.ecs_entity (fabricated handles broke downstream combat_system::deal_damage and the spell broadcast, which resolve via the real ECS index).
• application.cpp: periodic heal / mana_drain / mana_restore effect ticks never applied to players.
• game_handlers_combat.cpp: on_spell_cast broadcast resolved caster/targets with the bad cast; respawn invulnerability was keyed on a fabricated entity and never matched the real defender — the 3s respawn protection was silently inert.
• player_system::remove_player: effect cleanup used entity{id.value} instead of ecs_entity, leaking active effects on logout.
• wave4_handlers.cpp (legacy binary path): attack/cast fabricated caster entities from ctx.player.value; now resolve the player's real ecs_entity (their spell-knowledge/cooldown lookups could never match the JSON path's keys).

Confirmed legitimate (left untouched)

• Admin API get_player(player_id{req.player_id}) — the wire value is a real player id.
• Inventory/trade/crafting entity_id{pid.value} keying — consistent at creation and access.
• Spatial-index entity_id{ecs_entity.index()}.

Tests

• test_magic.cpp, test_safe_zone.cpp, test_melee_pve.cpp updated to resolve the real ecs_entity instead of fabricating entity(pid.value) — the old form only passed because test fixtures spawn no NPCs, so the player-id and ECS-index counters coincided.
• Full suite: 2522/2522 passing (3 pre-existing skips that need real map files).

Documented in docs/PROGRESS.md under Recent Changes.

🤖 Generated with Claude Code

[barrelin-oss]

Closing in favor of #1: the local master fast-forwarded to this branch's merge commit (536af07) and the fork master mirrored in #1 now includes the full audit content. Re-validated post-merge: clean build, 2531/2531 tests passing.