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

springbone の import で throw しない #2064

Merged
merged 4 commits into from
Jun 6, 2023

Conversation

ousttrue
Copy link
Contributor

@ousttrue ousttrue commented May 15, 2023

fixed #2023

index の範囲違反を含む SpringBone を import したときに throw しない。
SpringBone の import は中途半端な状態になるが、VRM 全体としてはロードできるようになる。

export 時の index -1 チェックをひとつ追加。

@ousttrue ousttrue added this to the v0.111 milestone May 15, 2023
@ousttrue ousttrue requested a review from Santarh May 15, 2023 04:19
Santarh
Santarh previously approved these changes Jun 5, 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.

問題は理解したが、保守性の観点でコードの治安が悪化傾向にあるなあという感想…
本来は Parser の段階で解決したいですね。

とはいえまあ VRM 0.x のコードなんで、いっか……

@@ -335,6 +335,17 @@ async Task<MeshWithMaterials> BuildMeshAsync(IAwaitCaller awaitCaller, Func<stri
#region Imported
protected GameObject Root;
public List<Transform> Nodes = new List<Transform>();
public bool TryGetNode(int index, out Transform node)
Copy link
Contributor

Choose a reason for hiding this comment

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

ここ protected にできませんか?

@ousttrue
Copy link
Contributor Author

ousttrue commented Jun 5, 2023

Unity 上で構成を変更した場合に、Spring とか HumanoidBone とかが参照しているノードが居なくなることがあって、
そのことが起因でエクスポート時に変なところに -1 が入っている可能性が増えている。
ような気がしますね。
node の index を指し示す部分を int? にする工事した方がよいかもしれない。

@ousttrue ousttrue requested a review from Santarh June 5, 2023 11:09
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.

LGTM

@Santarh
Copy link
Contributor

Santarh commented Jun 6, 2023

UniGLTF 層の glTF JSON 型定義を、VRM 1.0 と同様の水準にもっていきたいですね~ > int? まわり

@ousttrue ousttrue merged commit 48ebe7e into vrm-c:master Jun 6, 2023
1 check passed
@ousttrue ousttrue deleted the fix/broken_springbone_no_throw branch January 24, 2024 06:11
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.

[SpringBone] Importing Issue
2 participants