Skip to content

[code-simplifier] Code quality improvements#154

Merged
norrietaylor merged 4 commits intomainfrom
claude/issue-73-20260304-0300
Mar 28, 2026
Merged

[code-simplifier] Code quality improvements#154
norrietaylor merged 4 commits intomainfrom
claude/issue-73-20260304-0300

Conversation

@norrietaylor
Copy link
Copy Markdown
Owner

Applies code simplifications from the Code Simplifier analysis that failed to create a PR automatically (issue #73).

  • Replace hardcoded Player tag string with a constant in Collectible.cs
  • Use Mathf.Clamp instead of two-step clamp in PlayerHealth.cs
  • Remove unused [SerializeField] from _currentLevelName in LevelManager.cs
  • Replace magic int attack type with AttackKind enum in PlayerCombat.cs

Generated with Claude Code

Copilot AI review requested due to automatic review settings March 4, 2026 05:27
@norrietaylor
Copy link
Copy Markdown
Owner Author

@claude[agent] resolve conflicts in this branch

@Claude
Copy link
Copy Markdown
Contributor

Claude AI commented Mar 4, 2026

@norrietaylor I've opened a new pull request, #155, to work on those changes. Once the pull request is ready, I'll request review from you.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR applies a set of small, mechanical code-simplification improvements across core gameplay scripts (player health/combat, level management, and collectibles) based on prior “Code Simplifier” analysis.

Changes:

  • Simplifies health bounds logic by using Mathf.Clamp for both damage and healing.
  • Replaces a magic-number “attack type” integer with a strongly-typed AttackKind enum while keeping the Animator-facing API as an int.
  • Removes an unused serialized level-name field configuration and replaces a hardcoded "Player" tag literal with a local constant.

Reviewed changes

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

File Description
Unity/Assets/Scripts/Player/PlayerHealth.cs Uses Mathf.Clamp to enforce health bounds in a single expression for damage/heal.
Unity/Assets/Scripts/Player/PlayerCombat.cs Introduces AttackKind enum and updates combat state tracking while preserving AttackType int for Animator.
Unity/Assets/Scripts/Levels/LevelManager.cs Removes unused [SerializeField]/header from _currentLevelName (now set from active scene at runtime).
Unity/Assets/Scripts/Collectibles/Collectible.cs Replaces hardcoded "Player" tag string with a class constant for maintainability.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Collectible.cs: replace hardcoded Player string with a named constant
- PlayerHealth.cs: use Mathf.Clamp instead of two separate clamp operations
- LevelManager.cs: remove [SerializeField] from _currentLevelName (always overwritten in Start)
- PlayerCombat.cs: replace magic int attack type with AttackKind enum for type safety

Fixes #73

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@norrietaylor
Copy link
Copy Markdown
Owner Author

@claude this branch is failing the unity test suite

@claude
Copy link
Copy Markdown

claude Bot commented Mar 5, 2026

Claude finished @norrietaylor's task in 1m 44s —— View job


Fixed Unity Test Suite Failure

  • Check CI status and failure details
  • Identify issue: PlayerAnimator.cs:75 calls _animator.SetInteger(ATTACK_TYPE, _combat.AttackType)Animator.SetInteger requires an int, but AttackType now returns AttackKind (an enum) after the PR's changes, causing a compilation error
  • Fix PlayerAnimator.cs to cast AttackKind to int: (int)_combat.AttackType
  • Pushed fix (commit 8313106)

Root cause: The PR changed PlayerCombat.AttackType from int to AttackKind (enum), but PlayerAnimator.cs was not updated — it still passed the enum value directly to Animator.SetInteger() which expects an int. The fix adds an explicit (int) cast, which is safe since the AttackKind enum values are None=0, Punch=1, Kick=2, Stomp=3 — exactly what the Animator expects.
| Branch

…mator

The AttackType property on PlayerCombat was changed from int to AttackKind
enum, but PlayerAnimator.SetInteger requires an int. Cast to int to fix
the compilation error.

Co-authored-by: Norrie Taylor <norrietaylor@users.noreply.github.com>
@norrietaylor norrietaylor merged commit 877d858 into main Mar 28, 2026
3 checks passed
@norrietaylor norrietaylor deleted the claude/issue-73-20260304-0300 branch March 28, 2026 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants