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

fix: set systemId for audio assets unless specified #133

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

xnv
Copy link
Member

@xnv xnv commented Oct 8, 2019

このpull requestが解決する内容

オーディオアセットの systemId は、省略された場合 "sound" 相当として扱われます (仕様) が、 hint オプションの値が "sound" と食い違っていました。この食い違いが遠因となり、一部環境で「systemId が省略されたアセット」のロードに失敗することがありました。

これを修正します。

ユニットテストを追加した他、手元で akashic serve に組み込んで当該コンテンツのロード時の引数が修正されることを確認しています。

破壊的な変更を含んでいるか?

  • なし

@@ -281,12 +281,16 @@ namespace g {
// durationというメンバは後から追加したため、古いgame.jsonではundefinedになる場合がある
if (conf.duration === undefined)
conf.duration = 0;
if (!conf.systemId || !audioSystemConfMap[conf.systemId])
conf.systemId = this.game.defaultAudioSystemId;
Copy link
Member Author

Choose a reason for hiding this comment

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

normalize の過程で、ない systemId も補完してしまうことにします。

const audioSystemConf = audioSystemConfMap[conf.systemId];
if (!audioSystemConf)
throw ExceptionFactory.createAssertionError("AssetManager#_normalize: unknown systemId for the audio asset: " + p);
Copy link
Member Author

Choose a reason for hiding this comment

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

なかったら直ちにエラー。

Comment on lines -334 to +338
var system = conf.systemId ? this.game.audio[conf.systemId] : this.game.audio[this.game.defaultAudioSystemId];
var system = this.game.audio[conf.systemId] || this.game.audio[this.game.defaultAudioSystemId];
Copy link
Member Author

Choose a reason for hiding this comment

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

  • いずれにせよ異常系ですが、最悪の場合 conf.systemId && !this.game.audio[conf.systemId] が成立しうるので、より安全になるようにしておきます。
  • 一見「 normalize の段階で !!this.game.audio[conf.systemId] が確実に成り立つ = この || は不要」に見えますが、この箇所は外部リソースアセット (game.json に記述がないものを読み込む) でも使うので必要です。

## 2.5.3

機能追加
* `g.Font` をインタフェースから抽象クラスに変更

Copy link
Contributor

Choose a reason for hiding this comment

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

意図しない不要な改行かと思います。

@ShinobuTakahashi
Copy link
Contributor

1点、不要な改行のコメントつけましたがapprovedです

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.

3 participants