Skip to content

Commit 4a60066

Browse files
SnaveSutitSnaveSutit
committed
🐛 Run root function keyframes in storage animations
- Storage mode's apply_frame never called them, so they silently did nothing - Root functions now go through each frame's frame_effects, after Variants and texture slots, for every animation mode
1 parent 279dbe5 commit 4a60066

8 files changed

Lines changed: 81 additions & 82 deletions

File tree

‎src/systems/datapackCompiler/1.20.4/main.mcb‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ IF (has_animations) {
421421
node => node.type === 'locator' && global.frame.node_transforms[node.uuid]
422422
)
423423
%%>
424-
IF (global.modified_effect_nodes.length > 0 || global.frame.function || getFrameEffects(animation).some(effects => effects.frame === animation.frames.length - 1)) {
424+
IF (global.modified_effect_nodes.length > 0 || getFrameEffects(animation).some(effects => effects.frame === animation.frames.length - 1)) {
425425
function last_frame_effects {
426426
REPEAT(global.modified_effect_nodes) as node {
427427
<%%
@@ -457,12 +457,6 @@ IF (has_animations) {
457457
function ../frame_effects/<%animation.frames.length - 1%>
458458
}
459459

460-
IF (global.frame.function) {
461-
execute <%global.frame.function_execute_condition ? global.frame.function_execute_condition + ' ' : ''%>run \
462-
block <%animation.duration%>_function_keyframe {
463-
<%global.frame.function%>
464-
}
465-
}
466460
}
467461
}
468462

@@ -612,12 +606,6 @@ IF (has_animations) {
612606
;hasFunction = true
613607
}
614608

615-
// Root function keyframes.
616-
if (frame.function) {
617-
const execute_condition = frame.function_execute_condition ? frame.function_execute_condition + ' ' : ''
618-
frameFuncEnd += `\nexecute unless entity @s[tag=${TAGS.TRANSFORMS_ONLY()}] at @s ${execute_condition}run block ${frameIndex}_root_function%NEWLINE_PATCH%{\n${frame.function}\n}`
619-
;hasFunction = true
620-
}
621609
frameFunc += frameFuncEnd;
622610
if (frameFunc.length > 0) {
623611
frameFunc = `function ${frameIndex}%NEWLINE_PATCH%{${frameFunc}\n}`
@@ -654,6 +642,13 @@ IF (has_animations) {
654642
function <%blueprint_id%>/texture_slots/<%slot.slot%>/<%slot.texture%>
655643
}
656644
}
645+
IF (effects.function) {
646+
execute at @s <%effects.function_condition%>run {
647+
<%%
648+
emit.mcb(effects.function)
649+
%%>
650+
}
651+
}
657652
}
658653
}
659654
}

‎src/systems/datapackCompiler/1.20.5/main.mcb‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ IF (has_animations) {
423423
node => node.type === 'locator' && global.frame.node_transforms[node.uuid]
424424
)
425425
%%>
426-
IF (global.modified_effect_nodes.length > 0 || global.frame.function || getFrameEffects(animation).some(effects => effects.frame === animation.frames.length - 1)) {
426+
IF (global.modified_effect_nodes.length > 0 || getFrameEffects(animation).some(effects => effects.frame === animation.frames.length - 1)) {
427427
function last_frame_effects {
428428
REPEAT(global.modified_effect_nodes) as node {
429429
<%%
@@ -459,12 +459,6 @@ IF (has_animations) {
459459
function ../frame_effects/<%animation.frames.length - 1%>
460460
}
461461

462-
IF (global.frame.function) {
463-
execute <%global.frame.function_execute_condition ? global.frame.function_execute_condition + ' ' : ''%>run \
464-
block <%animation.duration%>_function_keyframe {
465-
<%global.frame.function%>
466-
}
467-
}
468462
}
469463
}
470464

@@ -614,12 +608,6 @@ IF (has_animations) {
614608
;hasFunction = true
615609
}
616610

617-
// Root function keyframes.
618-
if (frame.function) {
619-
const execute_condition = frame.function_execute_condition ? frame.function_execute_condition + ' ' : ''
620-
frameFuncEnd += `\nexecute unless entity @s[tag=${TAGS.TRANSFORMS_ONLY()}] at @s ${execute_condition}run block ${frameIndex}_root_function%NEWLINE_PATCH%{\n${frame.function}\n}`
621-
;hasFunction = true
622-
}
623611
frameFunc += frameFuncEnd;
624612
if (frameFunc.length > 0) {
625613
frameFunc = `function ${frameIndex}%NEWLINE_PATCH%{${frameFunc}\n}`
@@ -656,6 +644,13 @@ IF (has_animations) {
656644
function <%blueprint_id%>/texture_slots/<%slot.slot%>/<%slot.texture%>
657645
}
658646
}
647+
IF (effects.function) {
648+
execute at @s <%effects.function_condition%>run {
649+
<%%
650+
emit.mcb(effects.function)
651+
%%>
652+
}
653+
}
659654
}
660655
}
661656
}

‎src/systems/datapackCompiler/1.21.0/main.mcb‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ IF (has_animations) {
423423
node => node.type === 'locator' && global.frame.node_transforms[node.uuid]
424424
)
425425
%%>
426-
IF (global.modified_effect_nodes.length > 0 || global.frame.function || getFrameEffects(animation).some(effects => effects.frame === animation.frames.length - 1)) {
426+
IF (global.modified_effect_nodes.length > 0 || getFrameEffects(animation).some(effects => effects.frame === animation.frames.length - 1)) {
427427
function last_frame_effects {
428428
REPEAT(global.modified_effect_nodes) as node {
429429
<%%
@@ -459,12 +459,6 @@ IF (has_animations) {
459459
function ../frame_effects/<%animation.frames.length - 1%>
460460
}
461461

462-
IF (global.frame.function) {
463-
execute <%global.frame.function_execute_condition ? global.frame.function_execute_condition + ' ' : ''%>run \
464-
block <%animation.duration%>_function_keyframe {
465-
<%global.frame.function%>
466-
}
467-
}
468462
}
469463
}
470464

@@ -614,12 +608,6 @@ IF (has_animations) {
614608
;hasFunction = true
615609
}
616610

617-
// Root function keyframes.
618-
if (frame.function) {
619-
const execute_condition = frame.function_execute_condition ? frame.function_execute_condition + ' ' : ''
620-
frameFuncEnd += `\nexecute unless entity @s[tag=${TAGS.TRANSFORMS_ONLY()}] at @s ${execute_condition}run block ${frameIndex}_root_function%NEWLINE_PATCH%{\n${frame.function}\n}`
621-
;hasFunction = true
622-
}
623611
frameFunc += frameFuncEnd;
624612
if (frameFunc.length > 0) {
625613
frameFunc = `function ${frameIndex}%NEWLINE_PATCH%{${frameFunc}\n}`
@@ -656,6 +644,13 @@ IF (has_animations) {
656644
function <%blueprint_id%>/texture_slots/<%slot.slot%>/<%slot.texture%>
657645
}
658646
}
647+
IF (effects.function) {
648+
execute at @s <%effects.function_condition%>run {
649+
<%%
650+
emit.mcb(effects.function)
651+
%%>
652+
}
653+
}
659654
}
660655
}
661656
}

‎src/systems/datapackCompiler/1.21.2/main.mcb‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ IF (has_animations) {
424424
node => node.type === 'locator' && global.frame.node_transforms[node.uuid]
425425
)
426426
%%>
427-
IF (global.modified_effect_nodes.length > 0 || global.frame.function || getFrameEffects(animation).some(effects => effects.frame === animation.frames.length - 1)) {
427+
IF (global.modified_effect_nodes.length > 0 || getFrameEffects(animation).some(effects => effects.frame === animation.frames.length - 1)) {
428428
function last_frame_effects {
429429
REPEAT(global.modified_effect_nodes) as node {
430430
<%%
@@ -460,12 +460,6 @@ IF (has_animations) {
460460
function ../frame_effects/<%animation.frames.length - 1%>
461461
}
462462

463-
IF (global.frame.function) {
464-
execute <%global.frame.function_execute_condition ? global.frame.function_execute_condition + ' ' : ''%>run \
465-
block <%animation.duration%>_function_keyframe {
466-
<%global.frame.function%>
467-
}
468-
}
469463
}
470464
}
471465

@@ -615,12 +609,6 @@ IF (has_animations) {
615609
;hasFunction = true
616610
}
617611

618-
// Root function keyframes.
619-
if (frame.function) {
620-
const execute_condition = frame.function_execute_condition ? frame.function_execute_condition + ' ' : ''
621-
frameFuncEnd += `\nexecute unless entity @s[tag=${TAGS.TRANSFORMS_ONLY()}] at @s ${execute_condition}run block ${frameIndex}_root_function%NEWLINE_PATCH%{\n${frame.function}\n}`
622-
;hasFunction = true
623-
}
624612
frameFunc += frameFuncEnd;
625613
if (frameFunc.length > 0) {
626614
frameFunc = `function ${frameIndex}%NEWLINE_PATCH%{${frameFunc}\n}`
@@ -657,6 +645,13 @@ IF (has_animations) {
657645
function <%blueprint_id%>/texture_slots/<%slot.slot%>/<%slot.texture%>
658646
}
659647
}
648+
IF (effects.function) {
649+
execute at @s <%effects.function_condition%>run {
650+
<%%
651+
emit.mcb(effects.function)
652+
%%>
653+
}
654+
}
660655
}
661656
}
662657
}

‎src/systems/datapackCompiler/1.21.4/main.mcb‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ IF (has_animations) {
512512
node => node.type === 'locator' && global.frame.node_transforms[node.uuid]
513513
)
514514
%%>
515-
IF (global.modified_effect_nodes.length > 0 || global.frame.function || getFrameEffects(animation).some(effects => effects.frame === animation.frames.length - 1)) {
515+
IF (global.modified_effect_nodes.length > 0 || getFrameEffects(animation).some(effects => effects.frame === animation.frames.length - 1)) {
516516
function last_frame_effects {
517517
REPEAT(global.modified_effect_nodes) as node {
518518
<%%
@@ -548,12 +548,6 @@ IF (has_animations) {
548548
function ../frame_effects/<%animation.frames.length - 1%>
549549
}
550550

551-
IF (global.frame.function) {
552-
execute <%global.frame.function_execute_condition ? global.frame.function_execute_condition + ' ' : ''%>run \
553-
block <%animation.duration%>_function_keyframe {
554-
<%global.frame.function%>
555-
}
556-
}
557551
}
558552
}
559553

@@ -703,12 +697,6 @@ IF (has_animations) {
703697
;hasFunction = true
704698
}
705699

706-
// Root function keyframes.
707-
if (frame.function) {
708-
const execute_condition = frame.function_execute_condition ? frame.function_execute_condition + ' ' : ''
709-
frameFuncEnd += `\nexecute unless entity @s[tag=${TAGS.TRANSFORMS_ONLY()}] at @s ${execute_condition}run block ${frameIndex}_root_function%NEWLINE_PATCH%{\n${frame.function}\n}`
710-
;hasFunction = true
711-
}
712700
frameFunc += frameFuncEnd;
713701
if (frameFunc.length > 0) {
714702
frameFunc = `function ${frameIndex}%NEWLINE_PATCH%{${frameFunc}\n}`
@@ -745,6 +733,13 @@ IF (has_animations) {
745733
function <%blueprint_id%>/texture_slots/<%slot.slot%>/<%slot.texture%>
746734
}
747735
}
736+
IF (effects.function) {
737+
execute at @s <%effects.function_condition%>run {
738+
<%%
739+
emit.mcb(effects.function)
740+
%%>
741+
}
742+
}
748743
}
749744
}
750745
}

‎src/systems/datapackCompiler/1.21.5/main.mcb‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ IF (has_animations) {
511511
node => node.type === 'locator' && global.frame.node_transforms[node.uuid]
512512
)
513513
%%>
514-
IF (global.modified_effect_nodes.length > 0 || global.frame.function || getFrameEffects(animation).some(effects => effects.frame === animation.frames.length - 1)) {
514+
IF (global.modified_effect_nodes.length > 0 || getFrameEffects(animation).some(effects => effects.frame === animation.frames.length - 1)) {
515515
function last_frame_effects {
516516
REPEAT(global.modified_effect_nodes) as node {
517517
<%%
@@ -547,12 +547,6 @@ IF (has_animations) {
547547
function ../frame_effects/<%animation.frames.length - 1%>
548548
}
549549

550-
IF (global.frame.function) {
551-
execute <%global.frame.function_execute_condition ? global.frame.function_execute_condition + ' ' : ''%>run \
552-
block <%animation.duration%>_function_keyframe {
553-
<%global.frame.function%>
554-
}
555-
}
556550
}
557551
}
558552

@@ -702,12 +696,6 @@ IF (has_animations) {
702696
;hasFunction = true
703697
}
704698

705-
// Root function keyframes.
706-
if (frame.function) {
707-
const execute_condition = frame.function_execute_condition ? frame.function_execute_condition + ' ' : ''
708-
frameFuncEnd += `\nexecute unless entity @s[tag=${TAGS.TRANSFORMS_ONLY()}] at @s ${execute_condition}run block ${frameIndex}_root_function%NEWLINE_PATCH%{\n${frame.function}\n}`
709-
;hasFunction = true
710-
}
711699
frameFunc += frameFuncEnd;
712700
if (frameFunc.length > 0) {
713701
frameFunc = `function ${frameIndex}%NEWLINE_PATCH%{${frameFunc}\n}`
@@ -744,6 +732,13 @@ IF (has_animations) {
744732
function <%blueprint_id%>/texture_slots/<%slot.slot%>/<%slot.texture%>
745733
}
746734
}
735+
IF (effects.function) {
736+
execute at @s <%effects.function_condition%>run {
737+
<%%
738+
emit.mcb(effects.function)
739+
%%>
740+
}
741+
}
747742
}
748743
}
749744
}

‎src/systems/datapackCompiler/index.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -277,13 +277,17 @@ interface FrameEffects {
277277
texture_slots: Array<{ slot: string; texture: string }>
278278
/** The texture slot keyframe's execute condition followed by a space, or nothing. */
279279
texture_slots_condition: string
280+
/** The root function keyframe's commands, run after the Variants and texture slots. */
281+
function?: string
282+
/** The function keyframe's execute condition followed by a space, or nothing. */
283+
function_condition: string
280284
}
281285

282286
const FRAME_EFFECTS_CACHE = new WeakMap<IRenderedAnimation, FrameEffects[]>()
283287

284288
/**
285-
* The Variants and texture slots each frame of `animation` changes, for frames that change any.
286-
* Slots and textures that didn't make it into the rig are skipped.
289+
* The Variants, texture slots and root function each frame of `animation` applies, for frames that
290+
* apply any. Slots and textures that didn't make it into the rig are skipped.
287291
*/
288292
function getFrameEffects(rig: IRenderedRig, animation: IRenderedAnimation): FrameEffects[] {
289293
const cached = FRAME_EFFECTS_CACHE.get(animation)
@@ -308,7 +312,7 @@ function getFrameEffects(rig: IRenderedRig, animation: IRenderedAnimation): Fram
308312
if (slot && texture) textureSlots.push({ slot: slot.name, texture: texture.name })
309313
}
310314

311-
if (!variants.length && !textureSlots.length) continue
315+
if (!variants.length && !textureSlots.length && !frame.function) continue
312316
result.push({
313317
frame: frameIndex,
314318
variants,
@@ -319,6 +323,10 @@ function getFrameEffects(rig: IRenderedRig, animation: IRenderedAnimation): Fram
319323
texture_slots_condition: frame.texture_slots_execute_condition
320324
? frame.texture_slots_execute_condition + ' '
321325
: '',
326+
function: frame.function,
327+
function_condition: frame.function_execute_condition
328+
? frame.function_execute_condition + ' '
329+
: '',
322330
})
323331
}
324332
FRAME_EFFECTS_CACHE.set(animation, result)

‎src/tests/effect-keyframe-export.test.ts‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ import { type ExportRun, relFiles, runFixtureExport } from './helpers/export'
1111

1212
/**
1313
* Adds a looping `effects` animation to the `Dummy` fixture: a `color1` + `color2` stack on frame
14-
* 0, a texture slot keyframe on frame 10, and another on the last frame.
14+
* 0, a texture slot keyframe and a function keyframe on frame 10, and another texture slot
15+
* keyframe on the last frame.
1516
*/
1617
function addEffectKeyframes(aj: any) {
1718
const g = globalThis as any
@@ -34,6 +35,10 @@ function addEffectKeyframes(aj: any) {
3435

3536
const last = fx.addKeyframe({ channel: 'texture_slot', time: 1, data_points: [{}] })
3637
last.texture_slots = { [slot.uuid]: textures[1].uuid }
38+
39+
const fn = fx.addKeyframe({ channel: 'function', time: 0.5, data_points: [{}] })
40+
fn.function = 'tag @s add gametest.function_keyframe'
41+
fn.execute_condition = 'if entity @s[tag=fn]'
3742
}
3843

3944
function readFiles(run: ExportRun, files: string[]) {
@@ -108,6 +113,22 @@ describe.each([
108113
expect(color2).toBeGreaterThan(color1)
109114
})
110115

116+
it('runs root function keyframes from frame effects, once, after the texture slots', () => {
117+
const effects = readFiles(run, effectFiles())
118+
expect(effects).toMatch(/execute at @s if entity @s\[tag=fn\] run function/)
119+
expect(effects).toContain('tag @s add gametest.function_keyframe')
120+
const frame10 = readFiles(
121+
run,
122+
dpFiles.filter(f => f.endsWith('/animations/effects/zzz/frame_effects/10.mcfunction'))
123+
)
124+
expect(frame10.indexOf('tag=swap')).toBeLessThan(frame10.indexOf('tag=fn'))
125+
const everywhere = readFiles(
126+
run,
127+
dpFiles.filter(f => f.includes('/animations/effects/'))
128+
)
129+
expect(everywhere.split('tag @s add gametest.function_keyframe').length - 1).toBe(1)
130+
})
131+
111132
it('switches texture slots behind their condition', () => {
112133
const effects = readFiles(run, effectFiles())
113134
expect(effects).toMatch(/execute if entity @s\[tag=swap\] run function/)

0 commit comments

Comments
 (0)