Skip to content

Commit

Permalink
Fixed check of option prerequisites (#27735)
Browse files Browse the repository at this point in the history 
* Fixed check of option prerequisites
* Fix Android builds
  • Loading branch information
@ZhilkinSerg @kevingranade
ZhilkinSerg authored and kevingranade committed Jan 24, 2019
1 parent 3d707cd commit be1b069
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 25 deletions.
52 changes: 28 additions & 24 deletions src/options.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,6 @@ void options_manager::add( const std::string &sNameIn, const std::string &sPageI
void options_manager::cOpt::setPrerequisites( const std::string &sOption,
const std::vector<std::string> &sAllowedValues )
{
static const std::vector<std::string> &sSupportedTypes = { "bool", "string", "string_select", "string_input" };
const bool hasOption = get_options().has_option( sOption );
if( !hasOption ) {
debugmsg( "setPrerequisite: unknown option %s", sType.c_str() );
Expand All @@ -369,29 +368,20 @@ void options_manager::cOpt::setPrerequisites( const std::string &sOption,
const cOpt &existingOption = get_options().get_option( sOption );
const std::string &existingOptionType = existingOption.getType();
bool isOfSupportType = false;
for( const std::string &sSupportedType : sSupportedTypes ) {
for( const std::string &sSupportedType : getPrerequisiteSupportedTypes() ) {
if( existingOptionType == sSupportedType ) {
isOfSupportType = true;
break;
}
}

if( !isOfSupportType ) {
debugmsg( "setPrerequisite: option %s not of type bool", sType.c_str() );
debugmsg( "setPrerequisite: option %s not of supported type", sType.c_str() );
return;
}

if( !sAllowedValues.empty() ) {
const std::string &existingOptionValue = existingOption.getValue();
for( const std::string &sAllowedValue : sAllowedValues ) {
if( existingOptionValue == sAllowedValue ) {
sPrerequisite = sOption;
break;
}
}
} else {
sPrerequisite = sOption;
}
sPrerequisite = sOption;
sPrerequisiteAllowedValues = sAllowedValues;
}

std::string options_manager::cOpt::getPrerequisite() const
Expand All @@ -404,6 +394,22 @@ bool options_manager::cOpt::hasPrerequisite() const
return !sPrerequisite.empty();
}

bool options_manager::cOpt::checkPrerequisite() const
{
if( !hasPrerequisite() ) {
return true;
}
bool isPrerequisiteFulfilled = false;
const std::string prerequisite_option_value = get_options().get_option( sPrerequisite ).getValue();
for( const std::string &sAllowedPrerequisiteValue : sPrerequisiteAllowedValues ) {
if( prerequisite_option_value == sAllowedPrerequisiteValue ) {
isPrerequisiteFulfilled = true;
break;
}
}
return isPrerequisiteFulfilled;
}

//helper functions
bool options_manager::cOpt::is_hidden() const
{
Expand Down Expand Up @@ -1554,9 +1560,9 @@ void options_manager::add_options_graphics()
#else
add( "SOFTWARE_RENDERING", "graphics", translate_marker( "Software rendering" ),
translate_marker( "Use software renderer instead of graphics card acceleration. Requires restart." ),
// take default setting from pre-game settings screen - important as both software + hardware rendering have issues with specific devices
android_get_default_setting( "Software rendering", false ),
COPT_CURSES_HIDE // take default setting from pre-game settings screen - important as both software + hardware rendering have issues with specific devices
false, COPT_CURSES_HIDE
COPT_CURSES_HIDE
);
#endif

Expand Down Expand Up @@ -2227,9 +2233,8 @@ std::string options_manager::show( bool ingame, const bool world_options_only )

nc_color cLineColor = c_light_green;
const cOpt &current_opt = cOPTIONS[mPageItems[iCurrentPage][i]];
bool hasPrerequisite = current_opt.hasPrerequisite();
bool prerequisiteEnabled = !hasPrerequisite ||
cOPTIONS[ current_opt.getPrerequisite() ].value_as<bool>();
const bool hasPrerequisite = current_opt.hasPrerequisite();
const bool hasPrerequisiteFulfilled = current_opt.checkPrerequisite();

int line_pos = i - iStartPos; // Current line position in window.

Expand All @@ -2245,9 +2250,9 @@ std::string options_manager::show( bool ingame, const bool world_options_only )

const std::string name = utf8_truncate( current_opt.getMenuText(), name_width );
mvwprintz( w_options, line_pos, name_col + 3, !hasPrerequisite ||
prerequisiteEnabled ? c_white : c_light_gray, name );
hasPrerequisiteFulfilled ? c_white : c_light_gray, name );

if( hasPrerequisite && !prerequisiteEnabled ) {
if( hasPrerequisite && !hasPrerequisiteFulfilled ) {
cLineColor = c_light_gray;

} else if( current_opt.getValue() == "false" ) {
Expand Down Expand Up @@ -2346,10 +2351,9 @@ std::string options_manager::show( bool ingame, const bool world_options_only )

cOpt &current_opt = cOPTIONS[mPageItems[iCurrentPage][iCurrentLine]];
bool hasPrerequisite = current_opt.hasPrerequisite();
bool prerequisiteEnabled = !hasPrerequisite ||
cOPTIONS[ current_opt.getPrerequisite() ].value_as<bool>();
bool hasPrerequisiteFulfilled = current_opt.checkPrerequisite();

if( hasPrerequisite && !prerequisiteEnabled &&
if( hasPrerequisite && !hasPrerequisiteFulfilled &&
( action == "RIGHT" || action == "LEFT" || action == "CONFIRM" ) ) {
popup( _( "Prerequisite for this option not met!\n(%s)" ),
get_options().get_option( current_opt.getPrerequisite() ).getMenuText() );
Expand Down
8 changes: 7 additions & 1 deletion src/options.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,17 @@ class options_manager
return !operator==( rhs );
}

static std::vector<std::string> getPrerequisiteSupportedTypes() {
return { "bool", "string", "string_select", "string_input" };
};

void setPrerequisites( const std::string &sOption, const std::vector<std::string> &sAllowedValues );
void setPrerequisite( const std::string &sOption, const std::string &sAllowedValue = "" ) {
void setPrerequisite( const std::string &sOption, const std::string &sAllowedValue = "true" ) {
setPrerequisites( sOption, { sAllowedValue } );
}
std::string getPrerequisite() const;
bool hasPrerequisite() const;
bool checkPrerequisite() const;

enum COPT_VALUE_TYPE {
CVT_UNKNOWN = 0,
Expand All @@ -140,6 +145,7 @@ class options_manager
std::string format;

std::string sPrerequisite;
std::vector<std::string> sPrerequisiteAllowedValues;

copt_hide_t hide;
int iSortPos;
Expand Down

2 comments on commit be1b069

@a1studmuffin
Copy link
Contributor

@a1studmuffin a1studmuffin commented on be1b069 Jan 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@ZhilkinSerg This commit appears to fix the Android compile errors we were seeing, but I've tried building and running the Android version with this change and now I'm getting an abort signal on Android startup (once you get past the "Z" Java UI and into the actual game). The root of the issue seems to sdltiles.cpp:402, which was changed in d146c85:

bool software_renderer = get_option<std::string>( "RENDERER" ).empty();

When that line executes, options_manager::get_option() calls debugmsg("requested non-existing option RENDERER") which triggers the program to abort.

@ZhilkinSerg
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I believe I need to add some guards for Android in sdltiles.cpp not to check this RENDERER option and expose renderer selection option to Java UI (that was my initial plan which I didn't follow as I've temporary lost access to Android build machine).

Please sign in to comment.