From ba2fd6a0a0c305b116518656c80e79f6b1ecfdf2 Mon Sep 17 00:00:00 2001 From: Untold Engine Date: Wed, 23 Sep 2026 08:35:52 -0700 Subject: [PATCH 1/2] [Bugfix] Show one display name per aliased animation clip registerRuntimeAnimationClips registers a single-clip asset under both its embedded name and the caller's preferred name when they differ, pointing both keys at the same AnimationClip so changeAnimation keeps working with either. getAllAnimationClips and removeAnimationClip treated every key as an independent clip, so the editor listed one animation twice and removing one name could leave the other dangling. AnimationComponent now tracks which keys are internal aliases so getAllAnimationClips reports the preferred name once, and removeAnimationClip removes every key sharing the same clip identity. Re-registering a preferred name against a clip with a different embedded name (asset re-export) prunes the old clip's stale alias too. --- Sources/UntoldEngine/ECS/Components.swift | 26 ++- .../Systems/RegistrationSystem.swift | 11 +- .../AnimationClipAliasTests.swift | 199 ++++++++++++++++++ 3 files changed, 233 insertions(+), 3 deletions(-) create mode 100644 Tests/UntoldEngineTests/AnimationClipAliasTests.swift diff --git a/Sources/UntoldEngine/ECS/Components.swift b/Sources/UntoldEngine/ECS/Components.swift index 8a821971..2be2333f 100644 --- a/Sources/UntoldEngine/ECS/Components.swift +++ b/Sources/UntoldEngine/ECS/Components.swift @@ -362,9 +362,18 @@ public class AnimationComponent: Component { didSet { let live = Set(animationClips.values.map(ObjectIdentifier.init)) compiledClips = compiledClips.filter { live.contains($0.key) } + hiddenClipAliases = hiddenClipAliases.filter { animationClips[$0] != nil } } } + /// Keys in `animationClips` that are internal aliases rather than + /// logical/display clip names. registerRuntimeAnimationClips adds an + /// alias when an asset's embedded clip name differs from the caller's + /// preferred name, so both names keep working as lookup keys (e.g. for + /// changeAnimation) while getAllAnimationClips() reports only the + /// preferred name — otherwise one logical animation shows up twice. + var hiddenClipAliases: Set = [] + var currentAnimation: AnimationClip? public var animationsFilenames: [URL] = [] var pause: Bool = false @@ -400,6 +409,7 @@ public class AnimationComponent: Component { func cleanUp() { animationClips.removeAll() + hiddenClipAliases.removeAll() currentAnimation?.cleanUp() currentAnimation = nil compiledClips.removeAll() @@ -415,12 +425,24 @@ public class AnimationComponent: Component { motionMatching = MotionMatchingState() } + /// One display name per logical animation: hidden aliases (see + /// `hiddenClipAliases`) are excluded so an asset whose embedded clip + /// name differs from its preferred name is listed once, not twice. func getAllAnimationClips() -> [String] { - Array(animationClips.keys) + Array(animationClips.keys.filter { hiddenClipAliases.contains($0) == false }) } + /// Removes the logical clip named `animationClip` along with every + /// other key (aliases) referencing the same `AnimationClip` instance, + /// so removing either the preferred name or the embedded-name alias + /// removes the whole animation rather than leaving the other name + /// dangling. func removeAnimationClip(animationClip: String) { - animationClips.removeValue(forKey: animationClip) + guard let clip = animationClips[animationClip] else { return } + let keysToRemove = animationClips.compactMap { key, value in value === clip ? key : nil } + for key in keysToRemove { + animationClips.removeValue(forKey: key) + } } /// Returns the compiled form of `clip` resolved against `skeleton`, diff --git a/Sources/UntoldEngine/Systems/RegistrationSystem.swift b/Sources/UntoldEngine/Systems/RegistrationSystem.swift index 5c3fe9ac..a9dfa424 100644 --- a/Sources/UntoldEngine/Systems/RegistrationSystem.swift +++ b/Sources/UntoldEngine/Systems/RegistrationSystem.swift @@ -815,7 +815,7 @@ private func ensureAnimationComponent(entityId: EntityID, errorEntityId: EntityI return animationComponent } -private func registerRuntimeAnimationClips( +func registerRuntimeAnimationClips( _ runtimeClips: [RuntimeAnimationClip], preferredName: String, to animationComponent: AnimationComponent @@ -825,6 +825,7 @@ private func registerRuntimeAnimationClips( for runtimeClip in runtimeClips { let animationClip = AnimationClip(runtimeClip: runtimeClip) animationComponent.animationClips[runtimeClip.name] = animationClip + animationComponent.hiddenClipAliases.remove(runtimeClip.name) registeredNames.append(runtimeClip.name) } @@ -837,7 +838,15 @@ private func registerRuntimeAnimationClips( // Reuse the same instance registered above under runtimeClip.name: // compiledClips is now keyed by clip identity, so a second // AnimationClip built from the same runtimeClip would compile twice. + // If preferredName previously named a different clip (e.g. the + // asset was re-exported with a different embedded action name), + // drop that old clip's own alias keys so they don't linger as + // unreachable, un-displayed entries in animationClips. + if let previousClip = animationComponent.animationClips[preferredName], previousClip !== aliasedClip { + animationComponent.removeAnimationClip(animationClip: preferredName) + } animationComponent.animationClips[preferredName] = aliasedClip + animationComponent.hiddenClipAliases.insert(runtimeClip.name) registeredNames.append(preferredName) } diff --git a/Tests/UntoldEngineTests/AnimationClipAliasTests.swift b/Tests/UntoldEngineTests/AnimationClipAliasTests.swift new file mode 100644 index 00000000..2977c9df --- /dev/null +++ b/Tests/UntoldEngineTests/AnimationClipAliasTests.swift @@ -0,0 +1,199 @@ +// +// AnimationClipAliasTests.swift +// +// +// Copyright (C) Untold Engine Studios +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +import simd +@testable import UntoldEngine +import XCTest + +/// Coverage for the preferred-name/embedded-name aliasing that +/// `registerRuntimeAnimationClips` performs when an asset's single embedded +/// clip name (e.g. a default Blender action name like "Anim") differs from +/// the caller-supplied preferred name (e.g. a filename-derived name like +/// "hol_idle_anim"). Both names must keep working as lookup keys, but +/// `getAllAnimationClips` must report one display name per logical +/// animation, and `removeAnimationClip` must remove every alias together. +@MainActor +final class AnimationClipAliasTests: XCTestCase { + // MARK: - Fixtures + + private func makeRuntimeClip(name: String, rootTranslationY: Float = 0) -> RuntimeAnimationClip { + let channel = RuntimeAnimationChannel( + jointPath: "root", + translations: [.init(time: 0.0, value: simd_float3(0, rootTranslationY, 0))] + ) + return RuntimeAnimationClip(name: name, duration: 1.0, channels: [channel]) + } + + // MARK: - registerRuntimeAnimationClips aliasing + + func testAliasIsCreatedWhenPreferredNameDiffersFromEmbeddedName() { + let component = AnimationComponent() + let runtimeClip = makeRuntimeClip(name: "Anim") + + let registeredNames = registerRuntimeAnimationClips([runtimeClip], preferredName: "hol_idle_anim", to: component) + + XCTAssertEqual(Set(registeredNames), ["Anim", "hol_idle_anim"]) + XCTAssertTrue(component.animationClips["Anim"] === component.animationClips["hol_idle_anim"], + "Both keys should reference the same AnimationClip instance") + XCTAssertEqual(component.getAllAnimationClips(), ["hol_idle_anim"], + "Only the preferred name should be a display name") + } + + func testNoAliasWhenPreferredNameMatchesEmbeddedName() { + let component = AnimationComponent() + let runtimeClip = makeRuntimeClip(name: "walk") + + let registeredNames = registerRuntimeAnimationClips([runtimeClip], preferredName: "walk", to: component) + + XCTAssertEqual(registeredNames, ["walk"]) + XCTAssertEqual(component.animationClips.count, 1) + XCTAssertEqual(component.getAllAnimationClips(), ["walk"]) + } + + func testMultiClipAssetListsEveryClipWithoutAliasing() { + let component = AnimationComponent() + let clips = [makeRuntimeClip(name: "Anim"), makeRuntimeClip(name: "Anim2")] + + let registeredNames = registerRuntimeAnimationClips(clips, preferredName: "ignoredForMultiClip", to: component) + + XCTAssertEqual(Set(registeredNames), ["Anim", "Anim2"]) + XCTAssertEqual(component.getAllAnimationClips().sorted(), ["Anim", "Anim2"]) + } + + func testIndependentlyExportedClipsSharingEmbeddedNameStayDistinctByPreferredName() { + let component = AnimationComponent() + let clipA = makeRuntimeClip(name: "Anim", rootTranslationY: 1) + let clipB = makeRuntimeClip(name: "Anim", rootTranslationY: 9) + + _ = registerRuntimeAnimationClips([clipA], preferredName: "walk_anim", to: component) + _ = registerRuntimeAnimationClips([clipB], preferredName: "run_anim", to: component) + + XCTAssertEqual(component.getAllAnimationClips().sorted(), ["run_anim", "walk_anim"], + "Each independently exported clip must be listed once under its own preferred name") + XCTAssertFalse(component.animationClips["walk_anim"] === component.animationClips["run_anim"], + "Distinct clips must not collapse into the same instance") + XCTAssertEqual(component.animationClips["walk_anim"]?.getPose(at: 0, jointPath: "root")?.columns.3.y, 1) + XCTAssertEqual(component.animationClips["run_anim"]?.getPose(at: 0, jointPath: "root")?.columns.3.y, 9) + } + + func testReregisteringUnderSamePreferredNameWithChangedEmbeddedNamePrunesStaleAlias() { + let component = AnimationComponent() + let original = makeRuntimeClip(name: "Anim", rootTranslationY: 1) + _ = registerRuntimeAnimationClips([original], preferredName: "hol_idle_anim", to: component) + XCTAssertNotNil(component.animationClips["Anim"]) + + // Re-export changed the embedded clip name but the caller still asks + // for the same preferred/display name. + let replacement = makeRuntimeClip(name: "Idle", rootTranslationY: 9) + _ = registerRuntimeAnimationClips([replacement], preferredName: "hol_idle_anim", to: component) + + XCTAssertNil(component.animationClips["Anim"], "Stale alias from the replaced clip must not linger") + XCTAssertTrue(component.animationClips["Idle"] === component.animationClips["hol_idle_anim"]) + XCTAssertEqual(component.getAllAnimationClips(), ["hol_idle_anim"]) + XCTAssertEqual(component.animationClips["hol_idle_anim"]?.getPose(at: 0, jointPath: "root")?.columns.3.y, 9) + } + + // MARK: - removeAnimationClip cascades across aliases + + func testRemoveAnimationClipByPreferredNameRemovesEmbeddedAliasToo() { + let component = AnimationComponent() + let runtimeClip = makeRuntimeClip(name: "Anim") + _ = registerRuntimeAnimationClips([runtimeClip], preferredName: "hol_idle_anim", to: component) + + component.removeAnimationClip(animationClip: "hol_idle_anim") + + XCTAssertNil(component.animationClips["hol_idle_anim"]) + XCTAssertNil(component.animationClips["Anim"]) + XCTAssertTrue(component.getAllAnimationClips().isEmpty) + } + + func testRemoveAnimationClipByEmbeddedAliasRemovesPreferredNameToo() { + let component = AnimationComponent() + let runtimeClip = makeRuntimeClip(name: "Anim") + _ = registerRuntimeAnimationClips([runtimeClip], preferredName: "hol_idle_anim", to: component) + + component.removeAnimationClip(animationClip: "Anim") + + XCTAssertNil(component.animationClips["Anim"]) + XCTAssertNil(component.animationClips["hol_idle_anim"]) + XCTAssertTrue(component.getAllAnimationClips().isEmpty) + } + + func testRemoveAnimationClipDoesNotAffectUnrelatedClips() { + let component = AnimationComponent() + _ = registerRuntimeAnimationClips([makeRuntimeClip(name: "Anim")], preferredName: "hol_idle_anim", to: component) + _ = registerRuntimeAnimationClips([makeRuntimeClip(name: "walk")], preferredName: "walk", to: component) + + component.removeAnimationClip(animationClip: "hol_idle_anim") + + XCTAssertEqual(component.getAllAnimationClips(), ["walk"]) + } + + // MARK: - Engine-level lookup/playback through aliases (changeAnimation, entity-level API) + + private func makeAliasedEntity() -> EntityID { + let entityId = createEntity() + registerComponent(entityId: entityId, componentType: SkeletonComponent.self) + registerComponent(entityId: entityId, componentType: AnimationComponent.self) + registerComponent(entityId: entityId, componentType: RenderComponent.self) + registerComponent(entityId: entityId, componentType: ScenegraphComponent.self) + registerComponent(entityId: entityId, componentType: LocalTransformComponent.self) + registerComponent(entityId: entityId, componentType: WorldTransformComponent.self) + + let runtimeSkeleton = RuntimeSkeleton( + jointPaths: ["root"], + parentIndices: [nil], + bindTransforms: [.identity], + restTransforms: [.identity] + ) + scene.get(component: SkeletonComponent.self, for: entityId)?.skeleton = + Skeleton(runtimeSkeleton: runtimeSkeleton) + + let animationComponent = scene.get(component: AnimationComponent.self, for: entityId)! + _ = registerRuntimeAnimationClips([makeRuntimeClip(name: "Anim")], preferredName: "hol_idle_anim", to: animationComponent) + return entityId + } + + func testChangeAnimationPlaysClipThroughEitherAliasName() throws { + resetEngineTestState() + let entityId = makeAliasedEntity() + defer { destroyEntity(entityId: entityId) } + + changeAnimation(entityId: entityId, name: "Anim", transitionHalflife: 0) + let animationComponent = try XCTUnwrap(scene.get(component: AnimationComponent.self, for: entityId)) + XCTAssertTrue(animationComponent.currentAnimation === animationComponent.animationClips["hol_idle_anim"], + "Looking up by the embedded alias should resolve the same logical clip") + + changeAnimation(entityId: entityId, name: "hol_idle_anim", transitionHalflife: 0) + XCTAssertTrue(animationComponent.currentAnimation === animationComponent.animationClips["Anim"], + "Looking up by the preferred name should resolve the same logical clip") + } + + func testEntityLevelGetAllAnimationClipsReturnsOneDisplayName() { + resetEngineTestState() + let entityId = makeAliasedEntity() + defer { destroyEntity(entityId: entityId) } + + XCTAssertEqual(getAllAnimationClips(entityId: entityId), ["hol_idle_anim"]) + } + + func testEntityLevelRemoveAnimationClipClearsBothAliases() { + resetEngineTestState() + let entityId = makeAliasedEntity() + defer { destroyEntity(entityId: entityId) } + + removeAnimationClip(entityId: entityId, animationClip: "hol_idle_anim") + + let animationComponent = scene.get(component: AnimationComponent.self, for: entityId) + XCTAssertTrue(getAllAnimationClips(entityId: entityId).isEmpty) + XCTAssertNil(animationComponent?.animationClips["Anim"]) + XCTAssertNil(animationComponent?.animationClips["hol_idle_anim"]) + } +} From 48a25a3a6f684fb056cb9152d6c0ed8574164f0c Mon Sep 17 00:00:00 2001 From: Untold Engine Date: Wed, 23 Sep 2026 08:36:44 -0700 Subject: [PATCH 2/2] [Chores] fixed formatting --- Tests/UntoldEngineTests/AnimationClipAliasTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/UntoldEngineTests/AnimationClipAliasTests.swift b/Tests/UntoldEngineTests/AnimationClipAliasTests.swift index 2977c9df..7cd3d59d 100644 --- a/Tests/UntoldEngineTests/AnimationClipAliasTests.swift +++ b/Tests/UntoldEngineTests/AnimationClipAliasTests.swift @@ -78,7 +78,7 @@ final class AnimationClipAliasTests: XCTestCase { XCTAssertEqual(component.getAllAnimationClips().sorted(), ["run_anim", "walk_anim"], "Each independently exported clip must be listed once under its own preferred name") XCTAssertFalse(component.animationClips["walk_anim"] === component.animationClips["run_anim"], - "Distinct clips must not collapse into the same instance") + "Distinct clips must not collapse into the same instance") XCTAssertEqual(component.animationClips["walk_anim"]?.getPose(at: 0, jointPath: "root")?.columns.3.y, 1) XCTAssertEqual(component.animationClips["run_anim"]?.getPose(at: 0, jointPath: "root")?.columns.3.y, 9) }