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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Gazyi
Copy link

@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.

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
Status: No status
Status: Alystrasz' TODOs
Development

Successfully merging this pull request may close these issues.

4 participants