Skip to content

GH-1354 Modern Godot Animations not importing Properly - #1355

Open
JoltedJon wants to merge 3 commits into
Redot-Engine:masterfrom
JoltedJon:GH-1354
Open

GH-1354 Modern Godot Animations not importing Properly#1355
JoltedJon wants to merge 3 commits into
Redot-Engine:masterfrom
JoltedJon:GH-1354

Conversation

@JoltedJon

@JoltedJon JoltedJon commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Fixes #1354

Summary by CodeRabbit

  • Bug Fixes
    • Improved animation library property handling, including clearer access to individual libraries.
    • Corrected loop detection for linear and ping-pong animations so loops trigger only after crossing playback boundaries.
    • Refined approximate-equality checks for more precise arithmetic and floating-point behavior.

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6144ff3c-9613-43da-aab6-967fd7e21cd2

📥 Commits

Reviewing files that changed from the base of the PR and between c614335 and ddb1a76.

📒 Files selected for processing (3)
  • core/math/math_funcs.h
  • scene/animation/animation_mixer.cpp
  • scene/animation/animation_player.cpp

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


Walkthrough

The change makes approximate-equality checks strict at the tolerance boundary. It also exposes animation libraries as individual properties and uses strict position comparisons for animation loop crossings.

Changes

Approximate comparison semantics

Layer / File(s) Summary
Strict approximate-equality checks
core/math/math_funcs.h
Arithmetic and floating-point is_equal_approx overloads now reject differences exactly equal to the tolerance.

Animation property and playback behavior

Layer / File(s) Summary
Per-library animation properties
scene/animation/animation_mixer.cpp
libraries matches only the whole-library property. Per-library paths now set, retrieve, and list individual AnimationLibrary values unconditionally.
Exact loop boundary detection
scene/animation/animation_player.cpp
Linear and pingpong playback now detect loop crossings and direction changes with strict comparisons against animation boundaries.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🔵 Low · up to ddb1a

Per-library animation replacement can remove the existing entry before a new one is accepted, so a rejected replacement may silently leave animation data missing. The impact is localized, but this bounded correctness risk warrants owner awareness or a failure-atomic fix before treating the change as fully merge-ready.

Suggested reviewers: mcdubhghlas

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the reported issue with modern Godot animations failing to import and references issue #1354.
Linked Issues check ✅ Passed The changes address animation library property handling and animation playback boundary detection. These changes support importing and preserving animation data from modern Godot 4.x assets, including…
Out of Scope Changes check ✅ Passed All changes concern animation library handling, animation playback behavior, or the approximate-equality logic used by those paths. No unrelated changes are present.
Full details: Linked Issues check

Explanation

The changes address animation library property handling and animation playback boundary detection. These changes support importing and preserving animation data from modern Godot 4.x assets, including the reported Flame FX case [#1354].

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@JoltedJon
JoltedJon marked this pull request as ready for review August 28, 2026 21:44
@JoltedJon
JoltedJon requested review from a team August 28, 2026 21:44
@JoltedJon

Copy link
Copy Markdown
Contributor Author

The user who reported this to me said that this PR fixed the bug that they were experiencing

@DaveTheEggman DaveTheEggman left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, just don't forget to squash the commits

@OldDev78 OldDev78 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

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

Projects

Status: Open

Development

Successfully merging this pull request may close these issues.

Import issues of Modern 4.x Godot Assets

5 participants