From 1113a47de67274947d54a3cf6afcbee2e9490a72 Mon Sep 17 00:00:00 2001 From: Maciek Kalisiak Date: Mon, 12 Aug 2024 01:53:08 -0400 Subject: [PATCH 1/4] Improve invlet management for Magiclysm spell list. First, impose sensible order on spell list. Specifically, we sort by 3 dimensions: 1. favorite spells before non-favorite 2. by invlet 3. by spell name Second, this change fixes some issues: - make auto-assigned invlets "permanent" (vs scrambling letters when spells are added or subtracted) - actually properly check if the desired invlet is actually in use (vs only checking against "reserved") Motivation: - addresses desire (and expected behavior in general), such as https://www.reddit.com/r/cataclysmdda/comments/oq2g3s/any_way_to_change_spell_list_order/ - might actually address Issue 50465 - might partially mitigate Issue 50018 --- src/inventory.h | 7 ++++ src/magic.cpp | 98 +++++++++++++++++++++++++++++++------------------ src/magic.h | 4 +- 3 files changed, 73 insertions(+), 36 deletions(-) diff --git a/src/inventory.h b/src/inventory.h index 810d2cf81aeb3..0b48b71cdb61d 100644 --- a/src/inventory.h +++ b/src/inventory.h @@ -55,6 +55,13 @@ class invlet_wrapper : private std::string explicit invlet_wrapper( const char *chars ) : std::string( chars ) { } bool valid( int invlet ) const; + + // Get ordinal number (first, second, third, ...) of invlet. + // Informs sorting order. + int ordinal( int invlet ) const { + return this->find( invlet ); + } + std::string get_allowed_chars() const { return *this; } diff --git a/src/magic.cpp b/src/magic.cpp index ae3846deae82f..1395f4d621049 100644 --- a/src/magic.cpp +++ b/src/magic.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -2345,7 +2346,7 @@ class spellcasting_callback : public uilist_callback void display_spell_info( size_t index ); public: // invlets reserved for special functions - const std::set reserved_invlets{ 'I', '=', '*' }; + static const std::set reserved_invlets; bool casting_ignore; spellcasting_callback( std::vector &spells, @@ -2361,8 +2362,11 @@ class spellcasting_callback : public uilist_callback int invlet = 0; invlet = popup_getkey( _( "Choose a new hotkey for this spell." ) ); if( inv_chars.valid( invlet ) ) { - const bool invlet_set = - get_player_character().magic->set_invlet( known_spells[entnum]->id(), invlet, reserved_invlets ); + std::set used_invlets{ spellcasting_callback::reserved_invlets }; + get_player_character().magic->update_used_invlets( used_invlets ); + const bool invlet_set = get_player_character().magic->set_invlet( + known_spells[entnum]->id(), invlet, used_invlets ); + // TODO: if key already in use, have spells swap invlets? if( !invlet_set ) { popup( _( "Hotkey already used." ) ); } else { @@ -2408,6 +2412,8 @@ class spellcasting_callback : public uilist_callback } }; +const std::set spellcasting_callback::reserved_invlets { 'I', '=', '*' }; + bool spell::casting_time_encumbered( const Character &guy ) const { int encumb = 0; @@ -2695,14 +2701,23 @@ bool known_magic::set_invlet( const spell_id &sp, int invlet, const std::set &used_invlets ) +{ + for( const std::pair &invlet_pair : invlets ) { + used_invlets.emplace( invlet_pair.second ); + } +} + void known_magic::toggle_favorite( const spell_id &sp ) { if( favorites.count( sp ) > 0 ) { @@ -2723,25 +2738,15 @@ int known_magic::get_invlet( const spell_id &sp, std::set &used_invlets ) if( found != invlets.end() ) { return found->second; } - for( const std::pair &invlet_pair : invlets ) { - used_invlets.emplace( invlet_pair.second ); - } - for( int i = 'a'; i <= 'z'; i++ ) { - if( used_invlets.count( i ) == 0 ) { - used_invlets.emplace( i ); - return i; - } - } - for( int i = 'A'; i <= 'Z'; i++ ) { - if( used_invlets.count( i ) == 0 ) { - used_invlets.emplace( i ); - return i; - } - } - for( int i = '!'; i <= '-'; i++ ) { - if( used_invlets.count( i ) == 0 ) { - used_invlets.emplace( i ); - return i; + update_used_invlets( used_invlets ); + // For spells without an invlet, assign first available one. + // Assignment is "sticky" (permanent), to avoid invlets getting scrambled + // when spells are added or subtracted. + // TODO: respect "Auto inventory letters" option? + for( auto &ch : inv_chars.get_allowed_chars() ) { + if( set_invlet( sp, ch, used_invlets ) ) { + used_invlets.emplace( ch ); + return ch; } } return 0; @@ -2749,15 +2754,40 @@ int known_magic::get_invlet( const spell_id &sp, std::set &used_invlets ) int known_magic::select_spell( Character &guy ) { - std::vector known_spells = get_spells(); + std::vector known_spells_sorted = get_spells(); + + std::set used_invlets{ spellcasting_callback::reserved_invlets }; + + // Sort the spell lists by 3 dimensions. + sort( known_spells_sorted.begin(), known_spells_sorted.end(), + [&guy, &used_invlets, this]( spell * left, spell * right ) -> int { + const bool l_fav = guy.magic->is_favorite( left->id() ); + const bool r_fav = guy.magic->is_favorite( right->id() ); + // 1. Favorite spells before non-favorite + if( l_fav != r_fav ) + { + return l_fav > r_fav; + } + const int l_invlet = get_invlet( left->id(), used_invlets ); + const int r_invlet = get_invlet( right->id(), used_invlets ); + // 2. By invlet, if present (but in allowed_chars order; e.g., + // lower-case first) + if( l_invlet != r_invlet ) + { + return inv_chars.ordinal( l_invlet ) < inv_chars.ordinal( r_invlet ); + } + // 3. By spell name + return strcmp( left->name().c_str(), right->name().c_str() ); + } ); uilist spell_menu; spell_menu.desired_bounds = { -1.0, -1.0, std::max( 80, TERMX * 3 / 8 ) *ImGui::CalcTextSize( "X" ).x, - clamp( static_cast( known_spells.size() ), 24, TERMY * 9 / 10 ) *ImGui::GetTextLineHeightWithSpacing(), + clamp( static_cast( known_spells_sorted.size() ), 24, TERMY * 9 / 10 ) *ImGui::GetTextLineHeightWithSpacing(), }; + spell_menu.title = _( "Choose a Spell" ); spell_menu.input_category = "SPELL_MENU"; spell_menu.additional_actions.emplace_back( "CHOOSE_INVLET", translation() ); @@ -2766,13 +2796,13 @@ int known_magic::select_spell( Character &guy ) spell_menu.additional_actions.emplace_back( "SCROLL_DOWN_SPELL_MENU", translation() ); spell_menu.additional_actions.emplace_back( "SCROLL_FAVORITE", translation() ); spell_menu.hilight_disabled = true; - spellcasting_callback cb( known_spells, casting_ignore ); + spellcasting_callback cb( known_spells_sorted, casting_ignore ); spell_menu.callback = &cb; spell_menu.add_category( "all", _( "All" ) ); spell_menu.add_category( "favorites", _( "Favorites" ) ); std::vector> categories; - for( const spell *s : known_spells ) { + for( const spell *s : known_spells_sorted ) { if( s->can_cast( guy ) && ( s->spell_class().is_valid() || s->spell_class() == trait_NONE ) ) { const std::string spell_class_name = s->spell_class() == trait_NONE ? _( "Classless" ) : s->spell_class().obj().name(); @@ -2789,16 +2819,16 @@ int known_magic::select_spell( Character &guy ) spell_menu.add_category( cat.first, cat.second ); } - spell_menu.set_category_filter( [&guy, known_spells]( const uilist_entry & entry, + spell_menu.set_category_filter( [&guy, known_spells_sorted]( const uilist_entry & entry, const std::string & key )->bool { if( key == "all" ) { return true; } else if( key == "favorites" ) { - return guy.magic->is_favorite( known_spells[entry.retval]->id() ); + return guy.magic->is_favorite( known_spells_sorted[entry.retval]->id() ); } - return ( known_spells[entry.retval]->spell_class().is_valid() || known_spells[entry.retval]->spell_class() == trait_NONE ) && known_spells[entry.retval]->spell_class().str() == key; + return ( known_spells_sorted[entry.retval]->spell_class().is_valid() || known_spells_sorted[entry.retval]->spell_class() == trait_NONE ) && known_spells_sorted[entry.retval]->spell_class().str() == key; } ); if( !favorites.empty() ) { spell_menu.set_category( "favorites" ); @@ -2806,13 +2836,11 @@ int known_magic::select_spell( Character &guy ) spell_menu.set_category( "all" ); } - std::set used_invlets{ cb.reserved_invlets }; - - for( size_t i = 0; i < known_spells.size(); i++ ) { - spell_menu.addentry( static_cast( i ), known_spells[i]->can_cast( guy ), - get_invlet( known_spells[i]->id(), used_invlets ), known_spells[i]->name() ); + for( size_t i = 0; i < known_spells_sorted.size(); i++ ) { + spell_menu.addentry( static_cast( i ), known_spells_sorted[i]->can_cast( guy ), + get_invlet( known_spells_sorted[i]->id(), used_invlets ), known_spells_sorted[i]->name() ); } - reflesh_favorite( &spell_menu, known_spells ); + reflesh_favorite( &spell_menu, known_spells_sorted ); spell_menu.query( true, -1, true ); diff --git a/src/magic.h b/src/magic.h index 89cbbdf476ae8..9226990415773 100644 --- a/src/magic.h +++ b/src/magic.h @@ -719,13 +719,15 @@ class known_magic // returns false if invlet is already used bool set_invlet( const spell_id &sp, int invlet, const std::set &used_invlets ); void rem_invlet( const spell_id &sp ); + // returns which invlets are already in use + void update_used_invlets( std::set &used_invlets ); void toggle_favorite( const spell_id &sp ); bool is_favorite( const spell_id &sp ); private: // gets length of longest spell name int get_spellname_max_width(); - // gets invlet if assigned, or -1 if not + // gets invlet if assigned, or 0 if not int get_invlet( const spell_id &sp, std::set &used_invlets ); }; From 5c9b84868d9309b26437d525c29e317ef419737b Mon Sep 17 00:00:00 2001 From: Maciek Date: Mon, 12 Aug 2024 19:33:17 -0400 Subject: [PATCH 2/4] Update magic.cpp - avoid gratuitous 'auto' use. --- src/magic.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/magic.cpp b/src/magic.cpp index 1395f4d621049..c5bfd20011c5b 100644 --- a/src/magic.cpp +++ b/src/magic.cpp @@ -2743,7 +2743,7 @@ int known_magic::get_invlet( const spell_id &sp, std::set &used_invlets ) // Assignment is "sticky" (permanent), to avoid invlets getting scrambled // when spells are added or subtracted. // TODO: respect "Auto inventory letters" option? - for( auto &ch : inv_chars.get_allowed_chars() ) { + for( int &ch : inv_chars.get_allowed_chars() ) { if( set_invlet( sp, ch, used_invlets ) ) { used_invlets.emplace( ch ); return ch; From 15da592f07d2d1dc6171ab1d6567222e210103c5 Mon Sep 17 00:00:00 2001 From: Maciek Kalisiak Date: Tue, 13 Aug 2024 09:37:51 -0400 Subject: [PATCH 3/4] Correct the type on previous fix of for() loop. It's weird, as Clang didn't complain about it when I built it locally, but clearly it's complaining on the GitHub continuous builds. I guess I was relying on an implicit conversion, and favoured "int" representation of everything... --- src/magic.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/magic.cpp b/src/magic.cpp index c5bfd20011c5b..4e8d93b2cd07a 100644 --- a/src/magic.cpp +++ b/src/magic.cpp @@ -2743,10 +2743,11 @@ int known_magic::get_invlet( const spell_id &sp, std::set &used_invlets ) // Assignment is "sticky" (permanent), to avoid invlets getting scrambled // when spells are added or subtracted. // TODO: respect "Auto inventory letters" option? - for( int &ch : inv_chars.get_allowed_chars() ) { - if( set_invlet( sp, ch, used_invlets ) ) { - used_invlets.emplace( ch ); - return ch; + for( char &ch : inv_chars.get_allowed_chars() ) { + int invlet = static_cast( ch ); + if( set_invlet( sp, invlet, used_invlets ) ) { + used_invlets.emplace( invlet ); + return invlet; } } return 0; From b794dc5b6c53251638a08ca3fa000844c21d84f5 Mon Sep 17 00:00:00 2001 From: Maciek Kalisiak Date: Mon, 26 Aug 2024 23:59:21 -0400 Subject: [PATCH 4/4] Introduce additional cast, to mitigate a compiler complaint. --- src/magic.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/magic.cpp b/src/magic.cpp index 4e8d93b2cd07a..9d5b8900d3117 100644 --- a/src/magic.cpp +++ b/src/magic.cpp @@ -2744,7 +2744,7 @@ int known_magic::get_invlet( const spell_id &sp, std::set &used_invlets ) // when spells are added or subtracted. // TODO: respect "Auto inventory letters" option? for( char &ch : inv_chars.get_allowed_chars() ) { - int invlet = static_cast( ch ); + int invlet = static_cast( static_cast( ch ) ); if( set_invlet( sp, invlet, used_invlets ) ) { used_invlets.emplace( invlet ); return invlet;