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

Replace patch and UI icons with SVGs #5578

Merged
merged 8 commits into from
Oct 15, 2024
Merged

Replace patch and UI icons with SVGs #5578

merged 8 commits into from
Oct 15, 2024

Conversation

HexapodPhilosopher
Copy link
Contributor

@HexapodPhilosopher HexapodPhilosopher commented Oct 9, 2024

Brief Description of What This PR Does

Adds new .svg icons for spinboxes, optionButtons and sliders.

Replaces patch icons with SVGs.

Re-scales compound .svg icons to 256x256.

Related Issues

Fixes some parts of #1581 :

  • spinbox up/down arrows...
  • slider circular part (grab.png with all variants and sliderTick.png)
  • all of the patch map, patch icons...

Progress Checklist

Note: before starting this checklist the PR should be marked as non-draft.

  • PR author has checked that this PR works as intended and doesn't
    break existing features:
    https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
    (this is important as to not waste the time of Thrive team
    members reviewing this PR)
  • Initial code review passed (this and further items should not be checked by the PR author)
  • Functionality is confirmed working by another person (see above checklist link)
  • Final code review is passed and code conforms to the
    styleguide.

Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.

@hhyyrylainen hhyyrylainen self-requested a review October 9, 2024 16:50
@hhyyrylainen hhyyrylainen added this to the Release 0.7.1 milestone Oct 9, 2024
@hhyyrylainen
Copy link
Member

Could you clarify which parts of #1581 this fixes so that when I have a look at this tomorrow it'll be easier to focus my testing?

Copy link
Member

@hhyyrylainen hhyyrylainen left a comment

Choose a reason for hiding this comment

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

There's a few concrete things I think should be fixed. Additionally I noticed that 4 patch icons now have border artifacts when zoomed out in the map fully:

kuva

If I reduce the resolution limit to 128 for the cave patch it seems to mostly get rid of the problem but not entirely, so that might be a doable stop-gap measure.

Also this pretty nicely improves the look of the compound icons so that is nice, but there is still ever so slight border artifacts on the iron icon. I tried reducing the size to 128x128 and that maybe kind of helped a bit but made the iron look a bit blurrier than other icons. So for now I think the icon tweaking is good enough.

I also wanted to say that it looks like SVGs have quite many pitfalls about their use in Godot, but I hope we can solve them enough that no players will notice it.

@HexapodPhilosopher
Copy link
Contributor Author

I've reduced the blurring of the corners and edge of the icons by increasing the base sizes and scaling down in the editor. The compound icon blur is not even visible to me anymore. I also removed a few disused ui textures.

Copy link
Member

@hhyyrylainen hhyyrylainen left a comment

Choose a reason for hiding this comment

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

I think all the artifacts I saw are now gone, and the slider tick is correctly aligned. So good job.

Let's remember for the future that tweaking the SVG scale to be less than 1 while increasing the SVG size (and setting a maximum size in Godot) is what seems to fix the artifacting problems.

@hhyyrylainen hhyyrylainen merged commit ed28a01 into master Oct 15, 2024
4 checks passed
@hhyyrylainen hhyyrylainen deleted the UI_Icons branch October 15, 2024 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants