Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VrmAnimation の Expression は float field に変更 #2124

Merged
merged 3 commits into from
Aug 15, 2023

Conversation

ousttrue
Copy link
Contributor

@ousttrue ousttrue commented Aug 8, 2023

AnimationClip に普通の float curve として格納する様に変更した。

VrmAnimationInstance の public field と連動する。

AnimationClip に普通の float curve として格納する様に変更した。

VrmAnimationInstance の public field と連動する。
@ousttrue ousttrue requested a review from Santarh August 8, 2023 09:55
@ousttrue ousttrue added this to the v0.114 milestone Aug 8, 2023
Santarh
Santarh previously approved these changes Aug 8, 2023
Copy link
Contributor

@Santarh Santarh left a comment

Choose a reason for hiding this comment

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

よさそう。 nameof() だけは修正してもいいかなあ、くらい

Comment on lines +27 to +30
// public float preset_lookup;
// public float preset_lookdown;
// public float preset_lookleft;
// public float preset_lookright;
Copy link
Contributor

Choose a reason for hiding this comment

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

LookAt 情報は float ではなく別にアニメーションデータが存在し、また
そもそも LookAt によって上書き制御されるので、ここには要らない

@@ -9,14 +9,14 @@ namespace UniGLTF
{
public static class AnimationImporterUtil
{
private enum TangentMode
public enum TangentMode
Copy link
Contributor

Choose a reason for hiding this comment

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

public にするなら class 内定義である必要はなさそうだが…まあ必要があればいずれリファクタ

var gltfAnimation = Data.GLTF.animations[0];
if (animation.Expressions.Preset is Preset preset)
{
{ if (GetExpression(gltfAnimation, ExpressionKey.CreateFromPreset(ExpressionPreset.happy), "preset_happy", preset.Happy) is ExpressionInfo info) yield return info; }
Copy link
Contributor

Choose a reason for hiding this comment

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

string リテラルの部分、たとえば nameof(VrmAnimationInstance.preset_happy) にするとコード追跡性が上がって良さそう(1行長くなるが…)

{ if (GetExpression(gltfAnimation, ExpressionKey.CreateFromPreset(ExpressionPreset.oh), "preset_oh", preset.Oh) is ExpressionInfo info) yield return info; }
{ if (GetExpression(gltfAnimation, ExpressionKey.CreateFromPreset(ExpressionPreset.blink), "preset_blink", preset.Blink) is ExpressionInfo info) yield return info; }
{ if (GetExpression(gltfAnimation, ExpressionKey.CreateFromPreset(ExpressionPreset.blinkLeft), "preset_blinkLeft", preset.BlinkLeft) is ExpressionInfo info) yield return info; }
{ if (GetExpression(gltfAnimation, ExpressionKey.CreateFromPreset(ExpressionPreset.blinkRight), "preset_blinkRight", preset.BlinkRight) is ExpressionInfo info) yield return info; }
Copy link
Contributor

Choose a reason for hiding this comment

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

あ、別に property 名を意図していたリテラル指定というわけではなくて
ちょっと違ったんですね……

Copy link
Contributor Author

Choose a reason for hiding this comment

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

部分的に不一致になっている・・・

@ousttrue ousttrue merged commit 6a32acb into vrm-c:master Aug 15, 2023
1 check passed
@ousttrue ousttrue deleted the feature/vrma_animation_clip branch January 24, 2024 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants