-
Notifications
You must be signed in to change notification settings - Fork 435
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
Refactoring GltfParser
#1076
Refactoring GltfParser
#1076
Conversation
/// Ambiguous file parser. | ||
/// Determine parsing method from the file extension. | ||
/// </summary> | ||
public sealed class AmbiguousGltfFileParser |
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.
従来の GltfParser
相当に、ファイルパス拡張子から判断してパース方法を変えるパーサ
/// <summary> | ||
/// .GLB file parser. | ||
/// </summary> | ||
public sealed class GlbFileParser |
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.
純粋に *.glb
ファイルだけをパースするパーサ
/// Low-level API. | ||
/// Parse from specified path & specified binary. | ||
/// </summary> | ||
public sealed class GlbLowLevelParser |
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.
VRM 1.0 Importer におけるマイグレーション時など、ファイルパスとそのバイナリが異なる場合の直接指定パーサ
} | ||
|
||
protected virtual void ParseExternalChunks(byte[] bytes, IReadOnlyList<GlbChunk> chunks) | ||
public static GltfData ParseGltf(string path, string json, IReadOnlyList<GlbChunk> chunks, IStorage storage, MigrationFlags migrationFlags) |
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.
パーサ実装のコア部分
/// <summary> | ||
/// .GLTF file with resources in same directory parser. | ||
/// </summary> | ||
public sealed class GltfFileWithResourceFilesParser |
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.
*.GLTF
ファイルのパーサ
/// For unit tests. | ||
/// JSON string with storage parser. | ||
/// </summary> | ||
public sealed class JsonWithStorageParser |
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.
ユニットテスト用のパーサ
/// <summary> | ||
/// Zip archived .GLTF file & resources parser. | ||
/// </summary> | ||
public sealed class ZipArchivedGltfFileParser |
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.
Zip された *.GLTF
ファイルディレクトリのパーサ
|
||
namespace UniGLTF | ||
{ | ||
public sealed class GltfData |
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.
GltfParser から別れた。
ImporterContext にはこいつを渡す。
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.
よさそう。
public GltfData Parse()
が変わっても良ければ、
static 関数で 全部の引数をこいつで受け付けてもよさそうな感触。
GltfParser
が、パース方法とパース結果の 2 つの責務を持っているのを分割。Assets/UniGLTF/Runtime/UniGLTF/IO/Parser/
以下に分割してクラス化。*.glb
や*.gltf
や*.zip
の違いが同じクラスに押し込められていたのも分割。GltfData
として定義。ImporterContext
が関心があるのはパース結果であるためGltfParser
ではなくGltfData
を引数に取る。インタフェースの変更が主なので
GltfData
が持つプロパティのアクセス修飾子などは変更していない。インタフェースの変更ではあるので、ユーザの API 呼び出し方法は変更される。
その点、この変更が良くなさそうであれば reject してください