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

update NR difficulty #6057

Draft
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

cooljeanius
Copy link
Contributor

@cooljeanius cooljeanius commented Aug 31, 2021

NR is notoriously difficult and annoying, so this PR attempts to soften it a little bit on the easier difficulties (not on HARD, though). These changes were originally made during my 1.14 playthru of NR, and haven't been tested on 1.15/1.16/1.17 yet, although I'm really dreading having to test them again... I'm leaving this marked as a draft because there are rejects files for map files that I haven't managed to wiggle back in yet, because this should really wait for until after 1.16 1.18 is released, and because I still need to do another pass at editing/testing.

there are still rejects files for maps though
minor comment movement, for fixing diff display
still trying to fix diff display
more updating of commentary for diff display purposes; hopefully this will be the final one...
@github-actions github-actions bot added the Campaign (any) Deprecated tag, replaced with separate tags for each mainline campaign label Aug 31, 2021
@cooljeanius
Copy link
Contributor Author

These changes originally come from here, btw: https://github.com/cooljeanius/wesnoth_mods/tree/master/campaigns/Northern_Rebirth/diffs

@@ -94,9 +94,13 @@
[side]
side=1
controller=human
recruit=Thug,Poacher,Spearman,Bowman,Gryphon,Dwarvish Fighter,Dwarvish Thunderer,Dwarvish Ulfserker,Dwarvish Scout,Footpad
# Tallin only asked the mages for their gryphons at the end of the previous scenario, so don't add any more of their units here:
recruit=Thug,Poacher,Spearman,Bowman,Gryphon,Dwarvish Fighter,Dwarvish Thunderer,Dwarvish Ulfserker,Dwarvish Scout,Footpad,Fencer,Heavy Infantryman
Copy link
Member

Choose a reason for hiding this comment

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

Fencers would look really odd in the Tallin's army of former ragged peasants. I think it's better to assume that the mages kept them to themselves as personal guardsmen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually initially added the fencers earlier in S06a "Old Friend", when Tallin and friends get the dwarves to forge new arms and armor for them and train them, the reasoning being that swords are newly forged, and that sometimes maps in NR get so crowded that units with skirmisher are necessary. The only reason I'm explicitly listing them in that scenario, too, is because since the recruit list is already explicitly listed there, it should at least be kept consistent with whatever units were previously added to it. If fencers don't seem ragged enough, perhaps some other unit with skirmisher could be created to fill that role instead?

Copy link
Member

Choose a reason for hiding this comment

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

@kabachuha That argument applies equally to heavy infantry as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kabachuha That argument applies equally to heavy infantry as well.

My same reply about the fencers applies to heavy infantry as well, except for the part about having skirmisher

Copy link
Member

Choose a reason for hiding this comment

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

If fencers don't seem ragged enough, perhaps some other unit with skirmisher could be created to fill that role instead?

What about the outlaw rogues? They have the skirmish ability and we already have thugs and footpads on the list, so I think they fit the band.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I dunno, I don't really feel like thieves fit, though... maybe earlier in the campaign when they had less options available to them, but at this point in the campaign they have other loyalist units and should be able to forge the high-quality swords necessary for fencers to be able to use

Copy link
Member

Choose a reason for hiding this comment

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

They still have thugs, poachers and footpads for recruit though.

Copy link
Member

@kabachuha kabachuha Sep 4, 2021

Choose a reason for hiding this comment

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

Fencing is not as much about the armor as about the training. I doubt Tallin's men could have mastered fencing in a couple of years the campaign takes place even if they had the best steel.

If they borrowed them from mages, one or two loyal fencer/HI units may be sufficient and not too breaking story- and gameplaywise.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about a campaign-specific unit tree? Possibly liminal, using recolored thief sprites (different hair color), and somewhere between a Fencer and an L1 Outlaw?

Among the close-knit community of survivors, thieves had little place. However, a fighting style of nimble footwork suited some more than heavy armor, and no-one would begrudge backstabbing, when said back belonged to a orc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about a campaign-specific unit tree? Possibly liminal, using recolored thief sprites (different hair color), and somewhere between a Fencer and an L1 Outlaw?

Among the close-knit community of survivors, thieves had little place. However, a fighting style of nimble footwork suited some more than heavy armor, and no-one would begrudge backstabbing, when said back belonged to a orc.

That might be a good longer-term goal, but it seems a bit out-of-scope for this PR...

@@ -158,51 +160,51 @@
{TRAIT_HEALTHY}
[/modifications]

hitpoints=1
{QUANTITY hitpoints 3 2 1}
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference between 1hp and 3hp? Both of them sound like a one-hit kill, and even the hp was raised to taking more than one hit then that would still sound like a design bug - if he's expected to survive any hits at all then there's a reliance on the RNG.

Copy link
Member

Choose a reason for hiding this comment

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

Well, even with 1hp there's a chance he'll survive.

Still, if he's expected to die, then this change does seem pointless.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe increase the variance a bit more, then?

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought the purpose of it was to make the player keep him at the back for this scenario, but it's a long time since I played it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought the purpose of it was to make the player keep him at the back for this scenario, but it's a long time since I played it.

Well, he's AI-controlled, and the AI retreats him anyways (has him switching between the keep for recruiting, and the nearest village for healing), so...

Copy link
Contributor

@stevecotton stevecotton Sep 4, 2021

Choose a reason for hiding this comment

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

Having the aim is good, but the rest of your answer seems to be missing my point. You're jumping from the aim to changing numbers instead of analysing what change to make.

With that aim, maybe the analysis would be "on easy he should always survive one troll getting through his defenses", and then the numbers come from how much a strong troll could do to him in a single turn. Possibly factor into that an assumption that he'll have some turns of healing on a village between the intro and that turn, so make the hitpoints still match the idea of a lich killing him in a single attack if Tallin's group hadn't rescued him.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think #8337 might be relevant here

Choose a reason for hiding this comment

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

I agree that 3/2/1 is not a reasonable change since it's just too minor. I'd suggest just going up at least slightly in some way (e.g. healing him after capturing the lich via event or something)

Copy link
Contributor Author

@cooljeanius cooljeanius Feb 29, 2024

Choose a reason for hiding this comment

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

With that aim, maybe the analysis would be "on easy he should always survive one troll getting through his defenses", and then the numbers come from how much a strong troll could do to him in a single turn.

Well if I went with a troll, they do 14 damage a hit (15 if strong), which would lead to a rather drastic variance ({29 15 1}, or {31 16 1} if we're assuming the "strong" trait)... how about if instead we use the weakest unit that's pre-placed for side 3, the Young Ogre. 1 hit from a Young Ogre is 5 damage. So, {11 6 1}, then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with {11 6 1} in 0149764 (untested).

@@ -343,6 +344,14 @@
id="Ro'Sothian"
[/recall]

[recall]
id=Varem
Copy link
Member

Choose a reason for hiding this comment

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

Who is Varem? Player NR like 7 times over the past 10 years and I do not know this character.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

one of the loyal dwarves who joins you when exploring Malifor's dungeons; really it could be any of them, but I just went with him because he's who I had available

Copy link
Member

Choose a reason for hiding this comment

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

I checked the code and he indeed does appear but the acquisition process is such that most players would not have him unless they prefer battling spiders.

Copy link
Member

Choose a reason for hiding this comment

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

If there's multiple loyal dwarves, you could just have it recall one of them at random? The recall tag can take an arbitrary filter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there's multiple loyal dwarves, you could just have it recall one of them at random? The recall tag can take an arbitrary filter.

That's a good idea, actually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I changed it to recall an arbitrary loyal dwarf in 0149764 (untested)

@cooljeanius
Copy link
Contributor Author

cross-referencing this PR against the feedback I left on the forums: https://forums.wesnoth.org/viewtopic.php?f=38&t=16993&p=660329#p660329

@Pentarctagon
Copy link
Member

@cooljeanius is this still something you're pursuing?

@cooljeanius
Copy link
Contributor Author

cooljeanius commented Jun 27, 2022

@cooljeanius is this still something you're pursuing?

yeah I intend to but it probably won't be this week though...

edit: IIRC the main thing getting in the way of me continuing with it was that merge conflict; I wish that wesnoth-map-diff tool could display the parts of maps that are causing a merge conflict, too... I guess I could just discard the changes to that map...

let's see if this fixes the merge conflict
remove some stray `^Efs`-es not in mainline
another one of these
almost done with this file?
@cooljeanius
Copy link
Contributor Author

ok I fixed the merge conflict; the failing map-diff CI task error message is:

/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/node_modules/@jimp/utils/dist/index.js:40
  throw error;
  ^

Error: The source must be a Jimp image
    at Jimp.throwError (/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/node_modules/@jimp/utils/dist/index.js:33:13)
    at Jimp.composite (/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/node_modules/@jimp/core/dist/composite/index.js:35:30)
    at paintMisc (/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/build/paint.js:52:16)
    at /home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/build/paint.js:76:17
    at Object.walkthrough (/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/build/tilemap.js:41:9)
    at paintTilemap (/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/build/paint.js:73:21)
    at Jimp.<anonymous> (/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/build/paint.js:84:9)
    at Timeout._onTimeout (/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/node_modules/@jimp/core/dist/index.js:264:25)
    at listOnTimeout (node:internal/timers:559:17)
    at processTimers (node:internal/timers:502:7)
Error: Process completed with exit code 1.

@macabeus do you know anything about that?

@macabeus
Copy link
Contributor

macabeus commented Aug 2, 2022

@cooljeanius I guess it's a bug already fixed on this PR: #6880
If it isn't a fixed bug, we'll at least have a better error message.

@cooljeanius
Copy link
Contributor Author

cooljeanius commented Aug 10, 2022

@cooljeanius I guess it's a bug already fixed on this PR: #6880 If it isn't a fixed bug, we'll at least have a better error message.

OK @macabeus so I updated this pull request to a version where that PR is merged and now the error is:

From https://github.com/wesnoth/wesnoth
 * branch              7e32194e43f418b0621af3be297d3c60ea911c7f -> FETCH_HEAD
** Working on data/campaigns/Northern_Rebirth/maps/05a_01_The_Pursuit.map **
/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/build/images.js:61
                    throw new Error(`Missing image for "^${miscCode}"`);
                          ^

Error: Missing image for "^Xo"
    at Object.getTile (/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/build/images.js:61:27)
    at paintTile (/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/build/paint.js:50:34)
    at /home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/build/paint.js:112:17
    at Object.walkthrough (/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/build/tilemap.js:41:9)
    at paintTilemap (/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/build/paint.js:110:21)
    at Jimp.<anonymous> (/home/runner/work/wesnoth/wesnoth/utils/wesnoth-map-diff/build/paint.js:123:9)
Error: Process completed with exit code 1.

(^Xo is the "impassable overlay", for reference)

cooljeanius added a commit to cooljeanius/wesnoth that referenced this pull request Mar 30, 2023
some suggestions from @macabeus for trying to fix the map diff failure in wesnoth#6057
(idk if this will actually work or not)
@github-actions
Copy link

github-actions bot commented Apr 13, 2023

data/campaigns/Northern_Rebirth/maps/05a_01_The_Pursuit.map


@cooljeanius
Copy link
Contributor Author

Yay, the map-diff CI task now passes! Looks like it didn't manage to upload the actual image successfully though; that's something that @macabeus and I are going to have to continue to work on... (I've seen it in a few other PRs, too...)

try adding some additional flags to curl
@cooljeanius
Copy link
Contributor Author

Yay, the map-diff CI task now passes! Looks like it didn't manage to upload the actual image successfully though; that's something that @macabeus and I are going to have to continue to work on... (I've seen it in a few other PRs, too...)

Relevant part of the log:

Imgur result:
{"errors":[{"id":"legacy-api-dfc4cf849-jssvj/BYtWSpW8As-1941785","code":"429","status":"Too Many Requests","detail":"Too Many Requests"}]}

@cooljeanius
Copy link
Contributor Author

blah, there's a merge conflict now... at least it's only in one file so far; if more files start having merge conflicts, I'm just going to give up on this...

@cooljeanius cooljeanius assigned cooljeanius and unassigned macabeus May 26, 2023
@Pentarctagon
Copy link
Member

What remains to be done? To be honest I'm kind of amazed that there's only been a single conflict after a year and a half.

@TheShadowOfHassen
Copy link
Contributor

Are the conflicts because of my dialog rewriting?

@cooljeanius
Copy link
Contributor Author

cooljeanius commented Jun 17, 2023

What remains to be done? To be honest I'm kind of amazed that there's only been a single conflict after a year and a half.

  • Come to some sort of agreement about Fencers and Heavy Infantrymen
  • Update recall filter I added for Varem to apply to any loyal dwarves instead (edit: done in 0149764, but still untested)
  • Another playthrough in which I retest my changes
  • Fix map-diff: change image upload location away from imgur #7557 for the map diff (actually never mind)

Are the conflicts because of my dialog rewriting?

Yeah but don't worry about it; it's ok... I intend to handle it.

@stevecotton stevecotton added Campaign NR The mainline Northern Rebirth campaign and removed Campaign (any) Deprecated tag, replaced with separate tags for each mainline campaign labels Dec 8, 2023
@cooljeanius cooljeanius added the Postponed Issues which cannot be worked on at this time. label Feb 17, 2024
hitpoints=12
experience=40
{QUANTITY hitpoints 16 14 12}
{QUANTITY experience 41 40 39}

Choose a reason for hiding this comment

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

this makes him weaker on HARD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll have to review this one once I'm back on my computer (on mobile currently)...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, right, an Ulfserker's max experience is 42. I can't go any higher without making him auto-advance, and I generally like to keep all 3 numbers different from one another, in a linear pattern...

Copy link

@FyiurAmron FyiurAmron Feb 29, 2024

Choose a reason for hiding this comment

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

@cooljeanius

I generally like to keep all 3 numbers different from one another, in a linear pattern

why? There's no need to differentiate the numbers for the sake of just being different, especially if the difference is this minuscule. It makes both maintaining a scenario a chore (more values to change on refactors etc), and, in this and many similar places, serves no actual purpose. I'd mirror what @stevecotton said before: balancing the scenario is not about having aesthetic numbers. It's about trying to find an aim first, and proposing the changes for that next.

If the unit is about to get AMLAd anyway, the HPs doesn't matter as long as it's the same amount of troll hits. Same with XP: if the unit is fighting trolls, then 40 XP and 41 XP are effectively the same, they'll either get AMLAd or killed, no other option here.

If you want to make the units able to survive more hits, you'll need to give them more than 2 HPs in this case (AFAIR, troll hits for 10-ish dam, both 12, 14 and 16 are two hits). Tweaking XP makes no sense whatsoever, with 40 XP it's just a single fight. That's why you're breaking HARD here, because 39 XP means surviving two troll combats, so it's virtually impossible and you're marking this unit to die.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cooljeanius

I generally like to keep all 3 numbers different from one another, in a linear pattern

why? There's no need to differentiate the numbers for the sake of just being different, especially if the difference is this minuscule.

Maybe not a NEED, but it's still a want, at least.

It makes both maintaining a scenario a chore (more values to change on refactors etc), and, in this and many similar places, serves no actual purpose.

It increases replayability, though.

I'd mirror what @stevecotton said before: balancing the scenario is not about having aesthetic numbers.

I disagree, aesthetic numbers are nice.

It's about trying to find an aim first, and proposing the changes for that next.

If the unit is about to get AMLAd anyway, the HPs doesn't matter as long as it's the same amount of troll hits. Same with XP: if the unit is fighting trolls, then 40 XP and 41 XP are effectively the same, they'll either get AMLAd or killed, no other option here.

If you want to make the units able to survive more hits, you'll need to give them more than 2 HPs in this case (AFAIR, troll hits for 10-ish dam, both 12, 14 and 16 are two hits). Tweaking XP makes no sense whatsoever, with 40 XP it's just a single fight. That's why you're breaking HARD here, because 39 XP means surviving two troll combats, so it's virtually impossible and you're marking this unit to die.

Well anyways, I'm no longer breaking HARD as of 0149764 (untested).

Copy link
Member

@CelticMinstrel CelticMinstrel Mar 1, 2024

Choose a reason for hiding this comment

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

You do realize that {QUANTITY 41 40 40} works just fine, right? Also, does 18 HP represent more hits than 12?

Copy link
Contributor Author

@cooljeanius cooljeanius Mar 1, 2024

Choose a reason for hiding this comment

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

You do realize that {QUANTITY 41 40 40} works just fine, right?

Yeah but I don't like repeating numbers between difficulties; I prefer the ifdef method in that case (when a number would repeat otherwise)

@FyiurAmron
Copy link

I'm inclined to agree that some changes are really groundbreaking, while some are too minor. I think that doing this per-scenario would be a better way to handle this, this is really huge.

@TheShadowOfHassen
Copy link
Contributor

So maybe separate the tweaks for each campaign and have a PR for each separately?

I really like the NR campaign, and I'd love to see it get a bit of a balance!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Map diff failure here is due to at least one of cooljeanius/wesnoth-map-diff#79, #7034, and/or cooljeanius/wesnoth-map-diff#80; I'm not quite sure which...

@cooljeanius
Copy link
Contributor Author

cooljeanius commented Jul 30, 2024

What remains to be done? To be honest I'm kind of amazed that there's only been a single conflict after a year and a half.

  • Come to some sort of agreement about Fencers and Heavy Infantrymen
  • Update recall filter I added for Varem to apply to any loyal dwarves instead (edit: done in 0149764, but still untested)
  • Another playthrough in which I retest my changes

OK so I fixed the merge conflicts and got the checks passing again; I still want to get checkboxes 1 and 3 resolved before taking this out of draft mode, though... (N.B. for the 3rd checkbox, it doesn't necessarily have to be me; someone else could also test for me...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Campaign NR The mainline Northern Rebirth campaign Postponed Issues which cannot be worked on at this time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet