[exporter] asset.generator を修正 #2171
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
|
現状のリポジトリとパッケージの構成名の関係から、どうするのが望ましいのかちょっと考えてからレビューをする
|
|
この PullReq で行いたいことは generator の出力値を
という認識で合ってますよね？ |
Santarh
requested changes
Oct 20, 2023
| @@ -6,7 +6,7 @@ namespace UniGLTF | |||
| [Serializable] | |||
| public class glTFAssets | |||
| { | |||
| public string generator; | |||
| public string generator = $"UniVRM-{UniVrmPackageVersion.VERSION}"; | |||
There was a problem hiding this comment.
このクラスは glTF 定義を表すものですし、Importer でも参照される定義なので
デフォルト値として UniVRM の知識を入れてしまうのは良くないと考えます。
たとえば generator の定義がない glTF ファイルを読み込んだときに、この値が入ってしまいます。
したがって Exporter 実装で代入する方が望ましいと考えます。
| { | ||
| public static partial class VRMVersion | ||
| public static partial class UniVrmPackageVersion |
There was a problem hiding this comment.
ここで言う "UniVRM" は com.vrmc.univrm ではなく UniVRM（リポジトリ名） であることは分かるのですが……
UniVRM という名前は含めたくない気持ちはけっこうあります。
これが存在しているだけで「UniGLTF は UniVRM を知っていていいんだな」という割れ窓になってしまう気持ちがあります。
PackageVersion でいいんじゃないかなあ…
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.
fixed #1896
バージョン番号の定数を vrm から unigltf に移動。
名前も vrm じゃなくて
UniVRMのパッケージ番号であることを明示した。