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

ItemEntity::tval/sval をBaseitemKey::bi_key として再定義した #2841

Merged

Conversation

Hourier
Copy link
Member

@Hourier Hourier commented Nov 26, 2022

掲題の通りです
Discord での発言通り、1コミットで135ファイルの手動編集がかかっております
文字通り差分が尋常でないため、このPRに関しては2人以上のOKをもってマージしようと思います
詳細な合議はDiscord で行うものとします (注:GitHubでのコメントもOKです)
確認よろしくお願いします

@Hourier Hourier added refactor 処理の整理、可読性の向上 Priority:MIDDLE 気が向いたときに相応に対処して欲しいもの labels Nov 26, 2022
@Hourier Hourier added this to the Next Sprint milestone Nov 26, 2022
@Hourier Hourier self-assigned this Nov 26, 2022
@Hourier Hourier added this to In Progress in C++移行 via automation Nov 26, 2022
@Hourier Hourier linked an issue Nov 26, 2022 that may be closed by this pull request
@Hourier Hourier force-pushed the Replace-ItemEntity-TvalSval-to-BaseitemKey branch 4 times, most recently from 239dbdf to c333944 Compare November 26, 2022 13:21
/* Type/Subtype */
o_ptr->tval = i2enum<ItemKindType>(rd_byte());
o_ptr->sval = rd_byte();
o_ptr->bi_key = BaseitemKey(i2enum<ItemKindType>(rd_byte()), rd_byte());
Copy link
Member

Choose a reason for hiding this comment

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

関数の実引数の評価順序は規定されていないので、どちらのrd_byteが先に呼ばれるかはコンパイラに依存します。
(実際にここのコードが使われることはないとは思いますが)

case PlayerClassType::ELEMENTALIST: {
if (player_ptr->inventory_list[INVEN_MAIN_HAND].tval <= ItemKindType::SWORD) {
case PlayerClassType::ELEMENTALIST:
if (tval_main <= ItemKindType::SWORD) {
cur_wgt += player_ptr->inventory_list[INVEN_MAIN_HAND].weight;
Copy link
Member

Choose a reason for hiding this comment

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

ここからの処理を item_main_hand と item_sub_hand で置き換えられます。

auto *o_ptr = &floor_ptr->o_list[this_o_idx];
if (o_ptr->tval == ItemKindType::CORPSE) {
auto corpse_r_idx = i2enum<MonsterRaceId>(o_ptr->pval);
const auto &o_ptr = floor_ptr->o_list[this_o_idx];
Copy link
Member

Choose a reason for hiding this comment

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

ポインタではない変数名が o_ptr のままです。

@Hourier
Copy link
Member Author

Hourier commented Nov 27, 2022

確認&コメントありがとうございます
修正してforce push しました

const BaseitemKey key(flavor_ptr->bow_ptr->tval, flavor_ptr->bow_ptr->sval);
if ((flavor_ptr->bow_ptr->bi_id != 0) && (key.tval() == key.get_arrow_kind())) {
const auto &bi_key = flavor_ptr->bow_ptr->bi_key;
if ((flavor_ptr->bow_ptr->bi_id != 0) && (bi_key.tval() == bi_key.get_arrow_kind())) {
Copy link
Member

Choose a reason for hiding this comment

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

前回の修正で比較の左側は o_ptr (矢弾)の tval を見なければならないところ、弓矢側の tval を見るようにしてしまったため、 #2862 の原因になっているようです。
この直後の鉄のくさびの判定も同様に間違っています。
合わせて修正しておいたほうがよさそうです。

Copy link
Member Author

Choose a reason for hiding this comment

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

「ここ常にfalseになりそうだけど大丈夫……?」と思ってましたが見事に大当たりでした
#2841 より前に修正して、左記はrebaseします

@Hourier Hourier force-pushed the Replace-ItemEntity-TvalSval-to-BaseitemKey branch from c0ac3c2 to d745744 Compare November 29, 2022 12:15
@Hourier
Copy link
Member Author

Hourier commented Nov 29, 2022

#2864 の修正を反映させてforce pushしました

@Hourier Hourier closed this Nov 29, 2022
@Hourier Hourier force-pushed the Replace-ItemEntity-TvalSval-to-BaseitemKey branch from d745744 to c9629a4 Compare November 29, 2022 12:17
C++移行 automation moved this from In Progress to Done Nov 29, 2022
@Hourier Hourier reopened this Nov 29, 2022
C++移行 automation moved this from Done to In Progress Nov 29, 2022
@Hourier Hourier force-pushed the Replace-ItemEntity-TvalSval-to-BaseitemKey branch 3 times, most recently from 8122682 to 0867a45 Compare December 1, 2022 11:24
@@ -135,7 +135,10 @@ static bool switch_random_art_bias(ItemEntity *o_ptr)

static bool random_art_bias_decrease_mana(ItemEntity *o_ptr)
{
if (((o_ptr->artifact_bias != BIAS_MAGE) && (o_ptr->artifact_bias != BIAS_PRIESTLY)) || (o_ptr->tval != ItemKindType::SOFT_ARMOR) || (o_ptr->sval != SV_ROBE) || o_ptr->art_flags.has(TR_DEC_MANA) || !one_in_(3)) {
auto should_decrease = (o_ptr->artifact_bias != BIAS_MAGE) && (o_ptr->artifact_bias != BIAS_PRIESTLY);
Copy link
Member

Choose a reason for hiding this comment

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

論理的処理自体は間違っていないようですが、「減らすべき」らしい判定でtrueの時除外する処理になっている気がします。混乱しそうなので命名変えた方がいい気がします。

Copy link
Member Author

Choose a reason for hiding this comment

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

should_skip にしておきます

@Hourier Hourier force-pushed the Replace-ItemEntity-TvalSval-to-BaseitemKey branch from 0867a45 to d3e5568 Compare December 1, 2022 12:53
@@ -285,12 +287,13 @@ void exe_eat_food(PlayerType *player_ptr, INVENTORY_IDX item)
}

if (PlayerRace(player_ptr).equals(PlayerRaceType::SKELETON)) {
if (!((o_ptr->sval == SV_FOOD_WAYBREAD) || (o_ptr->sval < SV_FOOD_BISCUIT))) {
const auto sval = bi_key.sval();
if ((sval != SV_FOOD_WAYBREAD) && (sval >= SV_FOOD_BISCUIT)) {
Copy link
Member

Choose a reason for hiding this comment

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

(備忘)王道を征くド・モルガン。問題なしと判断。

}
break;
const auto flgs = object_flags_known(o_ptr);
return flgs.has(TR_BLESSED) || (tval == ItemKindType::HAFTED);
Copy link
Member

Choose a reason for hiding this comment

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

ド=モルガンに基づいたロジック修正確認。


break;
auto flgs = object_flags_known(o_ptr);
return flgs.has(TR_RIDING);
Copy link
Member

Choose a reason for hiding this comment

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

ロジックの反転なし。

}
break;

return player_ptr->weapon_exp_max[tval][sval] >= PlayerSkill::weapon_exp_at(PlayerSkillRank::MASTER);
Copy link
Member

Choose a reason for hiding this comment

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

ロジックの反転なし。

}
break;

return player_ptr->weapon_exp_max[tval][sval] > PlayerSkill::weapon_exp_at(PlayerSkillRank::BEGINNER);
Copy link
Member

Choose a reason for hiding this comment

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

ロジックの反転なし。

should_mp_decrease &= flgs.has_not(TR_FREE_ACT);
should_mp_decrease &= flgs.has_not(TR_DEC_MANA);
should_mp_decrease &= flgs.has_not(TR_EASY_SPELL);
should_mp_decrease &= flgs.has_not(TR_MAGIC_MASTERY) || (o_ptr->pval <= 0);
Copy link
Member

Choose a reason for hiding this comment

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

ロジックの反転なし。

should_mp_decrease &= flgs.has_not(TR_DEC_MANA);
should_mp_decrease &= flgs.has_not(TR_EASY_SPELL);
should_mp_decrease &= flgs.has_not(TR_MAGIC_MASTERY) || (o_ptr->pval <= 0);
should_mp_decrease &= flgs.has_not(TR_DEX) || (o_ptr->pval <= 0);
Copy link
Member

Choose a reason for hiding this comment

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

ロジックの反転問題なし。

if ((o_ptr->sval == SV_LITE_TORCH) && !(o_ptr->fuel > 0)) {
if (o_ptr->bi_key.tval() == ItemKindType::LITE) {
const auto sval = o_ptr->bi_key.sval();
if ((sval == SV_LITE_TORCH) && (o_ptr->fuel <= 0)) {
Copy link
Member

Choose a reason for hiding this comment

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

ロジック反転問題なし。

continue;
}

if ((o_ptr->sval == SV_LITE_LANTERN) && !(o_ptr->fuel > 0)) {
if ((sval == SV_LITE_LANTERN) && (o_ptr->fuel <= 0)) {
Copy link
Member

Choose a reason for hiding this comment

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

ロジック反転問題なし。

Copy link
Member

@sikabane-works sikabane-works left a comment

Choose a reason for hiding this comment

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

お待たせしました。どうにか問題なさそうです。マージどうぞ。

@Hourier
Copy link
Member Author

Hourier commented Dec 8, 2022

確認ありがとうございました!
当初予定通り、Habu氏にも確認頂いた上でマージする運びとします
少々お待ち下さい

Copy link
Member

@habu1010 habu1010 left a comment

Choose a reason for hiding this comment

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

前回チェックした時から変更されているファイルを見ましたが、とりあえず問題なさそうですのでApproveします。

@Hourier Hourier merged commit 07f7bff into hengband:develop Dec 9, 2022
C++移行 automation moved this from In Progress to Done Dec 9, 2022
@Hourier Hourier deleted the Replace-ItemEntity-TvalSval-to-BaseitemKey branch December 9, 2022 03:39
@Hourier
Copy link
Member Author

Hourier commented Dec 9, 2022

確認ありがとうございました!
2人からOKが出たのでマージしました

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:MIDDLE 気が向いたときに相応に対処して欲しいもの refactor 処理の整理、可読性の向上
3 participants