-
Notifications
You must be signed in to change notification settings - Fork 410
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
[vrma] retarget phase を Vrm10Runtime.Process の先頭に移動 #2132
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
主旨は LGTM です。
今後変えにくそうな public な命名面でいくつか。
/// * Spring | ||
/// * LookAt | ||
/// * Expression | ||
/// | ||
/// </summary> | ||
public void Process() | ||
{ | ||
// 0. Update From VrmAnimation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
インデックス 1 始まりにずらしましょうw
@@ -1,6 +1,6 @@ | |||
using UnityEngine; | |||
|
|||
namespace UniVRM10.VRM10Viewer | |||
namespace UniVRM10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
namespace 移動 👍
@@ -1,6 +1,6 @@ | |||
using UnityEngine; | |||
|
|||
namespace UniVRM10.VRM10Viewer | |||
namespace UniVRM10 | |||
{ | |||
public static class VRM10Retarget |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
周りのクラスの命名が Vrm10
なので Vrm10Retarget
にしましょう
|
||
namespace UniVRM10 | ||
{ | ||
public interface IVrmAnimation : IDisposable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UniVRM としては VRM 1.0 の実装は Vrm10
Prefix をつけるということに経過的になっているし
IVrm10Animation
の方が良い気はします
@@ -5,11 +5,30 @@ | |||
|
|||
namespace UniVRM10 | |||
{ | |||
public class VrmAnimationInstance : MonoBehaviour | |||
public class VrmAnimationInstance : MonoBehaviour, IVrmAnimation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
同様に Vrm10AnimationInstance
かなあ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
SimpleVrma サンプルを追加