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

unityのwww対応 #239

Merged
merged 2 commits into from
May 31, 2019
Merged

unityのwww対応 #239

merged 2 commits into from
May 31, 2019

Conversation

hiroj
Copy link
Contributor

@hiroj hiroj commented Apr 24, 2019

  • WWWからUnityWebRequestTextureに変更

@hiroj hiroj added this to the v0.52.0 milestone Apr 24, 2019
@hiroj hiroj self-assigned this Apr 24, 2019
Texture.name = m_textureName;
}
}
#elif UNITY_5
Copy link
Contributor

Choose a reason for hiding this comment

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

ここは else とするか、 あるいは条件分岐の最後に

+ #else
+ #error Unsupported Unity version
  #endif

を追加することをお勧めします。今後何かのヒューマンエラーで条件に合致しない場合が発生してしまった場合、何も警告無く通信が行われないバグが発生する可能性があるからです。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

レビューありがとうございます、修正入れます

@hiroj hiroj merged commit fff1a29 into vrm-c:master May 31, 2019
@saturday06
Copy link
Contributor

saturday06 commented May 31, 2019

@hiroj あ、すみませんこれ細かい指摘なのですが、#if 分岐のエラー検知にLog.Errorしてもらいましたが、これを

Debug.LogError("Unsupported Unity version");

というコードを追加していただきましたが、そうではなく

#error Unsupported Unity version

としていただけないでしょうか?
後者の場合は、 #if 分岐が誤っている場合は必ずコンパイルエラーになるため、見逃すことがなくなります

@saturday06
Copy link
Contributor

@hiroj
Copy link
Contributor Author

hiroj commented May 31, 2019

なるほど、了解しました
別のプルリクで修正入れます

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants