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.

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

Medieval Swordsmanship Rewrite #50073

Conversation

QuintusAquila
Copy link
Contributor

@QuintusAquila QuintusAquila commented Jul 19, 2021

Summary

Content "Rewrite Medieval Swordsmanship"

Purpose of change

Currently the techniques used by Medieval Swordsmanship don't correlate to anything in German or Italian Swordsmanship. In particular, the code described the sweeping attack as an undercut, which isn't a cut that targets the legs, and cuts to the leg are somewhat niche and not leg sweeps. Vicious Strike is described as a wrath cut, which isn't wrong, but it's also not a particularly forceful cut.

It also suffers from balance issues. Counterattacks rarely trigger and the blunt damage techniques are worse than regular attacks, in particular mordhau has a move cost penalty.

Describe the solution

The majority of the old techniques were removed. Mordhau didn't work terribly well within the system even if it was one of the techniques that actually exists. Deflection and manslayer were removed. Deflection could somewhat correspond to something but I want to change how the setup works. The grab break was removed because most grappling techniques in Hema are focused around killing the opponent not breaking their grab. While you spend time getting away they spend time stabbing you. Pommel Strike was removed, because the technique hurt the style when it wasn't free and the concept of punishing an attack out of measure could be used as a dodge counter, I'd prefer to focus the style on blocking since dodging isn't a focus in german, or italian swordsmanship.

A feint was added in the form of Flow Drills. Flow drills are a series of consecutive cuts to help practice flowing from one strike to another so they're decently represented by the feint mechanic. The Sweep was reflavored into a grapple, representing ringen. This sort of grappling frequently uses the sword as a lever, so it retains full damage. Mastercut was added as a block counter with a move cost reduction to represent the block being part of the strikes move. Conserve Momentum is meant to represent the general concept of conserving the momentum of the sword as well as quick follow up cuts to take advantage of breaking the opponent's structure. Part of this is using momentum gained in a parry, but to keep from overlapping with master cut its limited to attacks. To avoid overlap with flow drills its restricted to successful attacks. To make it useable with the slower zweihander its given a two turn duration.

Name Level Available Type Effect
Swordsman's Stance Melee 0 Static Buff +1 Block, Blocked Damage reduced by 50% Strength
Flow Drills Melee 1 Melee Tech Feint
Conserve Momentum Melee 2 On Hit Buff 80% Move cost, Duration:2
Half Swording Melee 3 On Pause Buff -1 Accuracy, Blocked damage reduced by +50% of Strength, Duration: 2
Lethal Strike Melee 3 Melee Tech Requires Halfswording, 150% Str Armor Pen, 0% bash, can crit
Grab Melee 4 Melee Crit Tech Requires Halfswording, Down Duration: 2, 50% cut, 33% pierce damage
Mastercut Melee 5 Melee Tech Block Counter, 70% move cost

Strengths
Lethal Strike will help overcome high armor enemies.
It should still perform somewhat well against groups, even lacking grab break, because of the stronger block and move cost buff
Uses Swords

Weaknesses
Still requires some setup. Lethal Strike is stuck with an accuracy penalty.
The lack of a grab break and slowish weapons makes it weak against larger groups.
Uses Swords

I also added the cavalry sabre and cutlass to the style. Dusacks and messers are part of germans swordsmanship so I see no reason that sabres and cutlasses should be excluded.

Describe alternatives you've considered

Leaving it as a reflavor or splitting it into two separate German and Italian martial arts.
Adding an excludes buff requirement so prevent using mastercut and feint in halfswording, but it already has -1 accuracy which is bad enough and dealing with compiling/c++ is currently beyond me.

Testing

I started up the Game with a test character with the martial art and a sword and hit things till the techniques fired. I also checked in the martial arts menu the the new names were right and the that the buffs had the correct description.

I did some play testing to make sure everything is reasonable.

Additional context

@actual-nh actual-nh added <Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON Melee Melee weapons, tactics, techniques, reach attack labels Jul 19, 2021
@actual-nh actual-nh requested a review from mlangsdorf July 19, 2021 22:56
@actual-nh
Copy link
Contributor

Ping: @Hymore246?

@Broken27
Copy link
Contributor

Broken27 commented Jul 19, 2021

A rewrite of medieval swordmanship would be fine honestly, it's very bad currently - counters that do no damage and crits that reduce damage* aren't what i want from an style lol (using Brawling with an sword is honestly better).

*(Mordhau is +200% bashing damage, 0% cut damage, which is a rather strange technique for an style that uses bladed weapons, although admitedly it allows to cause brutal damage if you have hidraulic muscles).

@QuintusAquila
Copy link
Contributor Author

That's a great example for why I didn't touch balance. It didn't jump out to me as a low damage attack.

@Rabidweasel
Copy link

A rewrite of medieval swordmanship would be fine honestly, it's very bad currently - counters that do no damage and crits that reduce damage* aren't what i want from an style lol (using Brawling with an sword is honestly better).

*(Mordhau is +200% brawling damage, 0% cut damage, which is a rather strange technique for an style that uses bladed weapons, although admitedly it allows to cause brutal damage if you have hidraulic muscles).

Agreed, Medieval Swordsmanship is quite weak compared to the other weapon arts, especially since the rework to counter attacks making them no longer "free actions". I think a rework would be a good thing, it's quite boring in its current form (though it's also true that the overall martial art balance got upset a lot by that change and they could mostly use a balance pass anyway)

@pehamm
Copy link
Contributor

pehamm commented Jul 20, 2021

...but due to a lack of experience with the game, I don't really want to make any balance changes.

Please do not worry about balance, make it realistic and if it becomes too strong, this can be tackled later by making monsters stronger or giving them counters.

I am sure everyone here appreciates you lending your expertise to make this more grounded in reality. Do not worry about breaking stuff, if you are unsure about anything, the folks over at the dev discord are more than happy to help.

@QuintusAquila
Copy link
Contributor Author

I'm glad to hear that. If I were to do a rewrite, would it make more sense to close this now and then do it, or leave it and then make another pull request after this merges or is otherwise resolved?

Or I guess a third option of retooling this into the rewrite?

@actual-nh
Copy link
Contributor

Or I guess a third option of retooling this into the rewrite?

That's generally preferable. If the commits get too many it's possible to "squash" them into fewer, although I've never done it myself.

@QuintusAquila
Copy link
Contributor Author

Or I guess a third option of retooling this into the rewrite?

That's generally preferable. If the commits get too many it's possible to "squash" them into fewer, although I've never done it myself.

Makes sense, I guess I'll start on it here then. Does that mean this needs a draft tag?

@QuintusAquila QuintusAquila changed the title Medieval swordsmanship reflavor Medieval swordsmanship rewrite Jul 21, 2021
@QuintusAquila QuintusAquila changed the title Medieval swordsmanship rewrite Medieval Swordsmanship Rewrite Jul 21, 2021
@QuintusAquila
Copy link
Contributor Author

Does this look reasonable? I tried to keep it in line with the description of Italian and German sources, with armoured and unarmoured techniques.

@Broken27
Copy link
Contributor

Broken27 commented Jul 21, 2021

Does this look reasonable? I tried to keep it in line with the description of Italian and German sources, with armoured and unarmoured techniques.

-Checks changes-

An on-pause weak buff that allows to use an Arpen technique - that's pretty good. Maybe increase the arpen - 0.5 of dex is a bit low for a technique that requires set-up work (And reduces accurancy). Niten stance gives 0.5 per arpen without needing a setup.

A miss recovery, that's good
A minimun bash damage requirement and faster speed for Mordhau, good
A counter that actually causes damage, that's good too - although maybe change the speed multiplier to 0.5, at 0.75 is barely worth it.

Yeah it looks fine to me overall.

Anyway, i recommend than you edit the PR abstract to show the changes there, without needing to read the changed files recently.

You can check how Hymore styled it when he did his martial art rebalance, for example in #33210 or #34017

@Rabidweasel
Copy link

Rabidweasel commented Jul 21, 2021

Does this look reasonable? I tried to keep it in line with the description of Italian and German sources, with armoured and unarmoured techniques.

From a pure gameplay angle the loss of grab break feels bad and somewhat weakens the style at something which it was previously relatively good at (dealing with medium-sized groups of enemies). On the other hand it also serves to differentiate it from Fior Di Battaglia which is otherwise quite similar in a lot of ways.

The buff to Mordhau makes it more likely to actually be useful when it procs, which is nice, and it's cool that you can effectively choose to disable this technique by not pausing.

I can see what you're going for with the halfswording buff giving an accuracy penalty, but it also makes it somewhat weak for an onpause buff, though the long duration does ameliorate this a bit. It would also be ideal if the buff description text told you which techniques it activates.

It's also worth noting that as per #49485 armour penetration bonuses from martial arts may not be working properly - though perhaps as an individual technique field this code is working fine? There's also already a technique called precise strike - it might be better to rename if possible.

@Broken27
Copy link
Contributor

Broken27 commented Jul 21, 2021

It's also worth noting that as per #49485 armour penetration bonuses from martial arts may not be working properly - though perhaps as an individual technique field this code is working fine? There's also already a technique called precise strike - it might be better to rename if possible.

I reported #49485 - the technique here will work fine.

Arpen from specific attack techniques isn't bugged, only arpen from buffs is.

@Broken27
Copy link
Contributor

Broken27 commented Jul 21, 2021

@QuintusAquila, you are failing the json style check - meaning you have made some minor format error somewhere

Use the cdda linting tool to autocorrect the changed files to the proper format.

@QuintusAquila
Copy link
Contributor Author

Am I good to leave it and wait for a review now?

@emptytriangle
Copy link

emptytriangle commented Jul 22, 2021

regarding realism - is there a zombie against which the Mordhau technique would be effective? Like, is there a use case for that? I can't really think of one, there has to be some other technique you can use for that one. I don't wanna go around shitting on your idea but like that's a specific technique for punching through sheet metal armor right?

@QuintusAquila
Copy link
Contributor Author

QuintusAquila commented Jul 22, 2021

regarding realism - is there a zombie against which the Mordhau technique would be effective? Like, is there a use case for that? I can't really think of one, there has to be some other technique you can use for that one. I don't wanna go around shitting on your idea but like that's a specific technique for punching through sheet metal armor right?

Most of the swords have a 3:1 cut to bash, , which I originally assumed meant the 300% buff would get the bash damage equal to the cut damage, but it would actually take 400%. In theory it should be good against soldier zombies and the like, since they have much more cut than bash armor, but the Lethal strike technique cancels out most of the difference, so in hindsight, there isn't really a great usecase.

A mordhau can't actually punch through plate, but it can dent and break bone, and its usually depicted in treatises as targeting the head. You could also use it against chainmail, since a sword can't cut mail terribly well. So in the case of a zombie soldier with thick gear or a giant bug with chitinous plates it would make sense. Mechanically speaking it dosen't actually have a niche unless a monster has a significant difference between cut and bash, so I should probably still do something about it.

It dosen't really make sense to give it armor pen so I'm not really sure what a good fix is.

EDIT
I guess it does have the stun effect, so maybe give it a down duration instead? Or maybe the stun is enough?

EDIT 2
Stun should be enough on its on I think

EDIT 3
The purpose is to give something that doesn't care all that much about getting slashed(big hp pool/cut armor) a concussion

@QuintusAquila
Copy link
Contributor Author

@QuintusAquila Kevin suggested trying a rebase and having someone make sure to approve any tests if needed right after. If you can go on the discord and line someone up to approve your changes for right after a rebase which you may need someone to walk you through I don’t know you’re Git skills.

I rebased it. but I had less luck getting someone to approve the changes.

@Maleclypse
Copy link
Member

@QuintusAquila Kevin suggested trying a rebase and having someone make sure to approve any tests if needed right after. If you can go on the discord and line someone up to approve your changes for right after a rebase which you may need someone to walk you through I don’t know you’re Git skills.

I rebased it. but I had less luck getting someone to approve the changes.

It looks like it ran tests after the rebase but timed out. Let me try something later tonight

@QuintusAquila
Copy link
Contributor Author

@QuintusAquila Kevin suggested trying a rebase and having someone make sure to approve any tests if needed right after. If you can go on the discord and line someone up to approve your changes for right after a rebase which you may need someone to walk you through I don’t know you’re Git skills.

I rebased it. but I had less luck getting someone to approve the changes.

It looks like it ran tests after the rebase but timed out. Let me try something later tonight

I guess it didn't work?

@Maleclypse
Copy link
Member

I guess it didn't work?

Sorry I just tried it right now. I made a change to see if that restarts the tests and causes them all to run.

@QuintusAquila
Copy link
Contributor Author

QuintusAquila commented Sep 6, 2021

I'm pretty sure the problem started with one of these four commits:
1d90e51
fc6af77
501d21e
e7a925d
The test didn't time out before them, but did afterwards.

I may or may not get to testing it on my local machine to figure it out before classes start.

@QuintusAquila
Copy link
Contributor Author

QuintusAquila commented Sep 7, 2021

I tried the three windows methods to compile and failed over and over, and now I'm just going to try to see if conserve momentum is the issue. I strongly doubt the value tweaks/formatting I've done caused the problem, so if this doesn't work I'm lost again.

@QuintusAquila
Copy link
Contributor Author

@Maleclypse Could you start the tests proper?

@Maleclypse
Copy link
Member

@Maleclypse Could you start the tests proper?

Done.

@QuintusAquila
Copy link
Contributor Author

On closer examination, the test never actually passed, it just skipped the tests it was getting stuck on at one point. This probably won't work and I'm back to being totally lost.

@Maleclypse
Copy link
Member

OK I'm going to pull down your changes to my laptop tonight and test them and see what happens. I'm sorry you are having this happen on your very first PR.

@QuintusAquila
Copy link
Contributor Author

OK I'm going to pull down your changes to my laptop tonight and test them and see what happens. I'm sorry you are having this happen on your very first PR.

Ah, I'm used to technology fighting me tooth and nail, but thanks for all your help. I'd probably have given up far earlier without it.

Copy link
Member

@Maleclypse Maleclypse left a comment

Choose a reason for hiding this comment

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

I've pulled this onto my local machine and it works fine and I could find no errors nor any moves failing to work.

@actual-nh
Copy link
Contributor

Try turning off LTO for the Basic Build?

@esotericist
Copy link
Contributor

I've pulled this onto my local machine and it works fine and I could find no errors nor any moves failing to work.

does that include running the test suite? because that's where it times out in the workflow.

@Maleclypse
Copy link
Member

@QuintusAquila I have one more thing I'm thinking might work. We're in what if territory now, but please do me a favor and create a new PR from this branch and see if that will run.

@QuintusAquila
Copy link
Contributor Author

@QuintusAquila I have one more thing I'm thinking might work. We're in what if territory now, but please do me a favor and create a new PR from this branch and see if that will run.

I need to close this one to do that I think.

@QuintusAquila QuintusAquila deleted the MedievalSwordsmanshipReflavor branch September 8, 2021 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) [JSON] Changes (can be) made in JSON Melee Melee weapons, tactics, techniques, reach attack
Projects
None yet
Development

Successfully merging this pull request may close these issues.