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.

Sign up for GitHub

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

Jump to bottom

Character code reorganization 27-nov-2022 #2208

Merged
merged 32 commits into from
Dec 16, 2022
Merged

Character code reorganization 27-nov-2022 #2208

merged 32 commits into from
Dec 16, 2022

Conversation

olanti-p
Copy link
Contributor

@olanti-p olanti-p commented Nov 27, 2022 •
edited
Loading

Summary

SUMMARY: Infrastructure "Reorganized some Character-related code"

Purpose of change

Dismantling player.

Describe the solution

See commits.

  1. Move some functions and members out of player into Character
  2. Move some functions out of player and turn them into namespaced functions
  3. Break up character_functions.cpp as it keeps growing
  4. Create character_turn.h/.cpp for various process-on-each-turn functions
  5. Rename some moved functions
  6. Opportunistically refactor questionable spots
  7. Fix some bugs caused by wrong usage of passed item vs wielded item, or passed position vs character position
  8. Get rid of bionic indexing

Testing

Game compiles. No behavior changes intended.

TODO:

  • Playtest bionics ui
  • Fix clang-tidy warnings

Additional context

That's a lot of line changes, may cause lot of merge conflicts. Should I break it up?

@olanti-p
Move is_hallucination(), underwater check
54aa873
@olanti-p
Move dodge_roll()
b54dfc1
@olanti-p
Move reach_attack()
5108558
@olanti-p
Get rid of melee_attack() overload, use default arg
1cbcb8f
@olanti-p
Fix melee_value() checking wrong item
b0fb445
@olanti-p
Move weapon_value() family, encapsulate cached_info
3e5b188
@olanti-p
Remove dead code
bfd6b7e
@olanti-p
Get rid of get_artifact_items()
991503f
@olanti-p
Move can_autolearn()
3fe9406
@olanti-p
Move is_dead_state()
c29f414
@olanti-p
Move power_bionics(), get rid of bionic indexes
bd0be35
@olanti-p
Move power_mutations() and related definitions
bc17df2
@olanti-p
Move and rename calc_fatigue_cap()
cd5c75b
@olanti-p
Move needs_rates
5fa34cb
@olanti-p
Move sleep-related definitions, fix wrong pos usages
c2c27df
@olanti-p
Move avoid_trap()
a7593be
@olanti-p
Move vomit_mod()
f9b92ae
@olanti-p
Move update_body_wetness()
c11286f
@olanti-p
Move omt_path member from bionics section
c323182
@olanti-p
Move can_interface_armor()
64cc6d4
@olanti-p
Move has_mission_item()
b895042
@olanti-p
Move basic_symbol_color()
a0fa48a
@olanti-p
Move pause() and search_surroundings()
acdfc96
@olanti-p
Move talk_skill() and intimidation()
97b1785
@olanti-p
Move is_stealthy()
59a45c9
@olanti-p
Move stability_roll()
cf46d5e
@olanti-p
Break up character_functions.cpp
141b0a5
@olanti-p
Move and merge turn processing functions
ec42a58
@olanti-p
Move and rename weapname()
ddd4a6c
@olanti-p
Move pain-related functions
c78b905
@github-actions github-actions bot added src changes related to source code. tests changes related to tests labels Nov 27, 2022
@olanti-p olanti-p changed the title [WiP] Character code reorganization 27-nov-2022 Character code reorganization 27-nov-2022 Nov 28, 2022
@Coolthulhu Coolthulhu self-assigned this Dec 11, 2022
@Coolthulhu Coolthulhu merged commit cc23abc into cataclysmbnteam:upload Dec 16, 2022
@Coolthulhu
Copy link
Member

Much cleaner now!

That can_sleep thing reminded me we don't have a standard for "rng checking" functions.
roll_* is fine, but it feels a bit weird, like with bool getters named like get_male(). Feels like there could be a better prefix, like is_* is for bool getters.
Or maybe it's just an artifact of the old sleep check being written as one "check and maybe do" monolith instead of

if( roll() >= difficulty ) {
  do();
}

I'm pretty sure we do have some of the get_* bool getters. I'll try to rename them when change anything about them.

@olanti-p olanti-p deleted the code-reorg-27-nov-2022 branch December 17, 2022 22:06
@Coolthulhu Coolthulhu mentioned this pull request Dec 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Coolthulhu Coolthulhu

Labels
src changes related to source code. tests changes related to tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@olanti-p @Coolthulhu