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

Display why vehicle parts cannot be installed #65341

Merged
merged 4 commits into from
May 3, 2023
Merged

Display why vehicle parts cannot be installed #65341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ea9cc6e
Display why vehicle parts cannot be installed
RenechCDDA Apr 27, 2023
28aa585
Apply suggestions from code review
RenechCDDA May 1, 2023
bd38c2b
Move color coding out of strings by using colorize
RenechCDDA May 1, 2023
aabf21a
Correct strings for translation support
RenechCDDA May 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/veh_interact.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,14 @@ bool veh_interact::update_part_requirements()
}
nmsg += res.second;

ret_val<void> can_mount = veh->can_mount(
veh->part( cpart ).mount, sel_vpart_info->get_id() );
Copy link
Contributor

Choose a reason for hiding this comment

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

the main problem comes from here - cpart is only valid some of the time, when on empty square it's probably -1, you can test if veh_interact::dd is the mount point, i can't remember if you need to rotate() it to the correct orientation

looks like adding all parts to req_missing is also misbehaving but that's a smaller problem

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah looks like it's dd.rotate( 2 ). I was so pleased when I saw cpart so I 'could avoid that whole mess'. Shame on me, I suppose.

What's this about adding all parts to req_missing misbehaving?

Copy link
Contributor

Choose a reason for hiding this comment

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

i only checked briefly and not on my pc right now, but some parts had white (installable as opposed to gray) names, but were sorted together with uninstallable ones, if you don't see them in your testing could be just a hiccup, didn't have time to dig into it

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh no, I noticed that right before pushing the PR originally. It's tied to veh_interact::can_potentially_install being used in veh_interact::display_list. I totally ignored that and slammed the PR ahead regardless I gave it some thought over that night and figured it still does provide useful feedback to the player, although it is a slight change from previous behavior. Parts should only be getting colored white if you have all the components to install them, but it's agnostic of whether they're valid to install on that tile (would still be able to tell based on their placement in the list).

if( !can_mount.success() ) {
ok = false;
nmsg += _( "<color_white>Cannot install due to:</color>\n> " ) + colorize( can_mount.str(),
c_red ) + "\n";
}

sel_vpart_info->format_description( nmsg, c_light_gray, getmaxx( w_msg ) - 4 );

msg = colorize( nmsg, c_light_gray );
Expand Down Expand Up @@ -2363,7 +2371,7 @@ void veh_interact::move_cursor( const point &d, int dstart_at )
if( has_critter && vp.has_flag( VPFLAG_OBSTACLE ) ) {
continue;
}
if( veh->can_mount( vd, vp.get_id() ) ) {
if( veh->can_mount( vd, vp.get_id() ).success() ) {
if( vp.has_flag( VPFLAG_APPLIANCE ) ) {
// exclude "appliances" from vehicle part list
continue;
Expand All @@ -2377,6 +2385,8 @@ void veh_interact::move_cursor( const point &d, int dstart_at )
} else {
req_missing.push_back( &vp );
}
} else {
req_missing.push_back( &vp );
}
}
auto vpart_localized_sort = []( const vpart_info * a, const vpart_info * b ) {
Expand Down
47 changes: 30 additions & 17 deletions src/vehicle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1120,33 +1120,36 @@ bool vehicle::is_structural_part_removed() const
* @param id The id of the part to install.
* @return true if the part can be mounted, false if not.
*/
bool vehicle::can_mount( const point &dp, const vpart_id &id ) const
ret_val<void> vehicle::can_mount( const point &dp, const vpart_id &id ) const
{
//The part has to actually exist.
if( !id.is_valid() ) {
return false;
return ret_val<void>::make_failure(
_( "Invalid part ID! This should never appear." ) );
}

//It also has to be a real part, not the null part
const vpart_info &part = id.obj();
if( part.has_flag( "NOINSTALL" ) ) {
return false;
return ret_val<void>::make_failure( _( "Part cannot be installed." ) );
}

const std::vector<int> parts_in_square = parts_at_relative( dp, false, false );

//First part in an empty square MUST be a structural part or be an appliance
if( parts_in_square.empty() && part.location != part_location_structure &&
!part.has_flag( flag_APPLIANCE ) ) {
return false;
return ret_val<void>::make_failure( _( "There's no structure to support it." ) );
}
// If its a part that harnesses animals that don't allow placing on it.
if( !parts_in_square.empty() && part_info( parts_in_square[0] ).has_flag( "ANIMAL_CTRL" ) ) {
return false;
return ret_val<void>::make_failure(
_( "There's an animal harness in the way." ) );
}
//No other part can be placed on a protrusion
if( !parts_in_square.empty() && part_info( parts_in_square[0] ).has_flag( "PROTRUSION" ) ) {
return false;
return ret_val<void>::make_failure(
_( "An existing vehicle part is protruding." ) );
}

//No part type can stack with itself, or any other part in the same slot
Expand All @@ -1156,12 +1159,16 @@ bool vehicle::can_mount( const point &dp, const vpart_id &id ) const
//Parts with no location can stack with each other (but not themselves)
if( part.get_id() == other_part.get_id() ||
( !part.location.empty() && part.location == other_part.location ) ) {
return false;
return ret_val<void>::make_failure(
_( "The installed %1$s part shares the %2$s location." ), other_part.name(),
other_part.location );
}
// Until we have an interface for handling multiple components with CARGO space,
// exclude them from being mounted in the same tile.
if( part.has_flag( "CARGO" ) && other_part.has_flag( "CARGO" ) ) {
return false;
return ret_val<void>::make_failure(
_( "There can't be two cargo parts on the same tile. Part conflicts with existing %1$s." ),
other_part.name() );
}

}
Expand All @@ -1175,14 +1182,16 @@ bool vehicle::can_mount( const point &dp, const vpart_id &id ) const
!has_structural_part( dp + point_south ) &&
!has_structural_part( dp + point_west ) &&
!has_structural_part( dp + point_north ) ) {
return false;
return ret_val<void>::make_failure(
_( "Part needs to be adjacent to or on existing structure." ) );
}
}

// only one exclusive engine allowed
std::string empty;
if( has_engine_conflict( &part, empty ) ) {
return false;
return ret_val<void>::make_failure(
_( "Only one exclusive engine is allowed." ) );
}

// Check all the flags of the part to see if they require other flags
Expand All @@ -1196,7 +1205,8 @@ bool vehicle::can_mount( const point &dp, const vpart_id &id ) const
}
}
if( !anchor_found ) {
return false;
return ret_val<void>::make_failure(
_( "Part requires flag from another part." ) );
}
}
}
Expand All @@ -1205,7 +1215,8 @@ bool vehicle::can_mount( const point &dp, const vpart_id &id ) const
if( part.has_flag( "VISION" ) && !part.has_flag( "CAMERA" ) ) {
for( const int &elem : parts_in_square ) {
if( part_info( elem ).has_flag( "OPAQUE" ) ) {
return false;
return ret_val<void>::make_failure(
_( "Mirrors cannot be mounted on opaque parts." ) );
}
}
}
Expand All @@ -1214,7 +1225,8 @@ bool vehicle::can_mount( const point &dp, const vpart_id &id ) const
for( const int &elem : parts_in_square ) {
if( part_info( elem ).has_flag( "VISION" ) &&
!part_info( elem ).has_flag( "CAMERA" ) ) {
return false;
return ret_val<void>::make_failure(
_( "Opaque parts cannot be mounted on mirror parts." ) );
}
}
}
Expand All @@ -1223,13 +1235,14 @@ bool vehicle::can_mount( const point &dp, const vpart_id &id ) const
if( part.has_flag( "TURRET_MOUNT" ) ) {
for( const int &elem : parts_in_square ) {
if( part_info( elem ).has_flag( "TURRET_MOUNT" ) ) {
return false;
return ret_val<void>::make_failure(
_( "You can't install a turret on a turret." ) );
}
}
}

//Anything not explicitly denied is permitted
return true;
return ret_val<void>::make_success();
}

bool vehicle::can_unmount( const int p ) const
Expand Down Expand Up @@ -1401,7 +1414,7 @@ bool vehicle::is_appliance() const
int vehicle::install_part( const point &dp, const vpart_id &id, const std::string &variant_id,
bool force )
{
if( !( force || can_mount( dp, id ) ) ) {
if( !( force || can_mount( dp, id ).success() ) ) {
return -1;
}
return install_part( dp, vehicle_part( id, variant_id, dp, item( id.obj().base_item ) ) );
Expand All @@ -1410,7 +1423,7 @@ int vehicle::install_part( const point &dp, const vpart_id &id, const std::strin
int vehicle::install_part( const point &dp, const vpart_id &id, item &&obj,
const std::string &variant_id, bool force )
{
if( !( force || can_mount( dp, id ) ) ) {
if( !( force || can_mount( dp, id ).success() ) ) {
return -1;
}
return install_part( dp, vehicle_part( id, variant_id, dp, std::move( obj ) ) );
Expand Down
2 changes: 1 addition & 1 deletion src/vehicle.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,7 @@ class vehicle
const vpart_info &part_info( int index, bool include_removed = false ) const;

// check if certain part can be mounted at certain position (not accounting frame direction)
bool can_mount( const point &dp, const vpart_id &id ) const;
ret_val<void> can_mount( const point &dp, const vpart_id &id ) const;

// check if certain part can be unmounted
bool can_unmount( int p ) const;
Expand Down