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

空押し判定を実装 #1503

Merged
merged 6 commits into from Jun 24, 2023
Merged

空押し判定を実装 #1503

merged 6 commits into from Jun 24, 2023

Conversation

goe0
Copy link
Contributor

@goe0 goe0 commented Jun 10, 2023

🔨 変更内容 / Details of Changes

空押し判定を導入しました。譜面ヘッダーとsettingsで使用可否を設定でき、デフォルトはOFFです。
Flash時代のプギャー判定とは以下の点で仕様が異なります。

  • 全区間で判定するのではなく各ノーツの-16Fまでを判定
  • ダメージは通常ダメージの1/4
  • 判定はウワァンとしてカウント(リザルトでは「見逃し数(+空押し数)」で表示)

🔖 関連Issue, 変更理由 / Related Issues, Reason for Changes

固定運指可能key(5,7,7i,9A,9Bなど)でゲージ甘めの高密度譜面だと全押し連打でクリアできることがあり、その対策のため

📷 スクリーンショット / Screenshot

empty_jdg

📝 その他コメント / Other Comments

あまり見ないケースですがg_judgObj.arrowJ の4番目と5番目の値を別にしている作品がある場合、判定処理に影響を与えます

@cwtickle
Copy link
Owner

@goe0
PRありがとうございます! 空判定の選択肢を入れることについては問題ないと思います。
個人的にいくつか気になる点がありましたのでコメントします。

1. リザルト画面での判定表記

  • 空押しは表示自体は確かに判定ですが、総矢印数の内数に入らないので
    どちらかといえばFast/Slow的な位置づけに近いのかなと思いました。
    例えば、空判定数を推定Adj欄の下に入れるのはどうでしょうか?

2. プレイ画面上の空判定表記

  • 今の実装だとウワァンにまとめていると思いますが、
    1.に合わせて従来のようにプギャーとして独立させた方がわかりやすいように思いました。
    空判定数を下記のように載せるかどうか(入れるとするなら、フリーズコンボ欄から一段空白を入れてその下か)
    は個人的には入れても入れなくてもといった感じです。

@cwtickle
Copy link
Owner

cwtickle commented Jun 10, 2023

DiscordのソースチャンネルにこのPRのリンクを張りました。
https://discord.com/channels/698460971231870977/944491021918683196/1116994364758036530

Copy link
Owner

@cwtickle cwtickle left a comment

Choose a reason for hiding this comment

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

Discordでさまざまコメントが出ていますが、こちらではコード上の気づいた点を上げます。

emptyJdgUse有効時、ゼロで終了したときの表示がおかしい

  • 下記のように、矢印を1つも取らずに終了した場合にウワァンがNaN(+undefined)になります。ウワァンがゼロで終了した場合でも同様です。
    条件がわかりませんでしたが、NaN(+NaN)のように出ることがあります。
  • おそらく初期化周りの問題だと思います。g_resultObjの要素として追加するのであれば、
    danoni_constants.jsで初期化した方が初期化漏れを防げます。

@goe0
Copy link
Contributor Author

goe0 commented Jun 11, 2023

ありがとうございます。修正しました。
議論により判定名が変わるかもしれませんが、一旦emptyUwanのままとしています。

Copy link
Owner

@cwtickle cwtickle left a comment

Choose a reason for hiding this comment

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

ありがとうございます。
追加で変更があると思いますが、今のところ問題点は直っていますのでApproveします。

@goe0
Copy link
Contributor Author

goe0 commented Jun 23, 2023

Discord上の議論を反映し、空押し判定のウワァンから独立、オプション化などを行いました。
displayDiffに処理をまとめたことで、変更前よりシンプルな実装になったかと思います。

Copy link
Owner

@cwtickle cwtickle left a comment

Choose a reason for hiding this comment

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

@goe0
ありがとうございます。コード確認しました。
概ね問題無いと思いますが、次の2点だけ。

judgeTargetArrow の条件式

  • judgeTargetArrowの空押しの条件式が_difCnt <= g_judgObj.arrowJ[g_judgPosObj.uwan] && _difCnt > g_judgObj.arrowJ[g_judgPosObj.shobon]になっています。
    枠外判定が別にあるので結果的に問題はないですが、早押しした場合のみExcessive判定を出すということであれば
    明示的に_difFrameで判定させるように書いた方が良いと思いました。
    ※displayDiffの方は_difFrameを使って記述されています。

Excessiveの説明文(英語)について

  • 空押し=Excessive Missのようなので、"Excessive miss judgment"かと思います。
    ※CW Editionでは(judgementではなく)judgmentで統一しています。

@goe0
Copy link
Contributor Author

goe0 commented Jun 24, 2023

ご確認ありがとうございます。修正しました。

@codeclimate
Copy link

codeclimate bot commented Jun 24, 2023

Code Climate has analyzed commit 7c73687 and detected 0 issues on this pull request.

View more on Code Climate.

@goe0
Copy link
Contributor Author

goe0 commented Jun 24, 2023

追加ですみませんが、空押し判定OFFのときのTwitterリザルトの出力内容を修正しました。

Copy link
Owner

@cwtickle cwtickle left a comment

Choose a reason for hiding this comment

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

空押し判定OFFのときのリザルト表記については分岐必要でしたね。確認しました。
本来実施する範囲の内容としては問題無いと思いますので、この内容でマージします。
お疲れさまでした m(_ _)m

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

Successfully merging this pull request may close these issues.

None yet

2 participants