Skip to content

Add missing level 3 definition for Stubborn skill - #5

Merged
RobGruhl merged 1 commit into
mainfrom
claude/fix-stubborn-level-3-visibility-011CUpFX25bBYpf9gJLU6TUg
Nov 5, 2025
Merged

Add missing level 3 definition for Stubborn skill#5
RobGruhl merged 1 commit into
mainfrom
claude/fix-stubborn-level-3-visibility-011CUpFX25bBYpf9gJLU6TUg

Conversation

@RobGruhl

@RobGruhl RobGruhl commented Nov 5, 2025

Copy link
Copy Markdown
Owner

The Stubborn skill was defined with maxLevel 3 for Balcor (and other characters) but only had levels 1 and 2 in the skill definition. This caused level 3 to be selectable but not visible in the UX.

Added level 3 as a reaction ability: "Use after being targeted with an attack. Ignore all hits scored, and then make a Move action and an Assault action in any order."

Fixes visibility issue for Balcor's Stubborn level 3.

The Stubborn skill was defined with maxLevel 3 for Balcor (and other characters)
but only had levels 1 and 2 in the skill definition. This caused level 3 to be
selectable but not visible in the UX.

Added level 3 as a reaction ability: "Use after being targeted with an attack.
Ignore all hits scored, and then make a Move action and an Assault action in any order."

Fixes visibility issue for Balcor's Stubborn level 3.
@RobGruhl
RobGruhl merged commit 46a2bc2 into main Nov 5, 2025
1 check passed
@claude

claude Bot commented Nov 5, 2025

Copy link
Copy Markdown

PR Review: Add missing level 3 definition for Stubborn skill

Summary

This PR adds the missing level 3 definition for the Stubborn skill, which was previously defined with maxLevel: 3 for Balcor and other characters but only had levels 1-2 in the skill definition. This caused level 3 to be selectable but not visible in the UX.

Code Quality: ✅ Good

Positives:

  • The JSON structure is well-formed and properly indented
  • The added level follows the existing schema pattern consistently
  • The change is minimal and focused on fixing the specific issue

Schema Consistency: ⚠️ Issue Found

Problem: The new level 3 uses "variant": "active" while other reaction-type skills in the codebase use "type": "reaction" at the skill level and don't have a "variant" field in individual levels.

Looking at other reaction skills (Ambush, Counter Shot, Disarm, Evade, etc.), they follow this pattern:

{
  "id": "evade",
  "name": "Evade", 
  "type": "reaction",
  "levels": [
    {
      "level": 1,
      "isPassive": false,
      "isReaction": true,
      "trigger": "...",
      "effect": "...",
      "pegCost": 1
    }
  ]
}

However, the Stubborn skill uses:

  • "type": "mixed" at the skill level (appropriate since it has both standard and reaction abilities)
  • "variant": "active" in each level
  • "description" instead of separate "trigger" and "effect" fields

Recommendation: Since levels 1-2 already use this pattern, the new level 3 is consistent with the existing Stubborn skill structure. However, you may want to consider:

  1. Whether "mixed" type skills intentionally use a different schema than pure "reaction" skills
  2. If level 3 should use separate "trigger" and "effect" fields for better UX clarity (e.g., "Use after being targeted with an attack" as trigger, and the rest as effect)

Functional Review: ✅ Good

Positives:

  • Fixes the visibility issue as described (docs/data/corespace-data.json:2362-2370)
  • The ability progression makes sense: Level 1 (Move) → Level 2 (Move + Assault) → Level 3 (Reaction: ignore hits, Move + Assault)
  • isReaction: true is correctly set for level 3
  • pegCost: 1 is consistent with other levels

Test Coverage: ⚠️ No Tests Found

Observation:

  • No test files were found in the repository
  • For a data-driven application, consider adding JSON schema validation tests
  • Could add tests to verify:
    • All skills with maxLevel: 3 have 3 level definitions
    • Required fields are present in all skill levels
    • Reaction skills have appropriate trigger/effect or description fields

Security: ✅ No Concerns

This is a static data file with no security implications.

Performance: ✅ No Concerns

Adding 9 lines to a JSON file has negligible performance impact.

Recommendations:

  1. Consider schema clarification - Document whether "mixed" type skills should use description vs trigger/effect fields
  2. Add validation tests - Prevent similar issues in the future by validating that all skill maxLevel values have corresponding level definitions
  3. Consider data integrity check - Run a script to verify all characters' skill references point to valid skill definitions with the specified max levels

Overall: ✅ Approved with suggestions

This is a solid bug fix that resolves the immediate issue. The code quality is good and the change is safe to merge. The suggestions above are for future improvements to prevent similar issues.

@RobGruhl
RobGruhl deleted the claude/fix-stubborn-level-3-visibility-011CUpFX25bBYpf9gJLU6TUg branch November 24, 2025 23:56
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