Skip to content

Comments

Animated Chapter Select Icons#927

Merged
maddie480 merged 4 commits intoEverestAPI:devfrom
ZenTheMod:feat/animated-map-icons
Jun 30, 2025
Merged

Animated Chapter Select Icons#927
maddie480 merged 4 commits intoEverestAPI:devfrom
ZenTheMod:feat/animated-map-icons

Conversation

@ZenTheMod
Copy link
Contributor

This PR allows for chapter icons (OuiChapterSelectIcon) to be animated;
The system for this is similar to that of Animated Parallax, using a .meta.yaml file to provide extra data if needed.

More detail on usage can be found on my (presumably now temporary) gamebanana page for AnimatedMapIconLib.

Changes include:

  • Modifying OuiChapterSelectIcon's ctor to allow for the detection of animated icons, this is done by checking if the path to the icon cotains the keyword "AnimatedMapIcons"; e.g. Graphics\Atlases\Gui\areas\MyCoolMod\AnimatedMapIcons
  • Modifying OuiChapterSelectIcon.Update to allow for updating both the front and back textures with the supplied animated textures.

Please feel free to inform me of any alternative implementation, or any incorrect practices, and or make any adjustments.

@maddie480-bot maddie480-bot added the 1: review needed This PR needs 2 approvals to be merged (bot-managed) label Jun 20, 2025
@maddie480-bot maddie480-bot added 2: changes requested This PR cannot be merged because changes were requested (bot-managed) and removed 1: review needed This PR needs 2 approvals to be merged (bot-managed) labels Jun 20, 2025
Now allows for front and back sprites to be animated independently; and no longer requires a unique string in the file path anymore.
(Also streamlined the yaml parsing.)
@ZenTheMod
Copy link
Contributor Author

ZenTheMod commented Jun 20, 2025

All requested changes have been made,
I'm also allowing both the back and front icons to be animated independently.

(Perhaps I may want to add an isAnimated bool for both the front and back? Not sure if anyone is going to be just animating the back or something.)

@ZenTheMod ZenTheMod requested a review from JaThePlayer June 20, 2025 20:18
Copy link
Member

@JaThePlayer JaThePlayer left a comment

Choose a reason for hiding this comment

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

lgtm, switching over to generated regex for slightly better perf would be nice but its not the end of the world here.

#pragma warning disable CS0649 // Field is never assigned to, and will always have its default value
#pragma warning disable CS0626 // Method, operator, or accessor is marked external and has no attributes on it
#pragma warning disable CS8632 // The annotation for nullable reference types should only be used in code within a #nullable annotations context
#pragma warning disable SYSLIB1045 // Use GeneratedRegexAttribute to generate the regular expression implementation at compile time.
Copy link
Member

Choose a reason for hiding this comment

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

Well, we should be doing that

@maddie480-bot maddie480-bot added 1: review needed This PR needs 2 approvals to be merged (bot-managed) and removed 2: changes requested This PR cannot be merged because changes were requested (bot-managed) labels Jun 21, 2025
Minor changes suggested by Wartori; including clarification on the `SaveData.Instance == null` check
@maddie480-bot
Copy link
Member

The pull request was approved and entered the 3-day last-call window.
If no further reviews happen, it will end on Jun 29, 2025, 12:00 AM UTC, after which the pull request will be able to be merged.

@maddie480-bot maddie480-bot added 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) and removed 1: review needed This PR needs 2 approvals to be merged (bot-managed) labels Jun 27, 2025
@maddie480-bot
Copy link
Member

The pull request was approved and entered the 3-day last-call window.
If no further reviews happen, it will end on Jun 30, 2025, 8:53 AM UTC, after which the pull request will be able to be merged.

@maddie480-bot
Copy link
Member

The last-call window for this pull request ended. It can now be merged if no blockers were brought up.

@maddie480-bot maddie480-bot added 4: ready to merge This PR was approved and the last-call window is over (bot-managed) and removed 3: last call window This PR was approved, and is in the 5-day last-call window before getting merged (bot-managed) labels Jun 30, 2025
@maddie480 maddie480 merged commit ecad4be into EverestAPI:dev Jun 30, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4: ready to merge This PR was approved and the last-call window is over (bot-managed)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants