Adds new animation for shield bash#136
Conversation
WalkthroughThe Bash skill creation now accepts a Camera pointer and initializes tooltip and animation assets. The call site in skill_create is updated accordingly. Animation setup uses "Extras/ShieldBash.png" with defined frames and properties. A previous animation_run call in skill_bash is removed. No other public interfaces are changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Game as Game Loop
participant Skills as skill_create(t, cam)
participant Bash as create_bash(cam)
participant UI as Tooltip
participant Anim as Animation
Game->>Skills: Request create Skill(BASH, cam)
Skills->>Bash: create_bash(cam)
Bash->>UI: init bash_tooltip(cam)
Bash->>Anim: load "Extras/ShieldBash.png"
Bash->>Anim: configure frames, loop, sprite dim, clip, rotation point
Bash-->>Skills: return Skill{ tooltip, animation }
Skills-->>Game: Skill instance
note over Game,Skills: During use: removed direct sword animation_run call in skill_bash
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/skill.c (1)
583-584: Consistency nit: centralize tooltip creation pattern.Most skills create tooltips in skill_create, but Flurry and Bash do it in their create_* functions. Consider picking one pattern for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/Extras/ShieldBash.pngis excluded by!**/*.png
📒 Files selected for processing (1)
src/skill.c(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/skill.c (2)
src/tooltip.c (1)
tooltip_create(122-129)src/animation.c (3)
animation_create(25-37)animation_load_texture(39-43)animation_set_frames(88-94)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Ubuntu (mingw)
- GitHub Check: Ubuntu (GCC)
- GitHub Check: Mac (Clang)
- GitHub Check: Windows (MSVC)
- GitHub Check: Ubuntu (Clang)
- GitHub Check: Mac (GCC)
- GitHub Check: Analyze (c-cpp)
🔇 Additional comments (3)
src/skill.c (3)
568-576: Creation wiring looks good (tooltip + animation).Signature change to accept Camera, tooltip creation, and animation setup mirror Flurry’s pattern. Destruction path handles animation/tooltip. LGTM.
Also applies to: 583-601
1083-1083: Call site update matches new signature.create_bash(cam) aligns with the new create_bash(Camera*) signature.
587-597: Asset present and frame count correct: Extras/ShieldBash.png is 224×32, matching seven 32×32 frames.
| skill->tooltip = tooltip_create(bash_tooltip, cam); | ||
| skill->animation = animation_create(7); | ||
| Animation *a = skill->animation; | ||
|
|
||
| animation_load_texture(a, "Extras/ShieldBash.png", cam->renderer); | ||
| animation_set_frames(a, (AnimationClip[]) { | ||
| { 0, 0, 32, 32, 20 }, | ||
| { 32, 0, 32, 32, 20 }, | ||
| { 64, 0, 32, 32, 20 }, | ||
| { 96, 0, 32, 32, 20 }, | ||
| { 128, 0, 32, 32, 20 }, | ||
| { 160, 0, 32, 32, 20 }, | ||
| { 192, 0, 32, 32, 20 }, | ||
| }); | ||
| a->loop = false; | ||
| a->sprite->dim = GAME_DIMENSION; | ||
| a->sprite->clip = (SDL_Rect) { 0, 0, 32, 32 }; | ||
| a->sprite->rotationPoint = (SDL_Point) { 16, 16 }; | ||
| return skill; |
There was a problem hiding this comment.
New animation is never triggered; run it on use and bind to player.
You set up skill->animation but don’t start it. Unless there’s a framework hook that auto-runs skill animations, the Shield Bash animation won’t play.
Proposed fix: trigger it in skill_bash after check_skill_validity and bind sprite pos/angle to the player.
static bool
skill_bash(Skill *skill, SkillData *data)
{
- UNUSED (skill);
+ // Run shield bash animation anchored to the player
+ // (after check_skill_validity, so player_turn has set facing/angle)
Position playerPos, targetPos;
if (!check_skill_validity(&playerPos, &targetPos, data)) {
return false;
}
+
+ if (skill && skill->animation) {
+ Animation *a = skill->animation;
+ a->sprite->pos = data->player->sprite->pos;
+ a->sprite->angle = data->player->sprite->angle;
+ animation_run(a);
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/skill.c around lines 583 to 601, you create and configure
skill->animation but never start it or bind its sprite to the player, so the
Shield Bash frames never play; modify the skill_bash handler (after the
check_skill_validity return path) to call the animation start function on
skill->animation (e.g., set current frame/time and mark playing or call
animation_play) and set the animation's sprite position and rotationPoint to the
player's sprite position/angle (bind/update on each tick or set a reference) so
the animation is triggered and follows the player when the skill is used.
1 participant
Summary by CodeRabbit
New Features
Enhancements