-
Notifications
You must be signed in to change notification settings - Fork 287
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.
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
feat: Martial Arts Expansion #6054
base: main
Are you sure you want to change the base?
Conversation
Initial commit. Also, Added knockback_follow_full, strictly_running, consume_buffs (which isn't fully functional yet), annd switch_pos (also renamed side_switch to switch_side). Started work on reach attacks.
Autofix has formatted code style violation in this PR. I edit commits locally (e.g: git, github desktop) and want to keep autofix
I do not want the automated commit
If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT. |
This reverts commit 5b6f75d.
… into MA-Changes
…r. method Added: req_running req_half_wall switch_side_self switch_pos reach_tec reach_ok pull_target pull_self consumes_buffs req_buffs req_target_effects Updated Names for: side_switch -> switch_side_target stealthy -> quiet_movement quiet -> quiet_attacks powerful_knockback -> knockback_type knockback_follow -> knockback_follow_type
@@ -534,10 +599,24 @@ bool ma_requirements::is_valid_character( const Character &u ) const | |||
} | |||
} | |||
|
|||
if( wall_adjacent && !get_map().is_wall_adjacent( u.pos() ) ) { | |||
if( req_running && !u.movement_mode_is( CMM_RUN ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mixed feelings on running requirement.
On one hand, it allows mode switching, which is pretty powerful - same technique could switch between, say, offensive or defensive, charge forward or stay rooted etc.
On the other hand, it allows somewhat clunky, tedious mode switching.
I'm not saying no, it's just that there may be a better way to do mode switching - something that doesn't tie it to an unrelated mode option. Tying it to movement mode feels like a workaround (for lack of free keybinds) rather than "the right thing".
Still, if no better solution is found, this is good enough.
src/martialarts.cpp
Outdated
dump += _( "* Moves target <info>behind</info> you" ) + std::string( "\n" ); | ||
} | ||
|
||
if( wall_adjacent ) { | ||
if( switch_pos ) { | ||
dump += _( "* switchs positions with the target" ) + std::string( "\n" ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Switches"
t.setpos( dest ); | ||
} else { | ||
if( in_vehicle ) { //Don't unboard unless it's the player, and unless there is actually a place to swap to. | ||
get_map().unboard_vehicle( pos() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need robust testing, boarding/unboarding code is fragile as fuck when combined with dragged vehicles.
Be sure to test at least the following cases:
- Player is dragging a cart, zombie stands on cart, player swaps with zombie, player moves off cart
- As above, but player is standing on cart2
} | ||
} | ||
|
||
if( technique.stun_dur > 0 && !technique.powerful_knockback ) { | ||
if( technique.stun_dur > 0 && technique.knockback_type.empty() ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using std::optional
would be better than testing for empty string.
src/melee.cpp
Outdated
|
||
for( const mabuff_id &consume_id : technique.reqs.consume_buffs ) { | ||
if( has_mabuff( consume_id ) ) { | ||
remove_effect( efftype_id( std::string( "mabuff:" ) + consume_id.str() ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Evil string manipulation.
In order to limit the evilness, move that to load/finalize time. This will enable verification if the mabuff
exists on load time, rather than only during runtime.
…tuff -consumed_buffs and required_buffs are now vector<pair<mabuff_id, int>> -added new effect removal methods that allow for effects to have part of the intensity reduced externally. Not happy with the location of these methods and am going to change it. -added more safety rails for existing fields that will soon be deprecated.
… into MA-Changes
Purpose of change (The Why)
The current martial arts system is very clunky/coarse in its approximation of a state machine, while also being quite limited in what it allows martial arts to do. It should be updated to fix both of these problems so we can make existing stuff cleaner, and make new stuff cooler.
(TODO: Look for related issues and add them. I'm sure there are plenty)
Describe the solution (The How)
This PR aims to greatly expand the JSON options available when defining a Martial Art, both to clean up/streamline existing logic, and to allow for more interesting/diverse patterns & mechanics. Most of the changes I am proposing either have an obvious intended use, or are an expansion of existing functionality.
The full list of planned changes can be found on this spreadsheet, but I will be attaching gifs and examples of each field below.
Also, TBD but I would really like to clean up the in-game martial arts description. It is VERY difficult to parse, and now seems like a good time since I will have to add entries for all of these field anyway.
Describe alternatives you've considered
While limited, I think the martial arts system is also relatively focused right now, and I've tried to preserve that by not expanding it too much. Changes I considered but decided against include:
"req_buffs": [ "buff_example_onhit" ]
->"req_effects": [ "buff_example_onhit", "grabbed" ]
"downed_target": true
->"req_effects_target": [ "downed" ]
on_hit_you
effects weapons can trigger.Testing
All of these effects will be tested. Gifs and JSON examples incoming.
Additional context
Ideally this should not remove any functionality, from the game. There is an implied follow up task where existing clunky martial arts are rewritten using better logic enabled by this pr, but that is not included.
Checklist
Mandatory
closes #1234
in Summary of the PR so it can be closed automatically.main
so it won't cause conflict when updatingmain
branch later.Optional
doc/
folder.lang/bn_extract_json_strings.sh
script if it does not support them yet.-->