Skip to content

Commit

Permalink
Refactor power grid traverse code (cataclysmbnteam#1599)
Browse files Browse the repository at this point in the history 
I was randomly reading code and found this function confusing. Refactored for readability.

* Split graph traversion and whatever caller wants to do with items.
* Added comments in places where I failed to understand fast.
* Removed `amount` updating from traversing and few hacks on call-sites caused by this.
* Removed redundant "only vehicles" traversion method.
* Removed raw pointers from public interface.
  • Loading branch information
@AngelicosPhosphoros
AngelicosPhosphoros authored Jun 10, 2022
1 parent fe7444e commit 8d144d2
Show file tree
Hide file tree
Showing 3 changed files with 120 additions and 98 deletions.
33 changes: 29 additions & 4 deletions src/distribution_grid.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,40 @@ class distribution_grid_tracker
void on_options_changed();
};

class vehicle;

namespace distribution_graph
{
enum class traverse_visitor_result {
stop,
continue_further,
};

/**
* Traverses the graph of connected vehicles and grids.
/** Traverses the graph of connected vehicles and grids.
*
* Runs Breadth-First over all vehicles and grids calling passed actions on each of them
* until any visitor action return traverse_visitor_result::stop.
* Connected vehicles checked by all POWER_TRANSFER part and grids by vehicle connectors.
*
* @param start Reference to the start node of the graph. Assumed to be already visited.
* @param veh_action Visitor function which accepts vehicle& or const & then returns traverse_visitor_result.
* @param grid_action Visitor function which accepts distribution_grid& or const & then returns traverse_visitor_result.
*/
template <typename VehFunc, typename GridFunc, typename StartPoint>
int traverse( StartPoint *start, int amount,
VehFunc veh_action, GridFunc grid_action );
void traverse( StartPoint &start,
VehFunc veh_action, GridFunc grid_action );

/* Useful if we want to act only in one type. */
inline constexpr traverse_visitor_result noop_visitor_grid( const distribution_grid & )
{
return traverse_visitor_result::continue_further;
}

/* Useful if we want to act only in one type. */
inline constexpr traverse_visitor_result noop_visitor_veh( const vehicle & )
{
return traverse_visitor_result::continue_further;
}

} // namespace distribution_graph

Expand Down
169 changes: 91 additions & 78 deletions src/vehicle.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3183,18 +3183,17 @@ int vehicle::fuel_left( const itype_id &ftype, bool recurse ) const
} );

if( recurse && ftype == fuel_type_battery ) {
auto fuel_counting_visitor = [&]( vehicle const * veh, int amount ) {
return amount + veh->fuel_left( ftype, false );
using tvr = distribution_graph::traverse_visitor_result;
auto fuel_counting_visitor = [&fl, &ftype]( vehicle const & veh ) {
fl += veh.fuel_left( ftype, false );
return tvr::continue_further;
};
auto power_counting_visitor = [&]( distribution_grid const * grid, int amount ) {
return amount + grid->get_resource( false );
auto power_counting_visitor = [&fl]( distribution_grid const & grid ) {
fl += grid.get_resource( false );
return tvr::continue_further;
};

// HAX: add 1 to the initial amount so traversal doesn't immediately stop just
// 'cause we have 0 fuel left in the current vehicle. Subtract the 1 immediately
// after traversal.
fl = distribution_graph::traverse( this, fl + 1,
fuel_counting_visitor, power_counting_visitor ) - 1;
distribution_graph::traverse( *this, fuel_counting_visitor, power_counting_visitor );
}

//muscle engines have infinite fuel
Expand Down Expand Up @@ -4759,27 +4758,18 @@ vehicle *vehicle::find_vehicle( const tripoint &where )
void vehicle::enumerate_vehicles( std::map<vehicle *, bool> &connected_vehicles,
const std::set<vehicle *> &vehicle_list )
{
auto enumerate_visitor = [&connected_vehicles]( vehicle * veh, int amount ) {
auto enumerate_visitor = [&connected_vehicles]( vehicle & veh ) {
// Only emplaces if element is not present already.
connected_vehicles.emplace( veh, false );
return amount;
connected_vehicles.emplace( &veh, false );
return distribution_graph::traverse_visitor_result::continue_further;
};
for( vehicle *veh : vehicle_list ) {
// This autovivifies, and also overwrites the value if already present.
connected_vehicles[veh] = true;
traverse_vehicle_graph( veh, 1, enumerate_visitor );
distribution_graph::traverse( *veh, enumerate_visitor, distribution_graph::noop_visitor_grid );
}
}

template <typename Func, typename Vehicle>
int vehicle::traverse_vehicle_graph( Vehicle *start_veh, int amount, Func action )
{
const auto do_nothing = []( const distribution_grid *, int amt ) {
return amt;
};
return distribution_graph::traverse( start_veh, amount, action, do_nothing );
}

// TODO: It looks out of place in vehicle.cpp
namespace distribution_graph
{
Expand Down Expand Up @@ -4820,66 +4810,87 @@ struct vehicle_or_grid {
};

template <typename VehFunc, typename GridFunc, typename StartPoint>
int traverse( StartPoint *start, int amount,
VehFunc veh_action, GridFunc grid_action )
void traverse( StartPoint &start,
VehFunc veh_action, GridFunc grid_action )
{
using tvr = traverse_visitor_result;
constexpr bool IsConst = std::is_const<StartPoint>::value;
struct hash {
const std::hash<char> char_hash = std::hash<char>();
const std::hash<size_t> ptr_hash = std::hash<size_t>();
auto operator()( const vehicle_or_grid<IsConst> &elem ) const {
return char_hash( static_cast<char>( elem.type ) ) ^
( ptr_hash( reinterpret_cast<size_t>( elem.veh ) | reinterpret_cast<size_t>( elem.grid ) ) );
ptr_hash(
// Because only one of pointers is not nullptr, binary OR would get value of set pointer.
reinterpret_cast<size_t>( elem.veh ) | reinterpret_cast<size_t>( elem.grid )
);
}
};

// Actually, they are visited elements with unvisited neighbours.
// Not all connected elements are here.
std::queue<vehicle_or_grid<IsConst>> connected_elements;
// For fast checking if we should visit some neighbour.
std::unordered_set<vehicle_or_grid<IsConst>, hash> visited_elements;
connected_elements.emplace( start );
visited_elements.insert( start );
connected_elements.emplace( &start );
visited_elements.insert( &start );
auto &grid_tracker = get_distribution_grid_tracker();

auto enqueue = [&connected_elements, &visited_elements]( vehicle_or_grid<IsConst> newly_visited ) {
connected_elements.push( newly_visited );
visited_elements.insert( newly_visited );
};
auto was_already_visited = [&visited_elements]( vehicle_or_grid<IsConst> to_visit ) {
return visited_elements.count( to_visit ) != 0;
};

auto process_vehicle = [&]( const tripoint_abs_ms & target_pos ) {
auto *veh = vehicle::find_vehicle( target_pos );
if( veh == nullptr ) {
debugmsg( "lost vehicle at %s", target_pos.to_string() );
return tvr::continue_further;
}
// Add this connected vehicle to the queue of vehicles to search next,
// but only if we haven't seen this one before.
if( veh != nullptr && visited_elements.count( veh ) == 0 ) {
connected_elements.emplace( veh );
visited_elements.insert( veh );

amount = veh_action( veh, amount );
g->u.add_msg_if_player( m_debug, "After remote veh %p, %d power",
static_cast<void *>( veh ), amount );
if( was_already_visited( veh ) ) {
return tvr::continue_further;
}

const tvr result = veh_action( *veh );
g->u.add_msg_if_player( m_debug, "After remote veh %p",
static_cast<void *>( veh ) );

return amount < 1;
// We do not need to check neighbours if we stop.
if( result == tvr::continue_further ) {
enqueue( veh );
}

return false;
return result;
};

auto process_grid = [&]( const tripoint_abs_ms & target_pos ) {
auto *grid = &grid_tracker.grid_at( target_pos );
if( !*grid ) {
auto &grid = grid_tracker.grid_at( target_pos );
if( !grid ) {
debugmsg( "lost grid at %s", target_pos.to_string() );
return tvr::continue_further;
}
if( *grid && visited_elements.count( grid ) == 0 ) {
connected_elements.emplace( grid );
visited_elements.insert( grid );

amount = grid_action( grid, amount );
g->u.add_msg_if_player( m_debug, "After remote grid %p, %d power",
static_cast<void *>( grid ), amount );
if( was_already_visited( &grid ) ) {
return tvr::continue_further;
}

return amount < 1;
const tvr result = grid_action( grid );
g->u.add_msg_if_player( m_debug, "After remote grid %p",
static_cast<void *>( &grid ) );

// We do not need to check neighbours if we stop.
if( result == tvr::continue_further ) {
enqueue( &grid );
}

return false;
return result;
};

while( amount > 0 && !connected_elements.empty() ) {
while( !connected_elements.empty() ) {
auto current = connected_elements.front();
connected_elements.pop();

Expand All @@ -4893,12 +4904,12 @@ int traverse( StartPoint *start, int amount,

const tripoint_abs_ms target_pos( current_veh.cpart( p ).target.second );
if( current_veh.cpart( p ).has_flag( vehicle_part::targets_grid ) ) {
if( process_grid( target_pos ) ) {
break;
if( process_grid( target_pos ) == tvr::stop ) {
return;
}
} else {
if( process_vehicle( target_pos ) ) {
break;
if( process_vehicle( target_pos ) == tvr::stop ) {
return;
}
}
}
Expand All @@ -4912,14 +4923,13 @@ int traverse( StartPoint *start, int amount,
}

for( const tripoint_abs_ms &target_pos : connector->connected_vehicles ) {
if( process_vehicle( target_pos ) ) {
break;
if( process_vehicle( target_pos ) == tvr::stop ) {
return;
}
}
}
}
}
return amount;
}

} // namespace distribution_graph
Expand Down Expand Up @@ -4951,18 +4961,20 @@ int vehicle::charge_battery( int amount, bool include_other_vehicles )
}
}

auto charge_veh = []( vehicle * veh, int amount ) {
g->u.add_msg_if_player( m_debug, "CHv: %d", amount );
return veh->charge_battery( amount, false );
};
auto charge_grid = []( distribution_grid * grid, int amount ) {
g->u.add_msg_if_player( m_debug, "CHg: %d", amount );
return grid->mod_resource( amount, false );
};

if( amount > 0 && include_other_vehicles ) {
// still a bit of charge we could send out...
amount = distribution_graph::traverse( this, amount, charge_veh, charge_grid );
using tvr = distribution_graph::traverse_visitor_result;
auto charge_veh = [&amount]( vehicle & veh ) {
g->u.add_msg_if_player( m_debug, "CHv: %d", amount );
amount = veh.charge_battery( amount, false );
return amount > 0 ? tvr::continue_further : tvr::stop;
};
auto charge_grid = [&amount]( distribution_grid & grid ) {
g->u.add_msg_if_player( m_debug, "CHg: %d", amount );
amount = grid.mod_resource( amount, false );
return amount > 0 ? tvr::continue_further : tvr::stop;
};
distribution_graph::traverse( *this, charge_veh, charge_grid );
}


Expand Down Expand Up @@ -4995,17 +5007,20 @@ int vehicle::discharge_battery( int amount, bool recurse )
}
}

auto discharge_vehicle = []( vehicle * veh, int amount ) {
g->u.add_msg_if_player( m_debug, "CHv: %d", amount );
return veh->discharge_battery( amount, false );
};
auto discharge_grid = []( distribution_grid * grid, int amount ) {
g->u.add_msg_if_player( m_debug, "CHg: %d", amount );
return -grid->mod_resource( -amount, false );
};
if( amount > 0 && recurse ) {
// need more power!
amount = distribution_graph::traverse( this, amount, discharge_vehicle, discharge_grid );
using tvr = distribution_graph::traverse_visitor_result;
auto discharge_vehicle = [&amount]( vehicle & veh ) {
g->u.add_msg_if_player( m_debug, "CHv: %d", amount );
amount = veh.discharge_battery( amount, false );
return amount > 0 ? tvr::continue_further : tvr::stop;
};
auto discharge_grid = [&amount]( distribution_grid & grid ) {
g->u.add_msg_if_player( m_debug, "CHg: %d", amount );
amount = -grid.mod_resource( -amount, false );
return amount > 0 ? tvr::continue_further : tvr::stop;
};
distribution_graph::traverse( *this, discharge_vehicle, discharge_grid );
}

return amount; // non-zero if we weren't able to fulfill demand.
Expand Down Expand Up @@ -5389,10 +5404,8 @@ void vehicle::gain_moves()
// Force off-map vehicles to load by visiting them every time we gain moves.
// Shouldn't be too expensive if there aren't fifty trillion vehicles in the graph...
// ...and if there are, it's the player's fault for putting them there.
auto nil_visitor = []( vehicle *, int amount ) {
return amount;
};
traverse_vehicle_graph( this, 1, nil_visitor );
distribution_graph::traverse( *this, distribution_graph::noop_visitor_veh,
distribution_graph::noop_visitor_grid );

if( check_environmental_effects ) {
check_environmental_effects = do_environmental_effects();
Expand Down
16 changes: 0 additions & 16 deletions src/vehicle.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -748,22 +748,6 @@ class vehicle
return find_vehicle( where.raw() );
}

private:
/**
* Traverses the graph of connected vehicles, starting from start_veh, and continuing
* along all vehicles connected by some kind of POWER_TRANSFER part.
* @param start_veh The vehicle to start traversing from. NB: the start_vehicle is
* assumed to have been already visited!
* @param amount An amount of power to traverse with. This is passed back to the visitor,
* and reset to the visitor's return value at each step.
* @param action A function(vehicle* veh, int amount, int loss) returning int. The function
* may do whatever it desires, and may be a lambda (including a capturing lambda).
* NB: returning 0 from a visitor will stop traversal immediately!
* @return The last visitor's return value.
*/
template <typename Func, typename Vehicle>
static int traverse_vehicle_graph( Vehicle *start_veh, int amount, Func action );
public:
vehicle( const vproto_id &type_id, int init_veh_fuel = -1, int init_veh_status = -1 );
vehicle();
vehicle( const vehicle & ) = delete;
Expand Down

0 comments on commit 8d144d2

Please sign in to comment.