Skip to content

053 | State - Part 5: Refactor Animation Logic#53

Open
Pedrohamoura-Git wants to merge 4 commits intofeat/068from
feat/069
Open

053 | State - Part 5: Refactor Animation Logic#53
Pedrohamoura-Git wants to merge 4 commits intofeat/068from
feat/069

Conversation

@Pedrohamoura-Git
Copy link
Copy Markdown
Contributor

@Pedrohamoura-Git Pedrohamoura-Git commented Mar 28, 2026

Information

Describe What Was Done

  • Update animation logic to use ´state´ component to find the correct sprite.
  • Fix movement system so that it allows the character to move in case it's in the DEAD state.

Detailed Summary (AI Generated)

🤖| Summary

This Pull Request refactors existing game character animation configurations, components, and systems to use standardized state constants. Additionally, it introduces minor updates to corresponding game logic for improved consistency and handling.


Refactor: Standardization of Character State Keys

  • Updated BASE_CHARACTER_SPRITES in characterSpriteModel.ts to replace hardcoded animation state keys with dynamic constants imported from CHARACTER_STATE.
    • Examples of replaced keys: 'IDLE'CHARACTER_STATE.IDLE, 'RUN'CHARACTER_STATE.RUN, and others.
  • Imported CHARACTER_STATE from the appropriate module to align animation configuration constants across the codebase.

Changes to Animation System

  • Updated AnimationSystem.ts:
    • Replaced animation.currentState with state.characterState to align with the new state management structure.
    • Added a check to ensure the state property exists before updating animation behavior.

Changes to Movement System

  • Updated MovementSystem.ts:
    • Introduced a new condition to check if the entity's state.aliveState is equal to ALIVE_STATE.ALIVE. Entities not marked as alive are skipped during the update process.
    • Imported ALIVE_STATE from constants to determine the entity's alive state.

Other Updates

  • Removed the unused currentState property from the AnimationComponent interface, as the system now relies on the unified state.characterState for managing animation states.

This refactor enhances the maintainability and readability of the game code by centralizing state definitions and improving consistency in their usage across animation and movement systems. No functional behavior changes were introduced.


Diagrams

📈 | Flowchart - Nothing to show yet

Evidence

⏪️ | Before - No animations
Gravacao.de.Tela.2026-01-31.193506.mp4
⏩ | After - Animations working
State.-.All.animations.working.mp4

Rollback Plan

Revert from the branch.


↗️ | Click to See The Preview

Copilot AI review requested due to automatic review settings March 28, 2026 12:46
@Pedrohamoura-Git Pedrohamoura-Git changed the base branch from main to feat/068 March 28, 2026 12:46
@Pedrohamoura-Git Pedrohamoura-Git self-assigned this Mar 28, 2026
@Pedrohamoura-Git Pedrohamoura-Git added refactor Refactor code client Changes in the client package labels Mar 28, 2026
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 continues the ECS “state” refactor by making gameplay systems (movement/animation) respect the entity’s StateComponent, and by aligning sprite-model animation keys with the canonical CHARACTER_STATE constants.

Changes:

  • Gate movement intent updates on state.aliveState === ALIVE in MovementSystem.
  • Drive animation selection from state.characterState in AnimationSystem and remove AnimationComponent.currentState.
  • Update CHARACTERS_SPRITES_MODEL base sprite definitions to use CHARACTER_STATE.* constants instead of raw string literals.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
packages/client/src/game/ecs/systems/movement/MovementSystem.ts Skips movement updates for dead entities by checking state.aliveState.
packages/client/src/game/ecs/systems/animation/AnimationSystem.ts Chooses animation based on state.characterState rather than an animation-local state field.
packages/client/src/game/ecs/components/animation/AnimationComponent.ts Removes currentState from the animation component shape.
packages/client/src/game/config/constants/character/sprite-model/characterSpriteModel.ts Uses CHARACTER_STATE constants for sprite config keys (typed as CharacterState).

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

@Pedrohamoura-Git Pedrohamoura-Git changed the title 050 | State - Part 5: 053 | State - Part 5: Refactor Animation Logic Mar 28, 2026
@Pedrohamoura-Git Pedrohamoura-Git marked this pull request as ready for review March 28, 2026 13:10
Copy link
Copy Markdown
Member

@anatfernandes anatfernandes left a comment

Choose a reason for hiding this comment

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

I believe these changes should not be implemented as they break animations for all other entities. The AnimationSystem was designed to be generic and reusable, coupling it specifically to player states prevents other animated entities from functioning correctly (or forces them to implement unnecessary states just to stay functional).

To fix the player's animations, the individual state handlers should be responsible for updating the animation.currentState. This prevents system-level coupling and ensures all animations remain decoupled and functional.

entities.forEach(({ input, movement }) => {
if (!input || !movement) return;
entities.forEach(({ input, movement, state }) => {
if (!input || !movement || !state || state.aliveState !== ALIVE_STATE.ALIVE) return;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With this additional validation, the MovementSystem becomes restricted once again to moving only players. This prevents other moving entities, such as platforms or projectiles, from functioning correctly. Since this system is designed to be generic and reusable, I recommend removing this check and handling death as a separate concern, perhaps in future tasks focused on respawn and health mechanics.

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

Labels

client Changes in the client package refactor Refactor code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants