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

Don't add duplicate options to the page #37548

Merged
merged 3 commits into from
Feb 11, 2020
Merged

Don't add duplicate options to the page #37548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ad1fca4
Dynamically build display list for options menu
ZhilkinSerg Feb 1, 2020
3a1dcc8
Don't add duplicate options to the options page
ZhilkinSerg Feb 1, 2020
352d6d2
Guard DISPLAY option for TILES build
ZhilkinSerg Feb 2, 2020
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
15 changes: 15 additions & 0 deletions src/cata_tiles.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3605,4 +3605,19 @@ std::vector<options_manager::id_and_option> cata_tiles::build_renderer_list()
return renderer_names.empty() ? default_renderer_names : renderer_names;
}

std::vector<options_manager::id_and_option> cata_tiles::build_display_list()
{
std::vector<options_manager::id_and_option> display_names;
std::vector<options_manager::id_and_option> default_display_names = {
{ "0", translate_marker( "Display 0" ) }
};

int numdisplays = SDL_GetNumVideoDisplays();
for( int i = 0 ; i < numdisplays ; i++ ) {
display_names.emplace_back( std::to_string( i ), std::string( SDL_GetDisplayName( i ) ) );
}

return display_names.empty() ? default_display_names : display_names;
}

#endif // SDL_TILES
1 change: 1 addition & 0 deletions src/cata_tiles.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ class cata_tiles
void do_tile_loading_report();
point player_to_screen( const point & ) const;
static std::vector<options_manager::id_and_option> build_renderer_list();
static std::vector<options_manager::id_and_option> build_display_list();
protected:
template <typename maptype>
void tile_loading_report( const maptype &tiletypemap, const std::string &label,
Expand Down
14 changes: 12 additions & 2 deletions src/options.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,14 @@ void options_manager::addOptionToPage( const std::string &name, const std::strin
{
for( Page &p : pages_ ) {
if( p.id_ == page ) {
// Don't add duplicate options to the page
for( const cata::optional<std::string> &i : p.items_ ) {
if( i.has_value() && i.value() == name ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't duplicates be handled in the calling code? I mean something tries to add duplicate entries and that's weird.

Copy link
Contributor Author

@ZhilkinSerg ZhilkinSerg Jan 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Well, yes, DISPLAY option is added both through find_videodisplays and add_graphic_options.

We can probably go with RENDERER option way as an alternative.

return;
}
}
p.items_.emplace_back( name );
return;
}
}
// @TODO handle the case when an option has no valid page id (note: consider hidden external options as well)
Expand Down Expand Up @@ -1799,10 +1806,13 @@ void options_manager::add_options_graphics()

add_empty_line();

#if defined(TILES)
std::vector<options_manager::id_and_option> display_list = cata_tiles::build_display_list();
add( "DISPLAY", "graphics", translate_marker( "Display" ),
translate_marker( "Sets which video display will be used to show the game. Requires restart." ),
0, 10000, 0, COPT_CURSES_HIDE
);
display_list,
display_list.front().first, COPT_CURSES_HIDE );
#endif

#if !defined(__ANDROID__) // Android is always fullscreen
add( "FULLSCREEN", "graphics", translate_marker( "Fullscreen" ),
Expand Down
21 changes: 1 addition & 20 deletions src/sdltiles.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ static void WinCreate()
}
#endif

int display = get_option<int>( "DISPLAY" );
int display = std::stoi( get_option<std::string>( "DISPLAY" ) );
if( display < 0 || display >= SDL_GetNumVideoDisplays() ) {
display = 0;
}
Expand Down Expand Up @@ -925,23 +925,6 @@ void set_displaybuffer_rendertarget()
SetRenderTarget( renderer, display_buffer );
}

// Populate a map with the available video displays and their name
static void find_videodisplays()
{
std::vector< std::tuple<int, std::string> > displays;

int numdisplays = SDL_GetNumVideoDisplays();
for( int i = 0 ; i < numdisplays ; i++ ) {
displays.push_back( std::make_tuple( i, std::string( SDL_GetDisplayName( i ) ) ) );
}

int current_display = get_option<int>( "DISPLAY" );
get_options().add( "DISPLAY", "graphics", translate_marker( "Display" ),
translate_marker( "Sets which video display will be used to show the game. Requires restart." ),
displays, current_display, 0, options_manager::COPT_CURSES_HIDE, true
);
}

// line_id is one of the LINE_*_C constants
// FG is a curses color
void Font::draw_ascii_lines( unsigned char line_id, int drawx, int drawy, int FG ) const
Expand Down Expand Up @@ -3528,8 +3511,6 @@ void catacurses::init_interface()

InitSDL();

find_videodisplays();

init_term_size_and_scaling_factor();

WinCreate();
Expand Down