Adds new backstab, erupt and vampiric blow animations #141
Conversation
WalkthroughAdds a new SUCK sound effect to the mixer and updates the Fx enum. Makes several skill constructors camera-aware, exposes per-skill animation_properties (offset, on_player), updates skill creation to pass Camera, and changes player skill animation placement and sword frame timing. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Player
participant PlayerLogic as Player Logic
participant Skill as Skill (animation_properties)
participant Camera as Camera
participant Mixer as Mixer
rect rgba(230,245,255,0.6)
note right of Skill: Creation (camera-aware)
PlayerLogic->>Skill: create_* (Camera*)
Skill-->>Camera: use renderer for tooltip/animation init
end
rect rgba(240,255,240,0.6)
note right of PlayerLogic: Skill use flow
Player->>PlayerLogic: use_skill(skill)
PlayerLogic->>Skill: read animation_properties
alt on_player == true
PlayerLogic->>PlayerLogic: pos = player sprite position
else on_player == false
PlayerLogic->>PlayerLogic: pos = sword animation position/orientation
end
PlayerLogic->>PlayerLogic: pos += animation_properties.offset
PlayerLogic->>Mixer: play effect (e.g., SUCK)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-10-10T20:25:20.892ZApplied to files:
🧬 Code graph analysis (1)src/skill.c (3)
⏰ 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)
🔇 Additional comments (5)
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 |
Also updates sound for vampiric blow and modified the timer for the sword animation. These seem a bit off though...
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/skill.c (1)
1070-1104: Erupt animation pivot likely miscenteredRotation point for a 64x64 clip should be center (32,32). Using {64,64} sets pivot to bottom-right and can skew placement when rotation is applied. Offset looks right for 96x96 scaling.
a->sprite->clip = (SDL_Rect) { 0, 0, 64, 64 }; - a->sprite->rotationPoint = (SDL_Point) { 64, 64 }; + a->sprite->rotationPoint = (SDL_Point) { 32, 32 };
🧹 Nitpick comments (3)
src/skill.h (1)
71-73: Ensure Camera is forward‑declared to keep header self‑containedskill_create now references Camera*. Add a forward declaration here to avoid include-order surprises.
typedef struct Player Player; +typedef struct Camera Camera; Skill* skill_create(enum SkillType, Camera *cam);src/player.c (1)
493-505: Reset angle/flip when rendering on player to avoid stale orientationIf on_player, previous angle/flip may bleed over between uses. Reset explicitly.
- if (skill->animation_properties.on_player) { - a->sprite->pos = player->sprite->pos; + if (skill->animation_properties.on_player) { + a->sprite->pos = player->sprite->pos; + a->sprite->angle = 0; + a->sprite->flip = SDL_FLIP_NONE;src/skill.c (1)
724-759: Backstab animation: check alignment; consider explicit offsetDepending on sheet art, you may need a small offset to match the swapped position. Test in-game and set animation_properties.offset if misaligned.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
assets/Extras/AcidSplash.pngis excluded by!**/*.pngassets/Extras/BackStab.pngis excluded by!**/*.pngassets/Extras/VampiricBlow.pngis excluded by!**/*.pngassets/Sounds/FX/suck.wavis excluded by!**/*.wav
📒 Files selected for processing (5)
src/mixer.c(1 hunks)src/mixer.h(1 hunks)src/player.c(2 hunks)src/skill.c(11 hunks)src/skill.h(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/skill.c (3)
src/mixer.c (1)
mixer_play_effect(191-202)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 (Clang)
- GitHub Check: Analyze (c-cpp)
- GitHub Check: Windows (MSVC)
- GitHub Check: Mac (GCC)
- GitHub Check: Ubuntu (GCC)
- GitHub Check: Mac (Clang)
🔇 Additional comments (8)
src/skill.h (1)
65-69: animation_properties addition: LGTMClear defaultable fields for positioning. No issues spotted.
src/player.c (1)
585-590: Sword swing slowed (20 → 50 ms): confirm intended pacingThis lengthens the swing by 2.5x and may feel sluggish. Confirm this is deliberate or consider 30–40 ms for responsiveness, or make it a constant.
src/skill.c (4)
311-313: Good defaults for animation_propertiesSane zero offset and target-anchored rendering by default.
382-411: Vampiric blow animation wiring: OKCamera-aware ctor, frames, and non-loop setup look correct.
Visually verify alignment with sword swing in all four directions (no offset set). Adjust skill->animation_properties.offset if it appears a few pixels off.
1102-1104: Nice: on_player and offset set for eruptThis matches a centered AoE. Keep this pattern for future on-player effects.
1116-1146: Camera-aware skill_create calls: OKSignatures and usage are consistent.
src/mixer.c (1)
112-114: SUCK effect: asset and usage verifiedassets/Sounds/FX/suck.wav exists and mixer_play_effect(SUCK) is invoked in src/skill.c.
src/mixer.h (1)
74-82: No numeric coupling detected; safe to insert SUCK enum value
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/skill.c (3)
382-411: Vampiric blow: camera-aware constructor and animation — please verify asset and timingConstruction flow and 6-frame animation wiring look correct. Two quick checks:
- Verify Extras/VampiricBlow.png exists and loads.
- 120 ms/frame may feel slow (total ~720 ms). If pacing feels off, try 60–90 ms.
724-759: Backstab animation added — consider tuning frame durationFrames are set to 100 ms (total ~900 ms). If it feels sluggish, consider 60–80 ms to better match a snappy stab.
1070-1103: Erupt animation placement OK; add rotationPoint for consistencyNice use of on_player=true with a centering offset and scaled dim. To keep transforms centered (future rotations/scales), set the sprite rotationPoint to the frame center.
Apply this diff:
a->sprite->clip = (SDL_Rect) { 0, 0, 64, 64 }; + a->sprite->rotationPoint = (SDL_Point) { 32, 32 }; skill->animation_properties.offset = POS(-32, -32); skill->animation_properties.on_player = true;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/skill.c(11 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-10T20:25:20.892Z
Learnt from: LiquidityC
PR: Oliveshark/breakhack#141
File: src/skill.c:343-346
Timestamp: 2025-10-10T20:25:20.892Z
Learning: In the vampiric_blow_skill function in src/skill.c, the SUCK sound effect should play on every vampiric blow attempt, even on misses. This is an intentional design decision for audio feedback.
Applied to files:
src/skill.c
🧬 Code graph analysis (1)
src/skill.c (3)
src/mixer.c (1)
mixer_play_effect(191-202)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: Mac (Clang)
- GitHub Check: Ubuntu (mingw)
- GitHub Check: Analyze (c-cpp)
- GitHub Check: Ubuntu (Clang)
- GitHub Check: Ubuntu (GCC)
- GitHub Check: Mac (GCC)
- GitHub Check: Windows (MSVC)
🔇 Additional comments (3)
src/skill.c (3)
311-313: Default animation_properties initialization looks goodZero-offset and on_player=false as defaults are sensible. Confirm upstream code in use_skill respects these defaults as “target-tile” placement when on_player=false.
343-346: Intentional SUCK audio on every attemptPlaying SUCK before hit check matches the stated design (audio even on miss). No change requested.
Based on learnings
1115-1115: skill_create call sites correctly updated with Camera
Verified all invocations passcam.
Also updates sound for vampiric blow and modified the timer for the
sword animation.
These seem a bit off though...
Summary by CodeRabbit
New Features
Improvements