Fix unarmed weapon experience gain

[robob27]

Summary

Bugfixes "Using worn unarmed weapons raises unarmed skill"

Purpose of change

Resolves #61682

Note: If you are a mod creator and see this, please suggest changes to the PR to match the specific values you'd like to see on your items. At the time of writing this, the values are extremely WIP and I plan to make corrections to what is there. But if you have the exact values you'd like to see there, please let me know and I'll fix it up.

Describe the solution

Since the attack vector changes made in #53954 using unarmed weapons has not been properly training unarmed skills. This is because those changes removed a change added in #41209 which added a temporary UNARMED_WEAPON flag to items covering your hands when not wielding a weapon.

This first commit of the PR removes all usage of the UNARMED_WEAPON flag in c++, instead checking for arms where needed. This should fix all non mod related items to train unarmed properly.

The second commit removes all uses of the UNARMED_WEAPON flag in JSON, including all mods. Potentially includes some undesirable changes to mod items unless feedback is received for all items, but maybe better than stable with unarmed weapons not training properly.

The third commit removes the flag entirely, probably breaking some third party mods, but this sounds okay based on some discussion in the stable push channel.

Describe alternatives you've considered

Adding back UNARMED_WEAPON to all the unarmed weapons but I don't think that's the move.

Testing

WIP

Additional context

[Maleclypse]

Is this intended to still be in draft or is it ready for review?

[ghost]

Seems to be working properly with a cursory inspection. Checked that unarmed skill is increasing when no weapon is worn and checked that both the weapon's damage type and unarmed skill is gaining xp when wielding a set of skewer knuckles.

[robob27]

Hey sorry had a really busy end to the work week. I did want to take one more pass at the armor values on the mod items. But if it looks good to someone else we can probably mark it as ready for review. I just don't wanna break any items.

…

On Sat., Dec. 10, 2022, 2:30 p.m. intergalacticdata, < ***@***.***> wrote: Seems to be working properly with a cursory inspection. Checked that unarmed skill is increasing when no weapon is worn and checked that both the weapon's damage type and unarmed skill is gaining xp when wielding a set of skewer knuckles. — Reply to this email directly, view it on GitHub <#62578 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQBZ2YDYK33A5XIV5YTCQ3WMTK5PANCNFSM6AAAAAAST4O3LM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[ghost]

I'm now getting some errors that may be be related to this fix. The character in question has a set of skewer knuckles in their inventory, but is not wearing them. He also has less than 1 unarmed skill. I noticed the errors sometime after taking the skewers off. These messages occur during long activities, sleep being the easiest to trigger. They may be occurring more often.

00:19:21.736 ERROR : src/item.cpp:7284 [bool item::has_flag(const flag_id &, bool) const] Attempted to check invalid flag_id UNARMED_WEAPON
00:19:47.469 ERROR : (error message will follow backtrace)
./cataclysm-tiles(debug_write_backtrace(std::ostream&)+0x25) [0x152ba2a]
./cataclysm-tiles(DebugLog(DebugLevel, DebugClass)+0x278) [0x152afb8]
./cataclysm-tiles(realDebugmsg(char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)+0xa0) [0x152a892]
./cataclysm-tiles(void realDebugmsg<std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&>(char const*, char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)+0x5f) [0x1146f53]
./cataclysm-tiles(item::has_flag(string_id<json_flag> const&, bool) const+0x7f) [0x181c093]
./cataclysm-tiles(item::melee_skill() const+0x13) [0x185d9fb]
./cataclysm-tiles(Character::get_hit_weapon(item const&) const+0x16) [0x1bcad4c]
./cataclysm-tiles(item::effective_dps(Character const&, Creature&) const+0x4d) [0x182aa91]
./cataclysm-tiles(item::dps[abi:cxx11](bool, bool, Character const&) const+0x8f) [0x182b3af]
./cataclysm-tiles(item::melee_combat_info(std::vector<iteminfo, std::allocator >&, iteminfo_query const*, int, bool) const+0x42c) [0x1853588]
./cataclysm-tiles(item::info[abi:cxx11](std::vector<iteminfo, std::allocator >&, iteminfo_query const*, int) const+0x2e4) [0x1829bb8]
./cataclysm-tiles(item::info[abi:cxx11](bool, std::vector<iteminfo, std::allocator >&) const+0x28) [0x18298a6]
./cataclysm-tiles(inventory_selector::action_examine(item_location const&)+0x51) [0x180591d]
./cataclysm-tiles(inventory_pick_selector::execute()+0x89) [0x18069d9]
./cataclysm-tiles() [0x172367d]
./cataclysm-tiles(game_menus::inv::gun_to_modify(Character&, item const&)+0xce) [0x1728ff9]
./cataclysm-tiles(iuse::gunmod_attach(Character*, item*, bool, tripoint const&)+0x62) [0x1957413]
./cataclysm-tiles(iuse_function_wrapper::use(Character&, item&, bool, tripoint const&) const+0x16) [0x18ed400]
./cataclysm-tiles(use_function::call(Character&, item&, bool, tripoint const&) const+0xd) [0x195cd7b]
./cataclysm-tiles(itype::invoke(Character&, item&, tripoint const&, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&) const+0x92) [0x1929b46]

Attempting to repeat stack trace using debug symbols…
debug_write_backtrace(std::ostream&)
??:?
DebugLog(DebugLevel, DebugClass)
??:?
realDebugmsg(char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
??:?
void realDebugmsg<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(char const*, char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
??:?
item::has_flag(string_id<json_flag> const&, bool) const
??:?
item::melee_skill() const
??:?
Character::get_hit_weapon(item const&) const
??:?
item::effective_dps(Character const&, Creature&) const
??:?
item::dps[abi:cxx11](bool, bool, Character const&) const
??:?
item::melee_combat_info(std::vector<iteminfo, std::allocator<iteminfo> >&, iteminfo_query const*, int, bool) const
??:?
item::info[abi:cxx11](std::vector<iteminfo, std::allocator<iteminfo> >&, iteminfo_query const*, int) const
??:?
item::info[abi:cxx11](bool, std::vector<iteminfo, std::allocator<iteminfo> >&) const
??:?
inventory_selector::action_examine(item_location const&)
??:?
inventory_pick_selector::execute()
??:?
inv_internal(Character&, inventory_selector_preset const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, item_location)
game_inventory.cpp:?
game_menus::inv::gun_to_modify(Character&, item const&)
??:?
iuse::gunmod_attach(Character*, item*, bool, tripoint const&)
??:?
iuse_function_wrapper::use(Character&, item&, bool, tripoint const&) const
??:?
use_function::call(Character&, item&, bool, tripoint const&) const
??:?
itype::invoke(Character&, item&, tripoint const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const
??:?

Backtrace emission took 1 seconds.
(continued from above) ERROR : src/item.cpp:7284 [bool item::has_flag(const flag_id &, bool) const] Attempted to check invalid flag_id UNARMED_WEAPON
[ Previous repeated 17 times ]

[Maleclypse]

I'm now getting some errors that may be be related to this fix. The character in question has a set of skewer knuckles in their inventory, but is not wearing them. He also has less than 1 unarmed skill. I noticed the errors sometime after taking the skewers off. These messages occur during long activities, sleep being the easiest to trigger. They may be occurring more often.

00:19:21.736 ERROR : src/item.cpp:7284 [bool item::has_flag(const flag_id &, bool) const] Attempted to check invalid flag_id UNARMED_WEAPON 00:19:47.469 ERROR : (error message will follow backtrace) ./cataclysm-tiles(debug_write_backtrace(std::ostream&)+0x25) [0x152ba2a] ./cataclysm-tiles(DebugLog(DebugLevel, DebugClass)+0x278) [0x152afb8] ./cataclysm-tiles(realDebugmsg(char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)+0xa0) [0x152a892] ./cataclysm-tiles(void realDebugmsg<std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&>(char const*, char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)+0x5f) [0x1146f53] ./cataclysm-tiles(item::has_flag(string_id<json_flag> const&, bool) const+0x7f) [0x181c093] ./cataclysm-tiles(item::melee_skill() const+0x13) [0x185d9fb] ./cataclysm-tiles(Character::get_hit_weapon(item const&) const+0x16) [0x1bcad4c] ./cataclysm-tiles(item::effective_dps(Character const&, Creature&) const+0x4d) [0x182aa91] ./cataclysm-tiles(item::dps[abi:cxx11](bool, bool, Character const&) const+0x8f) [0x182b3af] ./cataclysm-tiles(item::melee_combat_info(std::vector<iteminfo, std::allocator >&, iteminfo_query const*, int, bool) const+0x42c) [0x1853588] ./cataclysm-tiles(item::info[abi:cxx11](std::vector<iteminfo, std::allocator >&, iteminfo_query const*, int) const+0x2e4) [0x1829bb8] ./cataclysm-tiles(item::info[abi:cxx11](bool, std::vector<iteminfo, std::allocator >&) const+0x28) [0x18298a6] ./cataclysm-tiles(inventory_selector::action_examine(item_location const&)+0x51) [0x180591d] ./cataclysm-tiles(inventory_pick_selector::execute()+0x89) [0x18069d9] ./cataclysm-tiles() [0x172367d] ./cataclysm-tiles(game_menus::inv::gun_to_modify(Character&, item const&)+0xce) [0x1728ff9] ./cataclysm-tiles(iuse::gunmod_attach(Character*, item*, bool, tripoint const&)+0x62) [0x1957413] ./cataclysm-tiles(iuse_function_wrapper::use(Character&, item&, bool, tripoint const&) const+0x16) [0x18ed400] ./cataclysm-tiles(use_function::call(Character&, item&, bool, tripoint const&) const+0xd) [0x195cd7b] ./cataclysm-tiles(itype::invoke(Character&, item&, tripoint const&, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&) const+0x92) [0x1929b46]

Attempting to repeat stack trace using debug symbols…
debug_write_backtrace(std::ostream&)
??:?
DebugLog(DebugLevel, DebugClass)
??:?
realDebugmsg(char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
??:?
void realDebugmsg<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(char const*, char const*, char const*, char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
??:?
item::has_flag(string_id<json_flag> const&, bool) const
??:?
item::melee_skill() const
??:?
Character::get_hit_weapon(item const&) const
??:?
item::effective_dps(Character const&, Creature&) const
??:?
item::dps[abi:cxx11](bool, bool, Character const&) const
??:?
item::melee_combat_info(std::vector<iteminfo, std::allocator<iteminfo> >&, iteminfo_query const*, int, bool) const
??:?
item::info[abi:cxx11](std::vector<iteminfo, std::allocator<iteminfo> >&, iteminfo_query const*, int) const
??:?
item::info[abi:cxx11](bool, std::vector<iteminfo, std::allocator<iteminfo> >&) const
??:?
inventory_selector::action_examine(item_location const&)
??:?
inventory_pick_selector::execute()
??:?
inv_internal(Character&, inventory_selector_preset const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, item_location)
game_inventory.cpp:?
game_menus::inv::gun_to_modify(Character&, item const&)
??:?
iuse::gunmod_attach(Character*, item*, bool, tripoint const&)
??:?
iuse_function_wrapper::use(Character&, item&, bool, tripoint const&) const
??:?
use_function::call(Character&, item&, bool, tripoint const&) const
??:?
itype::invoke(Character&, item&, tripoint const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const
??:?

Backtrace emission took 1 seconds. (continued from above) ERROR : src/item.cpp:7284 [bool item::has_flag(const flag_id &, bool) const] Attempted to check invalid flag_id UNARMED_WEAPON [ Previous repeated 17 times ]

So you are saying you are getting the errors when you loaded this fix into your world?

[Maleclypse]

Any movement possible on this PR?

[Fris0uman]

Could not trigger the skewer error with a new save or with an old save that got the skewer before the change

[ghost]

So you are saying you are getting the errors when you loaded this fix into your world?

No. When adding this fix, it worked fine. Several days later, on a new character I began seeing the issue while sleeping, crafting etc. I think there may have been some updates to experimental in that time period. I haven't looked at this patch in some time as unarmed experience seems to be working in experimental for most of the weapons I've used recently. Not all, but most.

Scrap knuckles aren't training bash damage currently (before anyone asks).