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.

Sign up for GitHub

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

Jump to bottom

SoF: fix thursagan's arcanister advancement breaking in some cases #6484

Merged
merged 2 commits into from Feb 15, 2022
Merged

SoF: fix thursagan's arcanister advancement breaking in some cases #6484

merged 2 commits into from Feb 15, 2022

Conversation

knyghtmare
Copy link
Member

Fix for the following bug:
image

@github-actions github-actions bot added the Campaign (any) Deprecated tag, replaced with separate tags for each mainline campaign label Feb 3, 2022
@stevecotton
Copy link
Contributor

stevecotton commented Feb 3, 2022 •
edited

I'm not sure about this, but think all of [modifications] can be overwritten on unit rebuild. So I'm not sure about this fix.

Edit: I was wrong here.

@knyghtmare knyghtmare added the Ready for testing Issues for which a potential fix is available but untested. label Feb 3, 2022
@Pentarctagon
Copy link
Member

What would be a specific situation to test this with?

@Wedge009 Wedge009 added the Units Issues that involve unit definitions or their implementation in the engine. label Feb 4, 2022
@knyghtmare
Copy link
Member Author

What would be a specific situation to test this with?

As discerned on Discord, in Gathering Materials, get Thursagan into a cart and dismount.

Expected:

  1. After dismount, max XP is 210 and blue XP bar

Bug:
2. 150 XP and purple XP bar

@Pentarctagon
Copy link
Member

And you retested that situation after applying this fix, and the bug no longer occurred?

@knyghtmare
Copy link
Member Author

And you retested that situation after applying this fix, and the bug no longer occurred?

yes.

@knyghtmare
Copy link
Member Author

If this patches the bug, should it be merged?

@stevecotton stevecotton added the Backport A reminder of a bugfix that was added to master that needs to be duplicated on the stable branch. label Feb 11, 2022
@stevecotton
Copy link
Contributor

Looks good to me.

@stevecotton
Copy link
Contributor

Based on the discussion of who-should-merge-what yesterday, I should. Sorry for taking so long.

@knyghtmare please will you backport it?

@stevecotton stevecotton merged commit d3472d9 into wesnoth:master Feb 15, 2022
@knyghtmare knyghtmare mentioned this pull request Feb 15, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Backport A reminder of a bugfix that was added to master that needs to be duplicated on the stable branch. Campaign (any) Deprecated tag, replaced with separate tags for each mainline campaign Ready for testing Issues for which a potential fix is available but untested. Units Issues that involve unit definitions or their implementation in the engine.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@knyghtmare @stevecotton @Pentarctagon @Wedge009