player_type から時限ステータスを分離した (朦朧のみ) #1548

Hourier · 2021-09-18T08:34:45Z

#553 の前段対応で、#1498 の設計変更を実施しました
ご確認下さい

TimedEffectsは、今後paralyzed やconfused 等、各種時限効果を移していく予定です。それによりplayer_type を軽くすることを意図します
朦朧の蓄積に関する仕様は現行のままです。おかしい点があればご連絡下さい
本当はフィールド変数はunique_ptr にした方がいいのかもしれませんが、player_type でそこまでやると影響が大きすぎるのと、各種演算子オーバーライドが面倒だったのでスルーしました
ある意味一番面倒なset_stun() について、このブランチで弄るのは差分が増えすぎるので丁重に放置中です

habu1010 · 2021-09-18T10:13:38Z

src/system/player-type-definition.h

@@ -10,6 +10,7 @@
 #include "player/player-sex.h"
 #include "system/angband.h"
 #include "system/system-variables.h"
+#include "timed-effect/timed-effects.h"


ここまでヘッダ同士の依存を減らす方向で来ているので、ここでのインクルードは無くして実装ファイルのコンストラクタで timed_efffect を初期化するようにしたほうがいいのではないでしょうか。
TimeEffects を使用する実装ファイルすべてでインクルードが必要になりますが依存を減らしていく方針ではしかたないかと思います。

作業途中の名残でした (下記とも絡む)
削除し、実装ファイル側でインクルードしていきます

habu1010 · 2021-09-18T10:15:25Z

src/system/player-type-definition.h

+    std::shared_ptr<TimedEffects> effects() const;
+
+private:
+    std::shared_ptr<TimedEffects> timed_effects = std::shared_ptr<TimedEffects>(new TimedEffects());


std::shared_ptr の初期化は特に理由が無い限り、以下の点で std::make_shared を使用したほうがいいです。

同じ型を何度も書く必要がない

クラス本体とコントロールブロックのメモリ領域の確保がまとめて行われるので効率的

実装途中はmake_shared() だと実行時エラーでソフトが落ちていました (この形式だと何故かOKだった)が、設計が完成した現在は大丈夫なようです
問題ないようなのでmake_shared() の方に切り替えます

habu1010 · 2021-09-18T10:17:08Z

src/system/player-type-definition.h

@@ -19,8 +20,9 @@ enum class RF_ABILITY;

 struct floor_type;
 struct object_type;
-;
-typedef struct player_type {
+class TimedEffects;


上述したようにヘッダの依存を減らすのであれば前方宣言が必要ですが、"timed-effects.h"をplayer-type-definition.hからインクルードする方針なのであれば必要ありません。

前方宣言を残す方針にします

habu1010 · 2021-09-18T10:18:45Z

src/timed-effect/player-stun.h

+#include <string>
+#include <tuple>
+
+enum class StunRank {


モンスターにも朦朧状態はあるので、PlayerStunRankにしたほうが良いです。

Playerを付けます

habu1010 · 2021-09-18T10:20:16Z

src/timed-effect/player-stun.h

+    short current() const;
+    StunRank get_rank() const;
+    StunRank get_rank(short value) const;
+    std::string_view get_stun_mes(StunRank stun_rank) const;


引数付きの get_rank() と、get_stun_mes はインスタンス変数に依存しないので、static メンバ関数にしたほうがいいです。

staticにします

habu1010 · 2021-09-18T10:24:52Z

src/timed-effect/player-stun.h

+    StunRank get_rank(short value) const;
+    std::string_view get_stun_mes(StunRank stun_rank) const;
+    int decrease_chance() const;
+    short decrease_damage() const;


decrese は他動詞として使うのが一般的なので、意味が通じにくいかもしれません。（名詞として使うにしても、chance_decreseのように単語の順序を逆にするのが適切のように感じます）
get_chance_penalty のような名前にしたらどうでしょうか。

habu1010 · 2021-09-18T10:27:46Z

src/timed-effect/timed-effects.h

@@ -0,0 +1,16 @@
+#pragma once
+
+#include "timed-effect/player-stun.h"


player-type-definition.h でのコメントと同様、ヘッダファイルの依存を減らすならここでインクルードするのはやめたほうがよいと思います。

ここも設計途中にコンパイルエラーが出ていたのみの部分です
削除します

Hourier · 2021-09-18T12:10:06Z

頂いた指摘を修正し、更に言語が正しく選択されない場合がある事象を解決させました
ご確認下さい

habu1010

1点だけコメントしました。

habu1010 · 2021-09-18T13:04:49Z

src/system/player-type-definition.cpp

+
+player_type::player_type()
+{
+    this->timed_effects = std::make_shared<TimedEffects>();


細かいところですが、メンバ初期化子で初期化できるときはそうしたほうがよいので、

player_type::player_type() : timed_effects(std::make_shared<TimedEffects>()) { }

としたほうが良いでしょう。（TimedEffects クラスも同様）

スマートポインタを初期化子リストで初期化する方法が分からなかったので放置してました
ぜひ採用します！

…ffect variables from player_type

…rent() and so on

…rease_chance()

…to decrease_damage()

… that the expression of stun is English always in some environments

Hourier · 2021-09-18T13:11:01Z

コメントを反映させてforce pushしました
ご確認下さい

habu1010

対応おつかれさまです。修正を確認しました。
朦朧の動作自体は大丈夫そうなので、Approveします。

Hourier · 2021-09-18T13:25:09Z

レビュー・動作確認ありがとうございました！

Hourier added the refactor 処理の整理、可読性の向上 label Sep 18, 2021

Hourier added this to the 3.0.0Alpha38Release milestone Sep 18, 2021

Hourier self-assigned this Sep 18, 2021

Hourier added this to In Progress in C++移行 via automation Sep 18, 2021

Hourier linked an issue Sep 18, 2021 that may be closed by this pull request

朦朧効果のリニューアル準備 (TimedEffects/PlayerStunクラスの定義) #1498

Closed

Hourier force-pushed the feature/Create-TimedEffects-Class branch from 4e473b2 to 9ddef9e Compare September 18, 2021 08:42

habu1010 reviewed Sep 18, 2021

View reviewed changes

Hourier force-pushed the feature/Create-TimedEffects-Class branch from 9ddef9e to de79b7d Compare September 18, 2021 12:07

habu1010 reviewed Sep 18, 2021

View reviewed changes

Hourier and others added 15 commits September 18, 2021 22:10

[Refactor] hengband#1498 Created timed-effect/ for separating timed-e…

6e00a9b

…ffect variables from player_type

[Refactor] hengband#1498 Defined StunRank and get_rank()

123fa9b

[Refactor] hengband#1498 Defined decrease_chance()

711ed76

[Refactor] hengband#1498 Defined set() and reset()

cb409ab

[Refactor] hengband#1498 Defined get_stun_mes()

1ed2664

[Refactor] hengband#1498 Defined is_stunned()

44081f3

[Refactor] hengband#1498 Replaced player_ptr->stun to PlayerStun::cur…

7278402

…rent() and so on

[Refactor] hengband#1498 Replaced direct fail-rate calculation to dec…

cd287e0

…rease_chance()

[Refactor] hengband#1498 Defined decrease_damage() in PlayerStun

9d06e97

[Refactor] hengband#1498 Replaced direct damage-decrease calculation …

8709a37

…to decrease_damage()

[Refactor] hengband#1498 Replaced direct substitution to set()/reset()

62dcbf2

[Refactor] hengband#1498 Defined get_expr() to divide View and Model

310826f

[Fix] Resolved compilation warning in item-loader.cpp

ad97a5c

[Refactor] hengband#1498 Habu氏から頂いたコメントを反映させた

0cfe580

[Fix] hengband#1498 環境によっては朦朧が英語表記になってしまう事象を修正した / Resolved the issue…

130d537

… that the expression of stun is English always in some environments

Hourier force-pushed the feature/Create-TimedEffects-Class branch from de79b7d to 130d537 Compare September 18, 2021 13:10

habu1010 approved these changes Sep 18, 2021

View reviewed changes

Hourier merged commit ccee6c1 into hengband:develop Sep 18, 2021

C++移行 automation moved this from In Progress to Done Sep 18, 2021

Hourier deleted the feature/Create-TimedEffects-Class branch September 18, 2021 13:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

player_type から時限ステータスを分離した (朦朧のみ) #1548

player_type から時限ステータスを分離した (朦朧のみ) #1548

Hourier commented Sep 18, 2021

habu1010 Sep 18, 2021

Hourier Sep 18, 2021

habu1010 Sep 18, 2021

Hourier Sep 18, 2021

habu1010 Sep 18, 2021

Hourier Sep 18, 2021

habu1010 Sep 18, 2021

Hourier Sep 18, 2021

habu1010 Sep 18, 2021

Hourier Sep 18, 2021

habu1010 Sep 18, 2021

habu1010 Sep 18, 2021

Hourier Sep 18, 2021

Hourier commented Sep 18, 2021

habu1010 left a comment

habu1010 Sep 18, 2021

Hourier Sep 18, 2021

Hourier commented Sep 18, 2021

habu1010 left a comment

Hourier commented Sep 18, 2021

		@@ -0,0 +1,16 @@
		#pragma once

		#include "timed-effect/player-stun.h"

player_type から時限ステータスを分離した (朦朧のみ) #1548

player_type から時限ステータスを分離した (朦朧のみ) #1548

Conversation

Hourier commented Sep 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hourier commented Sep 18, 2021

habu1010 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hourier commented Sep 18, 2021

habu1010 left a comment

Choose a reason for hiding this comment

Hourier commented Sep 18, 2021