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

EditorImport時、ユーザーが任意のMaterialDescriptorGeneratorを差し込むための拡張の窓を追加 #2116

Merged
merged 4 commits into from
Aug 8, 2023

Conversation

notargs
Copy link
Contributor

@notargs notargs commented Jul 26, 2023

プロジェクト固有のShaderを用いてEditorImportを行いたいユースケースに対応するために
任意のMaterialDescriptorGeneratorを差し込むための拡張の仕組みを追加しました

MaterialDescriptorGeneratorFactory abstract classを継承したScriptableObjectを作り、
ProjectSettingsからアタッチすることで、任意のMaterialDescriptorGeneratorを利用することができます
image

@notargs notargs requested a review from Santarh July 26, 2023 07:09
@ousttrue ousttrue added this to the v0.114 milestone Jul 26, 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.

よさそうです!
一点、使われて無さそうな string があるのだけちょっと対応していただけると!

}

return renderPipeline switch
Copy link
Contributor

Choose a reason for hiding this comment

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

ProjectSettings による override がなければデフォルトの実装を返す

namespace VRM10.Settings
{
[FilePath("ProjectSettings/Vrm10ProjectEditorSettings.asset", FilePathAttribute.Location.ProjectFolder)]
internal class Vrm10ProjectEditorSettings : ScriptableSingleton<Vrm10ProjectEditorSettings>
Copy link
Contributor

Choose a reason for hiding this comment

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

ScriptableSingleton<T> 、そういうのも生えてたんだなあ

@@ -16,17 +17,18 @@ public static class VrmScriptedImporterImpl
{
static IMaterialDescriptorGenerator GetMaterialDescriptorGenerator(RenderPipelineTypes renderPipeline)
{
switch (renderPipeline)
var settings = Vrm10ProjectEditorSettings.instance;
Copy link
Contributor

Choose a reason for hiding this comment

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

ScriptableSingleton<T>.instance は初回アクセス時にインスタンスを生成するのでおそらく null にはならない

https://docs.unity3d.com/ja/2021.2/ScriptReference/ScriptableSingleton_1-instance.html

[SettingsProvider]
public static SettingsProvider CreateProvider() => new Vrm10ProjectSettingsProvider();

private string AssetPath = "Assets/Vrm10ProjectEditorSettings.asset";
Copy link
Contributor

Choose a reason for hiding this comment

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

使われて無さそう?なんらか実装の変更で不要になったもの?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

修正しました 🙇

@notargs notargs requested a review from Santarh August 8, 2023 06:49
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 Santarh merged commit 1477102 into vrm-c:master Aug 8, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants