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

UI: Allow any mod sound to be selected as a multiplayer turn notification sound #11623

Merged
merged 5 commits into from
May 29, 2024

Conversation

SomeTroglodyte
Copy link
Collaborator

... Actually, "New architecture to find any non-texture-atlas media" would be more fitting. but that title up there can go into the changelog.

No issue, this is for a slightly miscast "Modding request": Allow any mod sound to be selected as a multiplayer turn notification sound. - borrowed the title even though my first one was a tad shorter.

This is the extreme version - started with "how would I imagine an easy to use yet still flexible tool to answer the question 'which media file if any should I use for a given key'..." and then coded top-down from the framework draft. I did not yet check whether the other intended clients do anything in a significantly different way. I suspect the priority between newgame-selected mods, perm audiovisual ones, and builtin is not always exactly the same - but it should be.

The ugliest part of that request is how to attach human-readable labels to sounds, the old solution to that was ugly, and it's not much better here. As mentioned in my replies to the above, moving UncivSound back to enum-based - nope, without large refactorings that ultimately fails on unexpected impediments. Refactoring all UncivSound usages to cleanly distinguish between the interface and the enum - possible, but not right now. Then once that enum exists, it could carry display labels... Again, not yet. But in here there's still a tricky little imp that still enumerates the existing UncivSound.X instances almost as if the container were an enum. Almost moot since most instances need a mapped label anyway, but - had fun coding it.

No screenshots since I don't know of any suitable mod

@5cover - your idea and you mentioned PR'ing the same - so your review would welcome, s'il vous plaît.

@5cover
Copy link

5cover commented May 20, 2024

Sounds good. This solves my modding request and allows new sources for the notification sound :

  1. sounds from permanent audiovisual mods
  2. attack sounds of units from non-base rulesets (which was a workaround I attempted when 1. didn't work)

I'm not exactly familiar with how localization is done in Unciv - but does it work properly for builtin sound names and the "Attack Sound" suffix?

@yairm210 yairm210 merged commit 9e5c113 into yairm210:master May 29, 2024
3 checks passed
@SomeTroglodyte SomeTroglodyte deleted the MediaManager branch May 29, 2024 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants