Skip to content

Added category filtering for hotkeys in settings window #9957

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

Draft
pazam-h2 wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

Added category filtering for hotkeys in settings window #9957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c24cebb
Added category filtering and UI improvements to hotkey dialog
pazam-h2 Jul 1, 2025
505ce52
fixed Clang formatting and added 'Category:' title
pazam-h2 Jul 2, 2025
93cf960
Fixed code formatting via clang-format
pazam-h2 Jul 2, 2025
9a06ceb
filter changed from buttons to a list on the left side
pazam-h2 Jul 2, 2025
512ce83
Added 'All' to the list of categories (selected by default)
pazam-h2 Jul 3, 2025
2ab9905
saved as utf 8 without signature
pazam-h2 Jul 6, 2025
5eb18f1
removed trailing whitespace
pazam-h2 Jul 6, 2025
6eb3c21
Add missing <set> include for IWYU compliance
pazam-h2 Jul 6, 2025
5f018e3
added 'Category:' subtitle and also made the whole category row click…
pazam-h2 Jul 6, 2025
48ee49d
whitespace removed
pazam-h2 Jul 6, 2025
58aa00c
Merge branch 'master' into pr/9957
ihhub Aug 30, 2025
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
Empty file added -
View file
Open in desktop
Empty file.
75 changes: 71 additions & 4 deletions src/fheroes2/dialog/dialog_hotkeys.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <cassert>
#include <cstdint>
#include <memory>
#include <set>
Copy link
Owner

@ihhub ihhub Jul 28, 2025

Choose a reason for hiding this comment

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

This header won't be needed with the changes proposed below.

#include <string>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -254,20 +255,55 @@ namespace fheroes2
fheroes2::Display & display = fheroes2::Display::instance();

// Dialog height is capped with the current screen height minus 100 pixels.
fheroes2::StandardWindow background( keyDescriptionLength + hotKeyLength + 8 + 75, std::min( display.height() - 100, 520 ), true, display );
const int32_t categoryWidth = 115;
fheroes2::StandardWindow background( categoryWidth + keyDescriptionLength + hotKeyLength + 8 + 95, std::min( display.height() - 100, 520 ), true, display );

const fheroes2::Rect roi( background.activeArea() );
const fheroes2::Rect listRoi( roi.x + 24, roi.y + 37, keyDescriptionLength + hotKeyLength + 8, roi.height - 75 );
const fheroes2::Rect listRoi( roi.x + categoryWidth + 50, roi.y + 50, keyDescriptionLength + hotKeyLength + 8, roi.height - 92 );

const fheroes2::Text title( _( "Hot Keys:" ), fheroes2::FontType::normalYellow() );
title.draw( roi.x + ( roi.width - title.width() ) / 2, roi.y + 16, display );

std::vector<std::pair<Game::HotKeyEvent, Game::HotKeyCategory>> hotKeyEvents = Game::getAllHotKeyEvents();
std::vector<Game::HotKeyCategory> uniqueCategories = { Game::HotKeyCategory::ALL };

std::set<Game::HotKeyCategory> seen;
for ( const auto & pair : hotKeyEvents ) {
if ( seen.find( pair.second ) == seen.end() ) {
seen.insert( pair.second );
uniqueCategories.push_back( pair.second );
}
}
Comment on lines +270 to +276
Copy link
Owner

@ihhub ihhub Jul 28, 2025

Choose a reason for hiding this comment

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

This code isn't needed. We can directly take all entries from the enumeration.


// We divide the list: action description and binded hot-key.
const fheroes2::Rect categoryRoi( roi.x + 15, listRoi.y, categoryWidth + 25, listRoi.height );
background.applyTextBackgroundShading( categoryRoi );
background.applyTextBackgroundShading( { listRoi.x, listRoi.y, keyDescriptionLength + 8, listRoi.height } );
background.applyTextBackgroundShading( { listRoi.x + keyDescriptionLength + 8, listRoi.y, hotKeyLength, listRoi.height } );

const bool isEvilInterface = Settings::Get().isEvilInterfaceEnabled();

std::vector<std::pair<Game::HotKeyCategory, fheroes2::Rect>> categoryRects;

int categoryOffsetY = categoryRoi.y + 4;

Game::HotKeyCategory selectedCategory = Game::HotKeyCategory::ALL;

const fheroes2::Text categoryHeader( _( "Category:" ), fheroes2::FontType::normalWhite() );
categoryHeader.draw( roi.x + 45, roi.y + 32, display );

for ( const Game::HotKeyCategory category : uniqueCategories ) {
Copy link
Owner

@ihhub ihhub Jul 28, 2025

Choose a reason for hiding this comment

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

We don't need uniqueCategories but directly use enumeration entries. Just add one more entry before the loop for _( All ).

const bool isSelected = ( category == selectedCategory );
const fheroes2::FontType fontType = isSelected ? fheroes2::FontType::normalYellow() : fheroes2::FontType::normalWhite();
const fheroes2::Text categoryText( _( Game::getHotKeyCategoryName( category ) ), fontType );
categoryText.draw( categoryRoi.x + 4, categoryOffsetY, display );

const fheroes2::Rect textRect( categoryRoi.x, categoryOffsetY, categoryRoi.width, categoryText.height() + 4 );
categoryRects.emplace_back( category, textRect );

categoryOffsetY += categoryText.height() + 4;
}
Comment on lines +296 to +305
Copy link
Owner

@ihhub ihhub Jul 28, 2025

Choose a reason for hiding this comment

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

Please move this code into a separate function in anonymous namespace and call it here and in the code below to avoid code duplication.


// Prepare OKAY button and render its shadow.
fheroes2::Button buttonOk;
const int buttonOkIcn = isEvilInterface ? ICN::BUTTON_SMALL_OKAY_EVIL : ICN::BUTTON_SMALL_OKAY_GOOD;
Expand All @@ -290,20 +326,51 @@ namespace fheroes2
listbox.setScrollBarImage( fheroes2::AGG::GetICN( listIcnId, 4 ) );
listbox.SetAreaMaxItems( ( listRoi.height - 7 ) / fheroes2::getFontHeight( fheroes2::FontSize::NORMAL ) );

std::vector<std::pair<Game::HotKeyEvent, Game::HotKeyCategory>> hotKeyEvents = Game::getAllHotKeyEvents();

Comment on lines -293 to -294
Copy link
Owner

@ihhub ihhub Jul 28, 2025

Choose a reason for hiding this comment

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

With the above comment we can revert this change since hotKeyEvents is not being used above.

listbox.SetListContent( hotKeyEvents );
listbox.updateScrollBarImage();
listbox.Redraw();

display.render( background.totalArea() );

std::vector<std::pair<Game::HotKeyEvent, Game::HotKeyCategory>> filtered = hotKeyEvents;
Copy link
Owner

@ihhub ihhub Jul 28, 2025

Choose a reason for hiding this comment

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

This variable is not needed especially outside the loop. Its lifetime is within the newly created below if-condition body.


LocalEvent & le = LocalEvent::Get();
while ( le.HandleEvents() ) {
buttonOk.drawOnState( le.isMouseLeftButtonPressedAndHeldInArea( buttonOk.area() ) );

listbox.QueueEventProcessing();

for ( const auto & pair : categoryRects ) {
if ( le.MouseClickLeft( pair.second ) ) {
Copy link
Owner

@ihhub ihhub Jul 28, 2025

Choose a reason for hiding this comment

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

We should update the list only when clicking is done on a different item. Now we rebuild everything without this conditon.

selectedCategory = pair.first;

// ? DO NOT re-declare 'filtered' here
filtered.clear();
for ( const auto & item : hotKeyEvents ) {
if ( selectedCategory == Game::HotKeyCategory::ALL || item.second == selectedCategory )
filtered.push_back( item );
}

categoryOffsetY = categoryRoi.y + 4;
for ( auto & categoryPair : categoryRects ) {
const bool isSelected = ( categoryPair.first == selectedCategory );
const fheroes2::FontType fontType = isSelected ? fheroes2::FontType::normalYellow() : fheroes2::FontType::normalWhite();
const fheroes2::Text categoryText( _( Game::getHotKeyCategoryName( categoryPair.first ) ), fontType );
categoryText.draw( categoryRoi.x + 4, categoryOffsetY, display );

categoryPair.second = { categoryRoi.x, categoryOffsetY, categoryRoi.width, categoryText.height() + 4 };
categoryOffsetY += categoryText.height() + 4;
}
Comment on lines +355 to +363
Copy link
Owner

@ihhub ihhub Jul 28, 2025

Choose a reason for hiding this comment

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

I would like to see how this dialog looks in the original resolution: 640x480 and whether every element fits. Also, please attach screenshots in other languages to see how it looks and whether the dialog is not being crumped for some languages.


listbox.SetListContent( filtered );
listbox.updateScrollBarImage();
listbox.Redraw();
display.render( roi );

break;
}
}

if ( le.MouseClickLeft( buttonOk.area() ) || Game::HotKeyCloseWindow() ) {
return;
}
Expand Down
2 changes: 2 additions & 0 deletions src/fheroes2/game/game_hotkeys.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,8 @@ void Game::globalKeyDownEvent( const fheroes2::Key key, const int32_t modifier )
const char * Game::getHotKeyCategoryName( const HotKeyCategory category )
{
switch ( category ) {
case HotKeyCategory::ALL:
Copy link
Owner

@ihhub ihhub Jul 28, 2025

Choose a reason for hiding this comment

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

Please revert this change. This doesn't belong to the class but only for a single specific dialog. So, let's make changes where they needed - in that dialog.

return gettext_noop( "All" );
case HotKeyCategory::DEFAULT:
return gettext_noop( "Default Actions" );
case HotKeyCategory::GLOBAL:
Expand Down
3 changes: 2 additions & 1 deletion src/fheroes2/game/game_hotkeys.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,9 @@ namespace Game
NO_EVENT,
};

enum class HotKeyCategory : uint8_t
enum class HotKeyCategory : int
{
ALL = -1,
Copy link
Owner

@ihhub ihhub Jul 28, 2025

Choose a reason for hiding this comment

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

Please revert this change. This seem like a hack rather than a solution. Usually, all items in enumeration start from 0 (default value). We call simply call Game::getAllHotKeyEvents(); for that particular entry in the dialog.

DEFAULT,
GLOBAL,
MAIN_MENU,
Expand Down
Loading