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

Get Flat Icon Fixes #4505

Merged
merged 3 commits into from
Oct 4, 2023
Merged

Conversation

Drulikar
Copy link
Contributor

@Drulikar Drulikar commented Sep 25, 2023

About the pull request

This PR fixes two issues with getFlatIcon: It failing to properly resize its template dimensions to whatever icon dimensions it was passed, and it ignoring appearance_flags for RESET_COLOR and RESET_ALPHA. This was done to address issues with #4475 but can be even more performance costly than it already was because now it applies color and alpha blends per overlay/underlay layer rather than once per recursive call if there is a color/alpha to apply.

Explain why it's good for the game

Fixes the broader potential uses for getFlatIcon such as flattening the tacmap.

Testing Photographs and Procedure

Screenshots & Videos

Example of tacmap flattened prior to dimension fixes:
image

Example of tacmap flatted prior to color fixes:
image

Example of tacmap properly flattened:
image

Changelog

🆑 Drathek
fix: Fix getFlatIcon not resizing its template nor respect appearance_flags of RESET_COLOR and RESET_ALPHA
/:cl:

Copy link
Member

@morrowwolf morrowwolf left a comment

Choose a reason for hiding this comment

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

So I'm not 100% on how the changes are working but would it be possible to put the more expensive stuff that's only needed for some things behind an argument or similar?

@morrowwolf morrowwolf marked this pull request as draft September 30, 2023 06:05
@gitbirb gitbirb mentioned this pull request Oct 1, 2023
3 tasks
@Drulikar
Copy link
Contributor Author

Drulikar commented Oct 2, 2023

With appearance_flags default FALSE
default

If I were to make the default TRUE (so all use the fix):
opt in

No changes that I can tell - but should be the older performance. The new tacmap PR must opt in to this fix else all icons are colored incorrectly. I did also try to take a stab at why the vents appear layered wrong in photos, but I'm not sure yet why they don't sort correctly. TG's photo method is drastically different.

@harryob harryob added this pull request to the merge queue Oct 4, 2023
Merged via the queue into cmss13-devs:master with commit eb614a6 Oct 4, 2023
26 checks passed
cm13-github added a commit that referenced this pull request Oct 4, 2023
@Drulikar Drulikar deleted the Drathek_GetFlatIcon_Fixes branch October 5, 2023 02:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Fix one bug, make ten more Testmerge Candidate we'll test this while you're asleep and the server has 10 players
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants