Add animation.cfg/NLA import-export (from PR #69), pep8 CI, and NLA test coverage by mrwonko · Pull Request #120 · mrwonko/Blender-Jedi-Academy-Tools

mrwonko · 2026-07-26T10:16:27Z

Summary

Cherry-pick the 9 NLA-specific commits from PR Ghoul2 importer updates #69 (3a05dbc chief commit through its typo/type fixups), adapted to master's conventions (no ImportHelper mixin, assert-based None-narrowing, PointerProperty for the new g2_sequence_prop instead of raw bpy.props attribute assignment). Adds AnimationLoadMode.CFG: importing a .gla next to an animation.cfg now creates one Action/NLA strip per named sequence instead of dumping every frame into one Action, plus a matching NLA-strips-based animation.cfg exporter.
Two real bugs found by actually running the ported code (not just static analysis): a missing JAFilesystem.PathToFile helper the code depended on but the cherry-pick never brought over, and Action.slots/action_slot usage that crashes on Blender 4.1 (Action Slots only exist from 4.4+) -- both fixed.
A third bug found via the new tests: the first frame of every sequence after the first was corrupted (stale scene frame during active-action evaluation while NLA tracks already exist) -- fixed by mirroring the existing ALL-mode path's scene.frame_set() call.
make format/make pep8 targets + a new pep8 CI job, plus a real fix to .pep8 itself (ignore = E501 was silently replacing pycodestyle's entire default-ignore list rather than extending it) and the ~24 genuine violations that exposed in the actively-maintained core.
New blender-run-script skill (generic counterpart to blender-tests) for one-off headless Blender work, used to generate the new simpleskel_nla.blend fixture on Blender 4.1 (the add-on's minimum).
New case_nla_export/case_nla_roundtrip test cases, backed by a new animation.cfg fixture for simpleskel.gla (two sequences split at frame 10, with deliberately different fps per sequence so the export comparison isn't vacuous).

Test plan

.claude/skills/blender-tests/run_blender_tests.sh 4.1 5.2 -- all 9 cases pass on both
pyright --pythonpath "$(command -v python3)" -- clean
make pep8 -- clean
Manual review of simpleskel_nla.blend / NLA editor behavior in the Blender GUI

New cfg parsing option will add all animations into NLA tracks which is faster than just importing all frames the usual way. Also added an option to export either from timeline markers or from NLA strips. TODO: Add custom properties for sequence looping and fps to the actions and maybe add a custom panel to change these in NLA view (cherry picked from commit 3a05dbc)

Adds a panel in the nla editor, so editing the fps and looping from the overview of all animations is possible and added a second panel in the actions section of the dopesheet editor. So working on an action directly also shows the options directly. (cherry picked from commit 891c5d1)

(cherry picked from commit 52c9575)

Strips got action slots in 4.0 I think. These need to be set now else the animations won't play. (cherry picked from commit e335fce)

(cherry picked from commit 40d936b)

(cherry picked from commit 1964423)

(cherry picked from commit 459cc5f)

(cherry picked from commit 12cf8c6)

(cherry picked from commit 2bc46cb)

Add missing JAFilesystem.PathToFile (used by animation.cfg lookup but never ported), assert non-None where Optional bpy types are known present, and use a local animData variable instead of repeated armature.animation_data access.

format runs autopep8 in-place; pep8 runs pycodestyle in check mode against the same file set, reusing .pep8's existing config.

pep8 job mirrors typecheck's structure; added to nightly/release's needs list alongside smoke-test/typecheck.

.pep8's `ignore = E501` was replacing pycodestyle's own default ignore list rather than extending it, silently enabling E121/E123/E126/E226/ E24/E704/W503/W504 repo-wide. Restate those defaults alongside E501. Also hand-fix the remaining genuine violations (E711/E712/E713/E741, mixed tabs) in the actively-maintained core; ASE/ROFF/MD3/Patch stay excluded from format/pep8, matching pyrightconfig.json's existing exclusion of those unverified formats.

Generic counterpart to blender-tests, which only runs the fixed test suite. Needed to generate the simpleskel_nla.blend fixture.

…e 10 Covers all 21 existing frames (0-9, 10-20) for the upcoming NLA import/export test cases.

Action.slots/AnimData.action_slot/NlaStrip.action_slot were only introduced in Blender 4.4 -- the ported code assumed they always exist, breaking NLA-mode import on 4.1 (this add-on's declared minimum). Found by actually generating a fixture on 4.1.

Captures the NLA-track state animation.cfg/CFG-mode import produces for simpleskel.gla (two Actions/strips split at frame 10, per the animation.cfg fixture), for the upcoming NLA export/roundtrip test cases. Generated via the new blender-run-script skill on Blender 4.1 (the add-on's minimum) so it stays openable on every supported version.

…port With animData.action assigned directly (not via NLA tweak mode), pose.visual_transform_apply evaluates the active action at whatever frame the scene is currently on, not the frame passed to keyframe_insert -- leaving the scene's frame stale from the previous sequence corrupted exactly frame 0 of every subsequent sequence. Mirror the ALL-mode path's scene.frame_set() call. Found via case_nla_roundtrip failing only at the frame-10 sequence boundary; regenerated simpleskel_nla.blend with the fix.

Exercise the animation.cfg/NLA import-export path added by the recent cherry-picks: exporting NLA strips back to animation.cfg metadata, and round-tripping the NLA-materialized skeleton back to an identical .gla.

Both were 20, so case_nla_export's fps comparison would pass even if export mixed up or hardcoded per-sequence fps. test_seq_2 is now 24, making the roundtrip check on that field meaningful. Regenerated simpleskel_nla.blend to match.

Add a "next version" changelog entry, describe the new Cfg animation import mode, and document GLAMetaExport's NLA vs Markers source setting.

mrwonko · 2026-07-26T11:36:16Z

@@ -0,0 +1,3 @@
+// name          start   length  loop    fps
+test_seq_1        0       10      -1      20
+test_seq_2        10      11      -1      24


add a trailing comment that the fps differs deliberately

mrwonko · 2026-07-26T11:37:21Z

+import bpy
+
+sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), ".."))
+import testutil  # noqa: E402


document why the exception is necessary

mrwonko · 2026-07-26T11:42:20Z

    testutil.check(testutil.compare_glm(actual_glm, expected_glm) + testutil.compare_gla(actual_gla, expected_gla))


+def _load_animation_cfg(cfg_dir):


use type annotations for arguments and return values in this file

mrwonko · 2026-07-26T11:45:45Z

+    same per-frame bone transforms as the original file, since Blender's NLA stack evaluation
+    combines strips transparently regardless of how the animation is organized in the editor."""
+    import bpy
+    bpy.ops.wm.open_mainfile(filepath=os.path.join(TESTDATA, "simpleskel_nla.blend"))


the roundtrip should start from .gla + .cfg, import and re-export both, instead of using the .blend file

mrwonko · 2026-07-26T11:46:34Z

+    actual_by_name = {seq.name: seq for seq in export_cfg.sequences}
+    expected_by_name = {seq.name: seq for seq in expected_cfg.sequences}
+    if set(actual_by_name) != set(expected_by_name):
+        mismatches.append(
+            f"sequence names differ: actual={set(actual_by_name)} expected={set(expected_by_name)}")
+
+    for name in sorted(set(actual_by_name) & set(expected_by_name)):
+        actual_seq = actual_by_name[name]
+        expected_seq = expected_by_name[name]
+        for field in ("start_frame", "num_frames", "loop", "fps"):
+            actual_value = getattr(actual_seq, field)
+            expected_value = getattr(expected_seq, field)
+            if actual_value != expected_value:
+                mismatches.append(
+                    f"sequence '{name}': {field} differs: actual={actual_value} expected={expected_value}")


refactor this into a helper function similar to compare_gla

mrwonko · 2026-07-26T11:56:29Z

+        self.fps = -1
+
+    def __str__(self):
+        return "{name}\t\t{start}\t{frames}\t{loop}\t{fps}".format(


let's use an f-string

mrwonko · 2026-07-26T12:07:05Z

+    def from_cfg_line(cls, txt_line):
+        try:
+            # remove comments inline first, someone might have annotated these
+            line = txt_line.split("//")[0]


this parser is over simplified, a mid-token // does not initiate a comment, and there are also multi-line comments. Tokens may also be quoted. Numbers can have trailing garbage. I'm not sure if an entry must even stay on the same line?

Build a generator-based token parser and use that instead. Use https://github.com/mrwonko/ghoul2-browser-tools/blob/main/src/commonTokenizer.ts for reference, but note that we don't need to retain whitespace and comments here, we can just yield a string for each token. Unit-test the generic token parser using our fixture.

You can also look at mrwonko/ghoul2-browser-tools#28 for animation.cfg parsing reference, but it has not been reviewed yet, so don't trust it completely. The canonical source is BG_ParseAnimationFile in https://github.com/jedis/jediacademy/blob/master/codemp/game/bg_panimate.c, you already have a local copy of that repo at ~/jediacademy.

Do test-driven development: first add a unit test to persist the current parse result for our fixture, plus maybe an additional case for partial final line. Make sure it passes. Then adjust the parser in a separate commit and verify it does not regress.

mrwonko · 2026-07-26T12:10:36Z

+                    pose_bone = bones[index]
+                    # pose_bone.matrix = transformation * scaleMatrix
+                    pose_bone.matrix = transformation
+                    # in the _humanoid face, the scale gets changed. that messes the re-export up. FIXME: understand why. Is there a problem?


File an issue for this, if there isn't one already. Could it be connected to the bug where we incorrectly marked bones as connected? If so, it may be fixed, we should check.

mrwonko · 2026-07-26T12:14:04Z

+  Python formatter and pyright type checking at `standard` mode. `make format` runs `autopep8 --in-place`
+  over the same file set; `make pep8` runs `pycodestyle` in check-only mode (what CI's `pep8` job runs).
+
+## Writing comments, commit messages, and PR descriptions


this also extends to issues

mrwonko · 2026-07-26T12:17:26Z

+  \item Added a faster \texttt{.gla} animation import mode that reads sequences from an
+  \texttt{animation.cfg} file into separate NLA strips/actions, instead of importing every frame
+  as one large action.
+  \item The Ghoul 2 Animation Metadata exporter can now export from NLA strips, in addition to
+  timeline markers.


Credit SomaZ for these contributions.

animation.cfg: comment that test_seq_2's fps intentionally differs from test_seq_1's. generate_simpleskel_nla_blend.py: explain the E402 noqa like run_tests.py already does.

TYPE_CHECKING-guarded imports matching testutil.py's existing pattern; annotate every function's parameters and return type.

Add testutil.compare_animation_cfg (mirrors compare_gla/compare_glm), used by both tests instead of the ad-hoc diff previously inlined in case_nla_export. case_nla_export now also re-exports and checks the .gla (previously only checked animation.cfg). case_nla_roundtrip now builds its Blender state from simpleskel.gla + animation.cfg directly (the same CFG-mode import generate_simpleskel_nla_blend.py performs) instead of opening the pre-baked .blend fixture, and checks both re-exported artifacts too.

New case_animation_cfg_parse: fixture parse result + a synthetic partial-final-line case. Regression safety net before replacing the naive split()-based parser with a proper tokenizer.

The old txt_line.split("//")[0] + .split() parser didn't match the real engine (bg_panimate.c/q_shared.c COM_ParseExt): // and /* only start a comment as the first character(s) of a new token (not mid-token), quoted tokens can contain whitespace, numbers tolerate trailing garbage (atoi-style), and entries can span multiple lines. New common_tokenizer.py implements those rules as a generator; ships with the addon (added to Makefile's PY_FILES). load_from_cfg now reads the whole file once and pulls 5 tokens per entry instead of iterating line by line, which that per-line loop couldn't represent correctly for multi-line entries/comments in the first place. Extends case_animation_cfg_parse (added in the prior commit to pin current behavior) with the edge cases the old parser got wrong; confirmed passing with no regression on the cases it already covered.

CLAUDE.md's comments/commits/PR-descriptions section also covers issue descriptions now. Manual changelog credits SomaZ for the two NLA import/export bullets.

SomaZ and others added 21 commits July 26, 2026 11:26

Update makefile to include JAG2AnimationCFG.py

fdc2c30

(cherry picked from commit 52c9575)

Fix animation playing after import in Blender versions > 4.0

43e1526

Strips got action slots in 4.0 I think. These need to be set now else the animations won't play. (cherry picked from commit e335fce)

Fix AnimationCGF typo

fd017f7

(cherry picked from commit 40d936b)

Type AnimationCFG arguments

b487683

(cherry picked from commit 1964423)

Fix differnce typo

689ed66

(cherry picked from commit 459cc5f)

Add TODO about testing non-English animation.cfg export

646aaa6

(cherry picked from commit 12cf8c6)

fix seqence typo

c456d44

(cherry picked from commit 2bc46cb)

Fix pyright errors in ported NLA code

f0bba81

Add missing JAFilesystem.PathToFile (used by animation.cfg lookup but never ported), assert non-None where Optional bpy types are known present, and use a local animData variable instead of repeated armature.animation_data access.

Add make format/pep8 targets

9801d0e

format runs autopep8 in-place; pep8 runs pycodestyle in check mode against the same file set, reusing .pep8's existing config.

Add pep8 CI job, document make format/pep8, note succinctness convention

9e1b0e8

pep8 job mirrors typecheck's structure; added to nightly/release's needs list alongside smoke-test/typecheck.

Add blender-run-script skill for one-off headless Blender scripts

e4923c6

Generic counterpart to blender-tests, which only runs the fixed test suite. Needed to generate the simpleskel_nla.blend fixture.

Add animation.cfg fixture for simpleskel, two sequences split at fram…

b824736

…e 10 Covers all 21 existing frames (0-9, 10-20) for the upcoming NLA import/export test cases.

Add case_nla_export and case_nla_roundtrip test cases

0f00cab

Exercise the animation.cfg/NLA import-export path added by the recent cherry-picks: exporting NLA strips back to animation.cfg metadata, and round-tripping the NLA-materialized skeleton back to an identical .gla.

Give the two test sequences distinct fps values

ba71449

Both were 20, so case_nla_export's fps comparison would pass even if export mixed up or hardcoded per-sequence fps. test_seq_2 is now 24, making the roundtrip check on that field meaningful. Regenerated simpleskel_nla.blend to match.

Document animation.cfg/NLA import-export in the manual

6c412a4

Add a "next version" changelog entry, describe the new Cfg animation import mode, and document GLAMetaExport's NLA vs Markers source setting.

mrwonko commented Jul 26, 2026

View reviewed changes

mrwonko added 7 commits July 26, 2026 14:51

Address review: note deliberate fps difference, explain noqa

d9793eb

animation.cfg: comment that test_seq_2's fps intentionally differs from test_seq_1's. generate_simpleskel_nla_blend.py: explain the E402 noqa like run_tests.py already does.

Address review: add type annotations to tests/run_tests.py

a1e58bd

TYPE_CHECKING-guarded imports matching testutil.py's existing pattern; annotate every function's parameters and return type.

Pin current animation.cfg parser behavior ahead of tokenizer rewrite

b32817a

New case_animation_cfg_parse: fixture parse result + a synthetic partial-final-line case. Regression safety net before replacing the naive split()-based parser with a proper tokenizer.

Address review: use an f-string in AnimationSequence.__str__

eb25989

Address review: extend comment convention to issues, credit SomaZ

b095e1d

CLAUDE.md's comments/commits/PR-descriptions section also covers issue descriptions now. Manual changelog credits SomaZ for the two NLA import/export bullets.

mrwonko mentioned this pull request Jul 26, 2026

pose_bone.scale drifts during frame baking on some skeletons (e.g. _humanoid face) #122

Open

mrwonko mentioned this pull request Jul 26, 2026

animation.cfg: support float fps and negative fps (backwards playback) #123

Open

mrwonko merged commit 7700e03 into master Jul 26, 2026
7 checks passed

mrwonko deleted the nla-merge branch July 26, 2026 13:15

mrwonko mentioned this pull request Jul 26, 2026

Release v2.1.0 #124

Merged

Navigation Menu

Add animation.cfg/NLA import-export (from PR #69), pep8 CI, and NLA test coverage - #120

Add animation.cfg/NLA import-export (from PR #69), pep8 CI, and NLA test coverage#120
mrwonko merged 28 commits into
masterfrom
nla-merge

mrwonko commented Jul 26, 2026 •

edited

Loading

Uh oh!

mrwonko Jul 26, 2026

Uh oh!

mrwonko Jul 26, 2026

Uh oh!

mrwonko Jul 26, 2026

Uh oh!

mrwonko Jul 26, 2026

Uh oh!

mrwonko Jul 26, 2026

Uh oh!

mrwonko Jul 26, 2026

Uh oh!

mrwonko Jul 26, 2026

Uh oh!

mrwonko Jul 26, 2026

Uh oh!

mrwonko Jul 26, 2026

Uh oh!

mrwonko Jul 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		testutil.check(testutil.compare_glm(actual_glm, expected_glm) + testutil.compare_gla(actual_gla, expected_gla))


		def _load_animation_cfg(cfg_dir):

Conversation

mrwonko commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mrwonko commented Jul 26, 2026 •

edited

Loading