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

Make tutorial hint messages persistent #6620

Merged
merged 6 commits into from May 20, 2022
Merged

Conversation

CelticMinstrel
Copy link
Member

Based on @stevecotton's #6615, this moves many of the tutorial hint messages into persistent overlay text. I played through the tutorial with these changes and I think it seems pretty good, but it probably needs a little refinement.

@github-actions github-actions bot added the Campaign (any) Deprecated tag, replaced with separate tags for each mainline campaign label Apr 10, 2022
@CelticMinstrel CelticMinstrel changed the title Tutorial hint messages Make tutorial hint messages persistent Apr 10, 2022
@CelticMinstrel
Copy link
Member Author

Here's an example of what it looks like.

@Wedge009
Copy link
Member

I haven't tried this but as with #6615 in principle it sounds like a good idea. It probably addresses the concerns mentioned in #6614 where the instructional part of the dialogue is missed amongst the 'flavour' text that some players may skip over.

@stevecotton
Copy link
Contributor

Some of the message timings should change - for example, S01's caption="Recruiting" message currently displays when you move to the keep, but would fit better if it appeared at the start of the lesson, when the "I'll recruit some elves" message appears.

Some text is looking a bit cut-off (this was in a 1280x1024 Wesnoth window), only noticed it on this message.
cutoff_overlay_text

@Pentarctagon
Copy link
Member

Pentarctagon commented May 12, 2022

So aside from the width issue, this seems good to merge? The message timings seem unrelated to this PR's purpose, since it's changing how a message is displayed rather than when.

@CelticMinstrel
Copy link
Member Author

Yes, I don't know of anything to block merging this. The width issue would be an issue in the API, not in this change. It would be nice to get a review from someone other than stevecotton though.

@Pentarctagon
Copy link
Member

@CelticMinstrel So should I click merge, or were you planning to?

@CelticMinstrel
Copy link
Member Author

Sure, I can do it.

@CelticMinstrel CelticMinstrel merged commit 0ffe926 into master May 20, 2022
@CelticMinstrel CelticMinstrel deleted the tutorial_hint_messages branch May 20, 2022 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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