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

Max torch final victory dialog handling of SoF s9 rework #6628

Conversation

max-torch
Copy link
Contributor

@max-torch max-torch commented Apr 18, 2022

This is my contribution to completing what was started in #6334 .
The issue this tries to close is #6332.
Things in the scenario didn't quite make logical sense considering the presence of Gryphon Riders. This resolves the issue by allowing them to trigger an event, and by acknowledging them in the dialog.

The last narrator dialog is slightly different depending on whether or not the player has a gryphon rider in addition having Krawg. Also, events are triggered when Gryphon Riders move to the stone tablet at the bottom of the map.

@github-actions github-actions bot added the Campaign (any) Deprecated tag, replaced with separate tags for each mainline campaign label Apr 18, 2022
@Wedge009
Copy link
Member

Wedge009 commented Apr 20, 2022

I tested this and it seems to cover the issues we discussed previously: a non-Krawg unit finding the final rune and also addresses the plot-hole regarding the Gryphon Riders. A bit unfortunate that the entire volcano eruption event has to be duplicated to cover both the Krawg version and the regular Gryphon Rider version - might it be possible to re-use that code?

At any rate, as it currently stands it seems to resolve #6332 satisfactorily. This is a change worth including in the change log. There are a few new strings, but not sure if there any fuzzies generated to notify translators about. Maybe @nemaara wants to review the string changes first, but I don't think there were any major ideas added, besides the explanation for Krawg being the only survivor.

@Wedge009 Wedge009 added the Backport A reminder of a bugfix that was added to master that needs to be duplicated on the stable branch. label Apr 20, 2022
@max-torch
Copy link
Contributor Author

max-torch commented Apr 20, 2022

might it be possible to re-use that code?

Thank you for testing and confirming on your end. I'll see about doing one more refinement of the dialog. As to minimizing duplication of code, I'll see what I can do. Perhaps I'll define the redundant code as a function/macro and then just call it for both conditions.

@knyghtmare
Copy link
Member

also, be sure to run wmlindent on this to get all 12 checks on the pesky CI.

@max-torch
Copy link
Contributor Author

max-torch commented Apr 21, 2022

I would like to provide some replays to show how I have tested the latest commit.
Case 1: No other gryphons in the level, and Krawg gets tablet
SoF-Caverns of Flame replay 20220421-180422.gz
Case 2: Level ends with Gryphon Master at the center, and Krawg gets tablet
SoF-Caverns of Flame replay 20220421-180536.gz
Case 3: No gryphons at the center, and a Gryphon Master gets the tablet
SoF-Caverns of Flame replay 20220421-180700.gz
Case 4: A gryphon is at the center, and a Gryphon Master gets the tablet
SoF-Caverns of Flame replay 20220421-180802.gz

Also, I removed a double-space in a piece of dialog, is it okay to bundle it into this PR?

@Wedge009
Copy link
Member

Wedge009 commented Apr 21, 2022

Also, I removed a double-space in a piece of dialog, is it okay to bundle it into this PR?

It's already removed in b775ba8 / 527cd41 - not sure why it's showing as a difference here. Maybe try re-basing your branch to current origin/master?

Also, I forgot to mention: would it be reasonable to use 'out-fly' instead of 'outrun'?

@max-torch
Copy link
Contributor Author

Also, I forgot to mention: would it be reasonable to use 'out-fly' instead of 'outrun'?

After a quick internet search, outrun seems to be used in a lot of contexts that don't involve literal running. Seems like it can be used in the context of anything that travels faster. Merriam-Webster lists outfly as a word but the Cambridge Dictionary doesn't. I found a graph showing outfly had little usage over time. So, for the sake of being less obscure, I guess I'm in favor of outrun.

@max-torch max-torch force-pushed the max-torch-final-victory-dialog-handling-of-SoF-S9-rework branch from a7d2c9e to 166796b Compare April 21, 2022 22:53
…ic tablet, and add translator hint

Things in the scenario didn't quite make logical sense considering the presence of Gryphon Riders. This resolves the issue by allowing them to trigger an event, and by acknowledging them in the endlevel dialog. Additionally, one translator hint is added for the "Smash" text when the rune is smashed.
@max-torch max-torch force-pushed the max-torch-final-victory-dialog-handling-of-SoF-S9-rework branch from 166796b to e0108f1 Compare April 21, 2022 22:55
@max-torch
Copy link
Contributor Author

max-torch commented Apr 21, 2022

The PR is now squashed neatly into a single commit and rebased on top of the HEAD of the present Master branch. Some grammatical/dialog issues have been addressed. Replays of possible test cases have been submitted. After this passes the CI (hopefully optimistic) I submit this for merging.

@Wedge009
Copy link
Member

Looks good to me. Thanks.

Copy link
Member

@knyghtmare knyghtmare left a comment

Choose a reason for hiding this comment

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

I check code but I feel like Krawg has way too much plot armour here, but I guess it's fine.

@Wedge009
Copy link
Member

The plot armour was always there - at least since the scenario was rewritten in 1.15.x - my request and this PR are trying to address the game-play issue of Krawg being the only one permitted to break the last rune and the story issue of the Gryphon Riders not being able to escape.

@knyghtmare
Copy link
Member

The plot armour was always there - at least since the scenario was rewritten in 1.15.x - my request and this PR are trying to address the game-play issue of Krawg being the only one permitted to break the last rune and the story issue of the Gryphon Riders not being able to escape.

One thing I like about this reworked SoF is that it removed that non-sensical concept of "Thursagan dying and then all other runecrafters dying with him"..

But I dont like SoF's high difficulty curve (since I cannot get pass the 5th level despite 20ish tries)

@Wedge009 Wedge009 merged commit f82079b into wesnoth:master Apr 22, 2022
@Wedge009
Copy link
Member

Thanks everyone for your contributions.

@max-torch max-torch deleted the max-torch-final-victory-dialog-handling-of-SoF-S9-rework branch April 22, 2022 04:27
[/message]
[message]
speaker=Thursagan
message= _ "Just do it! You riders have a chance to escape, our enemies cannot. We knew that death was likely when we fled here, just go!"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is one of the ones that might have another variant if there's only one gryphon rider, maybe: "Just do it! You and Krawg have a change to escape, our enemies cannot. We knew that death was likely when we fled here, just go!".

Edit: have realised that the scenario no longer recalls all of the player's units. Maybe we should just assume that there are some riders on the recall / recruit lists, who'll try to escape even if they weren't recruited.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants