From 8229677beb10362af4c390f9a44d77109e4ae90c Mon Sep 17 00:00:00 2001 From: irwiss Date: Tue, 11 Jul 2023 14:10:26 +0300 Subject: [PATCH] Make vehicle::remove_part use references --- src/item_location.cpp | 5 ++-- src/mapgen.cpp | 5 ++-- src/veh_interact.cpp | 6 ++--- src/veh_utils.cpp | 3 +-- src/vehicle.cpp | 61 ++++++++++++++++++++----------------------- src/vehicle.h | 10 +++++-- tests/vision_test.cpp | 2 +- 7 files changed, 47 insertions(+), 45 deletions(-) diff --git a/src/item_location.cpp b/src/item_location.cpp index 6e7dd9a01e72a..8820a50a155e7 100644 --- a/src/item_location.cpp +++ b/src/item_location.cpp @@ -516,9 +516,10 @@ class item_location::impl::item_on_vehicle : public item_location::impl void remove_item() override { on_contents_changed(); - item &base = cur.veh.part( cur.part ).base; + vehicle_part &vp = cur.veh.part( cur.part ); + item &base = vp.base; if( &base == target() ) { - cur.veh.remove_part( cur.part ); // vehicle_part::base + cur.veh.remove_part( vp ); // vehicle_part::base } else { cur.remove_item( *target() ); // item within CARGO } diff --git a/src/mapgen.cpp b/src/mapgen.cpp index 55a190b4f40a4..7949118017cf1 100644 --- a/src/mapgen.cpp +++ b/src/mapgen.cpp @@ -6735,10 +6735,11 @@ std::unique_ptr map::add_vehicle_to_map( std::vector parts_in_square = veh_to_add->parts_at_relative( source_point, true ); std::set parts_to_check; for( int index = parts_in_square.size() - 1; index >= 0; index-- ) { + vehicle_part &vp = veh_to_add->part( parts_in_square[index] ); if( handler_ptr ) { - veh_to_add->remove_part( parts_in_square[index], *handler_ptr ); + veh_to_add->remove_part( vp, *handler_ptr ); } else { - veh_to_add->remove_part( parts_in_square[index] ); + veh_to_add->remove_part( vp ); } parts_to_check.insert( parts_in_square[index] ); } diff --git a/src/veh_interact.cpp b/src/veh_interact.cpp index ae12492402157..1902434b4db28 100644 --- a/src/veh_interact.cpp +++ b/src/veh_interact.cpp @@ -3274,7 +3274,7 @@ void veh_interact::complete_vehicle( Character &you ) return; } } - const vehicle_part &vp = veh.part( vp_index ); + vehicle_part &vp = veh.part( vp_index ); const vpart_info &vpi = vp.info(); const bool appliance_removal = static_cast( you.activity.index ) == 'O'; const bool wall_wire_removal = appliance_removal && vpi.id == vpart_ap_wall_wiring; @@ -3306,7 +3306,7 @@ void veh_interact::complete_vehicle( Character &you ) // Power cables must remove parts from the target vehicle, too. if( vpi.has_flag( "POWER_TRANSFER" ) ) { - veh.remove_remote_part( vp_index ); + veh.remove_remote_part( vp ); } if( broken ) { @@ -3353,7 +3353,7 @@ void veh_interact::complete_vehicle( Character &you ) here.destroy_vehicle( &veh ); } else { const tripoint part_pos = veh.global_part_pos3( vp ); - veh.remove_part( vp_index ); + veh.remove_part( vp ); // part_removal_cleanup calls refresh, so parts_at_relative is valid veh.part_removal_cleanup(); if( veh.parts_at_relative( vp.mount, true ).empty() ) { diff --git a/src/veh_utils.cpp b/src/veh_utils.cpp index 1e23a58d1a47f..2090c369f5ac5 100644 --- a/src/veh_utils.cpp +++ b/src/veh_utils.cpp @@ -92,7 +92,6 @@ vehicle_part *most_repairable_part( vehicle &veh, Character &who ) bool repair_part( vehicle &veh, vehicle_part &pt, Character &who ) { - int part_index = veh.index_of_part( &pt ); const vpart_info &vp = pt.info(); const requirement_data reqs = pt.is_broken() @@ -141,7 +140,7 @@ bool repair_part( vehicle &veh, vehicle_part &pt, Character &who ) const units::angle direction = pt.direction; const std::string variant = pt.variant; get_map().spawn_items( who.pos(), pt.pieces_for_broken_part() ); - veh.remove_part( part_index ); + veh.remove_part( pt ); const int partnum = veh.install_part( mount, vpid, std::move( base ) ); if( partnum >= 0 ) { vehicle_part &vp = veh.part( partnum ); diff --git a/src/vehicle.cpp b/src/vehicle.cpp index f34ec07b31e59..445d270143979 100644 --- a/src/vehicle.cpp +++ b/src/vehicle.cpp @@ -875,11 +875,13 @@ void vehicle::smash( map &m, float hp_percent_loss_min, float hp_percent_loss_ma if( p == other_p ) { continue; } - const vpart_info &p_info = parts[p].info(); - const vpart_info &other_p_info = parts[other_p].info(); + vehicle_part &vp1 = parts[p]; + vehicle_part &vp2 = parts[other_p]; + const vpart_info &vpi1 = vp1.info(); + const vpart_info &vpi2 = vp2.info(); - if( p_info.id == other_p_info.id || - ( !p_info.location.empty() && p_info.location == other_p_info.location ) ) { + if( vpi1.id == vpi2.id || + ( !vpi1.location.empty() && vpi1.location == vpi2.location ) ) { // Deferred creation of the handler to here so it is only created when actually needed. if( !handler_ptr ) { // This is a heuristic: we just assume the default handler is good enough when called @@ -892,9 +894,9 @@ void vehicle::smash( map &m, float hp_percent_loss_min, float hp_percent_loss_ma } } if( part.is_fake ) { - remove_part( p, *handler_ptr ); + remove_part( vp1, *handler_ptr ); } else { - remove_part( other_p, *handler_ptr ); + remove_part( vp2, *handler_ptr ); } } } @@ -1780,28 +1782,19 @@ bool vehicle::merge_vehicle_parts( vehicle *veh ) return true; } -/** - * Mark a part as removed from the vehicle. - * @return bool true if the vehicle's 0,0 point shifted. - */ -bool vehicle::remove_part( const int p ) +bool vehicle::remove_part( vehicle_part &vp ) { DefaultRemovePartHandler handler; - return remove_part( p, handler ); + return remove_part( vp, handler ); } -bool vehicle::remove_part( const int p, RemovePartHandler &handler ) +bool vehicle::remove_part( vehicle_part &vp, RemovePartHandler &handler ) { // NOTE: Don't access g or g->m or anything from it directly here. // Forward all access to the handler. // There are currently two implementations of it: // - one for normal game play (vehicle is on the main map g->m), // - one for mapgen (vehicle is on a temporary map used only during mapgen). - if( p >= static_cast( parts.size() ) ) { - debugmsg( "Tried to remove part %d but only %d parts!", p, parts.size() ); - return false; - } - vehicle_part &vp = parts[p]; const vpart_info &vpi = vp.info(); if( vp.removed ) { /* This happens only when we had to remove part, because it was depending on @@ -1837,9 +1830,9 @@ bool vehicle::remove_part( const int p, RemovePartHandler &handler ) if( magic || ( dep < 0 ) || !vpi.has_flag( parent_flag ) ) { return false; } - const vehicle_part &vp_dep = parts[dep]; + vehicle_part &vp_dep = parts[dep]; handler.add_item_or_charges( part_loc, vp_dep.properties_to_item(), false ); - remove_part( dep, handler ); + remove_part( vp_dep, handler ); return true; }; @@ -1901,7 +1894,8 @@ bool vehicle::remove_part( const int p, RemovePartHandler &handler ) parts[vp.fake_part_at].removed = true; } - handler.removed( *this, p ); + const int vp_idx = index_of_part( &vp, /* include_removed = */ true ); + handler.removed( *this, vp_idx ); const point &vp_mount = vp.mount; const auto iter = labels.find( label( vp_mount ) ); @@ -1909,7 +1903,7 @@ bool vehicle::remove_part( const int p, RemovePartHandler &handler ) labels.erase( iter ); } - for( item &i : get_items( p ) ) { + for( item &i : get_items( vp_idx ) ) { // Note: this can spawn items on the other side of the wall! // TODO: fix this ^^ if( !magic ) { @@ -6543,7 +6537,7 @@ void vehicle::invalidate_towing( bool first_vehicle, Character *remover ) } else { get_map().add_item_or_charges( global_part_pos3( vp ), drop ); } - remove_part( tow_cable_idx ); + remove_part( vp ); } if( other_veh ) { other_veh->invalidate_towing(); @@ -6552,7 +6546,8 @@ void vehicle::invalidate_towing( bool first_vehicle, Character *remover ) } else { const int tow_cable_idx = get_tow_part(); if( tow_cable_idx > -1 ) { - remove_part( tow_cable_idx ); + vehicle_part &vp = parts[tow_cable_idx]; + remove_part( vp ); } tow_data.clear_towing(); } @@ -6632,7 +6627,7 @@ void vehicle::remove_remote_part( const vehicle_part &vp_local ) const for( const int remote_partnum : veh->loose_parts ) { vehicle_part &vp_remote = veh->parts[remote_partnum]; if( vp_remote.info().has_flag( "POWER_TRANSFER" ) && vp_remote.target.first == local_abs ) { - veh->remove_part( remote_partnum ); + veh->remove_part( vp_remote ); return; } } @@ -6684,7 +6679,7 @@ void vehicle::shed_loose_parts( const tripoint_bub_ms *src, const tripoint_bub_m here.add_item_or_charges( global_part_pos3( vp_loose ), drop ); } - remove_part( elem ); + remove_part( vp_loose ); } } @@ -6931,7 +6926,7 @@ bool vehicle::shift_if_needed( map &here ) int vehicle::break_off( map &here, int p, int dmg ) { - const vehicle_part &vp = parts[p]; + vehicle_part &vp = parts[p]; const vpart_info &vpi = vp.info(); /* Already-destroyed part - chance it could be torn off into pieces. * Chance increases with damage, and decreases with part max durability @@ -6990,12 +6985,12 @@ int vehicle::break_off( map &here, int p, int dmg ) here.add_item_or_charges( pos, vp_here.properties_to_item() ); } } - remove_part( parts_in_square[index], *handler_ptr ); + remove_part( vp_here, *handler_ptr ); } // After clearing the frame, remove it. add_msg_if_player_sees( pos, m_bad, _( "The %1$s's %2$s is destroyed!" ), name, vp.name() ); scatter_parts( vp ); - remove_part( p, *handler_ptr ); + remove_part( vp, *handler_ptr ); find_and_split_vehicles( here, { p } ); } else { if( vpi.has_flag( "TOW_CABLE" ) ) { @@ -7013,7 +7008,7 @@ int vehicle::break_off( map &here, int p, int dmg ) scatter_parts( vp ); } const point position = vp.mount; - remove_part( p, *handler_ptr ); + remove_part( vp, *handler_ptr ); // remove parts for which required flags are not present anymore if( !vpi.get_flags().empty() ) { @@ -7042,7 +7037,7 @@ int vehicle::break_off( map &here, int p, int dmg ) remove_remote_part( vp_here ); } here.add_item_or_charges( pos, vp_here.properties_to_item() ); - remove_part( part, *handler_ptr ); + remove_part( vp_here, *handler_ptr ); } } } @@ -7158,9 +7153,9 @@ int vehicle::damage_direct( map &here, int p, int dmg, const damage_type_id &typ } if( !g || &get_map() != &here ) { MapgenRemovePartHandler handler( here ); - remove_part( p, handler ); + remove_part( vp, handler ); } else { - remove_part( p ); + remove_part( vp ); } } } diff --git a/src/vehicle.h b/src/vehicle.h index 25cf340d343c9..670b3697b061a 100644 --- a/src/vehicle.h +++ b/src/vehicle.h @@ -1062,8 +1062,14 @@ class vehicle * on the temporary mapgen map), and when called during normal game play (when items * go on the main map g->m). */ - bool remove_part( int p, RemovePartHandler &handler ); - bool remove_part( int p ); + + // Mark a part to be removed from the vehicle. + // @returns true if the vehicle's 0,0 point shifted. + bool remove_part( vehicle_part &vp ); + // Mark a part to be removed from the vehicle. + // @returns true if the vehicle's 0,0 point shifted. + bool remove_part( vehicle_part &vp, RemovePartHandler &handler ); + void part_removal_cleanup(); // inner look for part_removal_cleanup. returns true if a part is removed // also called by remove_fake_parts diff --git a/tests/vision_test.cpp b/tests/vision_test.cpp index 88732c40f36d0..ea7c50576f2a7 100644 --- a/tests/vision_test.cpp +++ b/tests/vision_test.cpp @@ -908,7 +908,7 @@ TEST_CASE( "vision_vehicle_camera_skew", "[shadowcasting][vision][vehicle][vehic auto const fiddle_parts = [&]() { if( fiddle > 0 ) { std::vector const horns = v->get_parts_at( v->global_pos3(), "HORN", {} ); - v->remove_part( v->index_of_part( horns.front(), true ) ); + v->remove_part( *horns.front() ); } if( fiddle > 1 ) { REQUIRE( v->install_part( point_zero, vpart_inboard_mirror ) != -1 );