Skeletal Animation 01 - Asset Managing #491
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
McCallisterRomer
requested changes
Dec 26, 2023
| @@ -32,17 +33,20 @@ class NcAssetImpl : public NcAsset | |||
|
|
|||
| void OnBeforeSceneLoad() override; | |||
|
|
|||
| auto OnBoneUpdate() noexcept -> Signal<const BoneUpdateEventData&>& override; | |||
There was a problem hiding this comment.
My initial thoughts are that bones/meshes are not separate assets, therefore they (maybe?) shouldn't need different signals. I'm looking for the receiving code to better understand, but I'm only seeing the emit calls. Is there anything subscribing to this event in this PR, or is that coming down the pike?
There was a problem hiding this comment.
My personal preference is to leave it as two signals - we could combine it but I think it makes SkeletalAnimationSystem less clear, and we'd have to do "onSignalReceived() { if mesh.bonesData.has_value() ... } in the system as well, which I'd rather avoid
McCallisterRomer
approved these changes
Dec 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal
Add support for managing skeletal animation assets into NcEngine.
Background
SkeletalAnimationassets are generated from FBX files that contain animation clips. NcTools converts the FBX files into.ncafiles. This PR allows import of those.ncafiles.Changes
SkeletalAnimationAssetManagerto load/unloadSkeletalAnimationasset files.MeshAssetManagerto parse armature data from mesh.ncafiles.SkeletalAnimationAssetManager