refactor: combat and game separation

[dudantas]

Description

This refactor clarifies boundaries between Combat and Game, reduces coupling, and preserves behavior bit-for-bit.

Key changes:

• Moved pure combat helpers into src/creatures/combat/combat.cpp under an anonymous namespace, exposing only minimal static entry points on Combat where Game needs them.

• Kept Game loop/infrastructure in src/game/game.cpp; created InternalGame helpers for block/reflect processing, block effect dispatch, and small string utilities.

• Promoted applyImbuementElementalDamage to the public Combat interface and changed signature to in-place update:

  • before: CombatDamage applyImbuementElementalDamage(const std::shared_ptr<Player>&, std::shared_ptr<Item>, CombatDamage)
  • after: void applyImbuementElementalDamage(const std::shared_ptr<Player>&, std::shared_ptr<Item>, CombatDamage&)

• Ensured successful compilation when building messages from Combat by adding #include "server/network/protocol/protocolgame.hpp" wherever TextMessage is referenced from Combat headers.

• Migrated string building in moved code from std::stringstream to fmt::format / fmt::format_to while keeping output text identical.

• Modularized damage processing and reflection logic, keeping public APIs stable and not changing formulas, order of calls, or logs.

Behaviour

Actual

• Combat and Game mix responsibilities; message helpers in Combat can fail to compile due to missing TextMessage type.
• Damage flow code is more complex to maintain; reflection/blocking logic is interleaved with messaging.
• Some helpers are file-scattered and not clearly namespaced.

Expected

• Clear layering: Combat has combat helpers (anonymous namespace + minimal Combat:: entry points); Game has orchestration and InternalGame low-level helpers.
• Build succeeds with explicit protocolgame include when TextMessage is referenced.
• No change in runtime behavior, packet contents, visuals, or logs.

Fixes #issuenumber

N/A (structural refactor; behavior preserved)

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested

Validation focuses on parity and builds correctness:

• Build

  • Full rebuild across supported targets; verified no missing includes after moving message builders to Combat and adding protocolgame include.

• Parity tests (no behavioral change)

  • Snapshot tests for attacker/target/spectator messages before vs. after; byte-for-byte comparison.
  • Exercise PvP checks, hazard dodge, healing links, reflection caps, charms, leech, and Wheel bonuses; compared numbers, colors, effects, and logs.
  • Verified damage reflection and block effects still dispatch with the same limits and ordering via InternalGame helpers.

• Smoke runs

  • Ranged/melee/spell damage paths, with and without secondary damage.

  • Player↔Monster and Monster↔Player flows, including prey bonuses and party/impact trackers.

  • Test A: message snapshot parity (attacker/target/spectator)

  • Test B: hazard + reflection scenarios parity

Checklist

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I checked the PR checks reports
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works

[Copilot]

Pull Request Overview

This PR refactors the Combat and Game classes to establish clearer boundaries and reduce coupling between combat logic and game orchestration while preserving exact runtime behavior.

• Moved pure combat helpers from Game to Combat class under an anonymous namespace with minimal static entry points
• Created InternalGame helpers for block/reflect processing and string utilities within Game
• Changed applyImbuementElementalDamage signature from returning CombatDamage to in-place update with void return

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

File	Description
src/game/game.hpp	Removed combat helper method declarations that moved to Combat class
src/game/game.cpp	Added InternalGame namespace helpers and refactored to use Combat static methods
src/creatures/combat/combat.hpp	Added static method declarations for moved combat helpers
src/creatures/combat/combat.cpp	Implemented moved combat helpers with anonymous namespace organization

Comments suppressed due to low confidence (1)

src/game/game.cpp:1

• There appear to be duplicate calls to player->sendCancelMessage(RETURNVALUE_NOTPOSSIBLE) on lines 1933 and 1934. This will send the same cancel message twice to the player.

/**

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

The method signature shows removal of std::stringstream &ss parameter but this creates an inconsistency with the moved Combat methods which now use fmt::format internally. Consider updating the parameter list documentation or ensure all message building methods follow the same pattern.

[Copilot]

The item parameter should be const std::shared_ptr<Item> &item for consistency with other parameters and to avoid unnecessary copying since the method doesn't modify the shared_ptr itself.

Suggested change

	std::shared_ptr<Item> item,
	const std::shared_ptr<Item> &item,

[sonarqubecloud]

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

This PR is stale because it has been open 45 days with no activity.

[github-actions]

This PR is stale because it has been open 45 days with no activity.