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

Fix text rendering for the level up dialog #9479

Merged
merged 5 commits into from
Jan 29, 2025

Conversation

ihhub
Copy link
Owner

@ihhub ihhub commented Jan 22, 2025

close #9459

Additionally, right click popup messages for LEARN buttons were added.

@ihhub ihhub added bug Something doesn't work ui UI/GUI related stuff labels Jan 22, 2025
@ihhub ihhub added this to the 1.1.6 milestone Jan 22, 2025
@ihhub ihhub self-assigned this Jan 22, 2025
@ihhub ihhub requested a review from Branikolog January 22, 2025 15:15
@Branikolog
Copy link
Collaborator

Hi, @ihhub !
I've noticed, that text is aligned differently at lvlup window and hero window:
Lvlup:
image

hero/skill info windows:
image

The second one is shifted 1px to the left, while the first one is perfectly centered.

I suppose it is not related to the PR. Do you want me to open a separate issue for aligning texts on skills in hero/info windows?

@ihhub ihhub marked this pull request as draft January 25, 2025 12:44
@ihhub ihhub marked this pull request as ready for review January 29, 2025 04:22
@ihhub
Copy link
Owner Author

ihhub commented Jan 29, 2025

Hi @Branikolog , I've addressed your comment. Please take a look again.

@Mr-Bajs
Copy link
Contributor

Mr-Bajs commented Jan 29, 2025

Hi, @ihhub
I suggest that
You may learn either:\n%{skill1}\nor\n%{skill2}
into
The hero may learn either:\n%{skill1}\nor\n%{skill2}
To keep it consistent with the rest of the game.

@Branikolog
Copy link
Collaborator

@ihhub hi!
I've checked the lvlup dialog and everything looks good and consistent for both window and info hints.
However, I've noticed, that for some texts a shorter form is not used for hero info dialog:
👍
image

👎
image

The initial issue related to lvlup dialog only.
Should I open a separate issue for fixing hero info dialog?

@Branikolog
Copy link
Collaborator

Branikolog commented Jan 29, 2025

Hi, @Mr-Bajs !

Hi, @ihhub I suggest that You may learn either:\n%{skill1}\nor\n%{skill2} into The hero may learn either:\n%{skill1}\nor\n%{skill2} To keep it consistent with the rest of the game.

There are two separate buttons and each of the button corresponds each skill:
image

I think it's better to mention only 1 skill for each corresponding button.
The only thing I would improve here is the texts alignment and adding yellow headers. But since "hero" button also has same issue, I believe, it could be fixed in a separate PR for all three buttons.

@ihhub
Copy link
Owner Author

ihhub commented Jan 29, 2025

Hi @Branikolog , let's focus here on what was the original issue.

Hi @Mr-Bajs , I would like to keep the changes minimal as the proposed change is not related to the original problem.

Copy link
Collaborator

@Branikolog Branikolog left a comment

Choose a reason for hiding this comment

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

Good!
I'll open an issue for the hero window later then.

@ihhub ihhub merged commit dc4788e into master Jan 29, 2025
23 checks passed
@ihhub ihhub deleted the level-up-dialog-text-rendering branch January 29, 2025 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something doesn't work ui UI/GUI related stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adventure map, shortcut is not used for hero level up dialog
3 participants