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

Optional icons for server-side RUI announcements #883

Merged
merged 4 commits into from
Nov 22, 2024

Conversation

Gazyi
Copy link
Contributor

@Gazyi Gazyi commented Sep 14, 2024

PR adds optional icon support for server-side RUI announcements.
mp_forwardbase_kodai0003

@github-actions github-actions bot added needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code labels Sep 14, 2024
@GeckoEidechse GeckoEidechse self-assigned this Sep 20, 2024
@@ -453,6 +454,7 @@ void function AnnouncementMessageHandler_Threaded()
Announcement_SetPriority( announcement, client.announcementQueue[0].priority )
Announcement_SetSoundAlias( announcement, SFX_HUD_ANNOUNCE_QUICK )
Announcement_SetStyle( announcement, client.announcementQueue[0].style )
Announcement_SetIcon( announcement, StringToAsset( strip( client.announcementQueue[0].image ) ) )
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems weird that they are stored in the struct as a string and converted to asset here but eh, probably still the best way to do it

Copy link
Contributor

@Alystrasz Alystrasz left a comment

Choose a reason for hiding this comment

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

Code looks good, and feature works in game.
I could display images with the updated command; not passing the last parameter to it simply doesn't display any image.

Testing

Using the following command:

script NSSendAnnouncementMessageToPlayer(GetPlayerArray()[0], "FIRE", "I’M A’ FIRIN’ MAH LAZER!!", <1,0,1>, 1, 1, "rui/hud/scoreboard/status_dead")

I get:
image

@Alystrasz Alystrasz added READY TO MERGE This mergeable right now and removed needs testing Changes from the PR still need to be tested needs code review Changes from PR still need to be reviewed in code labels Nov 2, 2024
@GeckoEidechse
Copy link
Member

Will add this to the next feature release. Just need push out a patch release first. So consider this essentially merged.

I'll hopefully have some time tomorrow or something to figure out the strategy for the next few releases.

@GeckoEidechse GeckoEidechse changed the title Optional icons for server-side RUI announcements. Optional icons for server-side RUI announcements Nov 22, 2024
@GeckoEidechse GeckoEidechse merged commit c568cf2 into R2Northstar:main Nov 22, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
READY TO MERGE This mergeable right now
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants