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

'vehicle' class cleanup #64266

Merged
merged 4 commits into from
Mar 17, 2023
Merged

Conversation

irwiss
Copy link
Contributor

@irwiss irwiss commented Mar 14, 2023

Summary

None

Purpose of change

Cleans various unused, rarely used, stubs and duplicate functions from vehicle class

Describe the solution

num_parts/num_true_parts/part_count needed special treatment;

  • part_count(no_fake = false) - the default overload, returns parts.size() and is a duplicate of num_parts() except it uses fake_parts.size()
  • part_count(no_fake = true) - iterated over 'parts' and counted all parts where is_fake is false
  • num_true_parts() - returns parts.size() - fake_parts.size()

The problem is num_true_parts is used generic_vehicle_part_range which is in turn used inside vehicle::refresh() while fake_parts is being filled. Trying to use the actual real part count in the range class results in bike rack test failures, so to keep status quo and keep tests happy these were refactored to 3 functions:

  • int part_count() const; - returns parts.size()
  • int part_count_real() const; - uses std::count_if to iterate and count parts with with is_fake false
  • int part_count_real_cached() const; - returns parts.size() - fake_parts.size() and used in the range class

I'm 90% sure generic_vehicle_part_range shouldn't be using fake_parts as it's being filled and it can cause issues, but it'll require a separate patch to figure out what's getting broken there

Unused:

  • int break_off( int p, int dmg )
  • void remove_owner()
  • turret_data turret_query( const vehicle_part &pt ) const; - used once as a DIY const_cast hack, not actually needed
  • turret_data turret_query( const tripoint &pos ) const;
  • const vehicle_part &cpart( int part_num ) const; - no uses, no implementation, part() has const overload
  • void force_erase_part( int part_num );
  • bool valid_part( int part_num ) const; - no implementation
  • bool has_any_parts() const; - unused
  • int removed_part_count = 0; // NOLINT(cata-serialize) - unused variable, only written to

Useless:

  • tripoint get_abs_diff( const tripoint &one, const tripoint &two ) const; - same as (one-two).abs() with extra steps

Rarely used:

  • int num_fake_parts() const; - only used in tests, moved to the test
  • int num_active_fake_parts() const; - only used in tests, moved to the test
  • int num_true_parts() const; - renamed to part_count_real() to match part_count()
  • void refresh_mass() const; - stub immediately calling calc_mass_center( true );

Duplicates:

  • int num_parts() const; - duplicate of part_count(), replaced with part_count as that one had more users

Removed overloads:

  • int part_count( bool no_fake = false ) const; only had 1 user with no_fake == true, split to no-parameter part_count() and part_count_real() (deduplicated and renamed from num_true_parts())
  • std::vector<vehicle_part *> lights( bool active = false ); - all users had set active = true, removed the parameter, only returns enabled lights

Misc:

  • bool real_or_active_fake_part( int part_num ) const; - was moved to vehicle_part, a few checks that actually checked this but didn't use the function weer made to use the function instead
  • A test was also adjusted to use CAPTURE(...)s instead of std::cout prints

Describe alternatives you've considered

Testing

Compiler and tests should hopefully catch me

Additional context

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. Vehicles Vehicles, parts, mechanics & interactions json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Mar 14, 2023
@irwiss irwiss marked this pull request as ready for review March 14, 2023 22:40
@irwiss irwiss force-pushed the veh-class-cleanup branch from 0fa6cdc to 1b42580 Compare March 14, 2023 22:41
@irwiss irwiss marked this pull request as draft March 14, 2023 23:25
@irwiss irwiss marked this pull request as ready for review March 15, 2023 20:53
@irwiss irwiss force-pushed the veh-class-cleanup branch from 1b42580 to 29102d2 Compare March 15, 2023 20:53
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Mar 16, 2023
@kevingranade kevingranade merged commit 605a2c7 into CleverRaven:master Mar 17, 2023
@irwiss irwiss deleted the veh-class-cleanup branch March 17, 2023 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. json-styled JSON lint passed, label assigned by github actions Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants