Allow multi-select, pick random sound #6
Conversation
|
Exactly what I wanted! Would be great if this got merged! :) |
UI/Components/SoundSettings.cs
Outdated
| foreach (String file in fileDialog.FileNames) | ||
| { | ||
| path = path + "," + file; | ||
| } | ||
|
|
||
| textBox.Text = path; | ||
| callback(path); |
There was a problem hiding this comment.
Is there a reason we can't return an array of strings here instead?
There was a problem hiding this comment.
No, that should work, and would be smoother. I'm currently laid up from Vaxx # 2, but I can look into cleaning that up this weekend hopefully.
There was a problem hiding this comment.
I'm looking in to it now. It "works", but problems comes up with SettingsHelper. I'm currently fiddling to figure out how to get it to behave correctly, specifically around loading settings and saving empty fields.
{{EDIT: It may actually be the text box data binding that's the issue}}
There was a problem hiding this comment.
It was both. I think I handled it now.
Create New Data Bindings to handle array based Update SettingsHelper calls to translate Arrays to Strings and back Update PlaySound logic to use Array Update onsplit call to use array
| SettingsHelper.CreateSetting(document, parent, "Pause", Pause) ^ | ||
| SettingsHelper.CreateSetting(document, parent, "Resume", Resume) ^ | ||
| SettingsHelper.CreateSetting(document, parent, "StartTimer", StartTimer) ^ | ||
| SettingsHelper.CreateSetting(document, parent, "Split", string.Join(",", Split)) ^ |
There was a problem hiding this comment.
I think we'd want to store the multiple paths as child XML elements instead of having a comma separated list.
There was a problem hiding this comment.
I'm wanting this functionality, but unsure how the SettingsHelper works. Are you suggesting looping over Split and adding each to "Split1", "Split2", or something similar?
There was a problem hiding this comment.
Yes, that's a refactor that I could potentially look in to, but honestly since it takes ~5-6 months per review, and I do not program in C# any more, I've largely given up on this pull request.
There was a problem hiding this comment.
I held off on making changes too, in hopes my comment would lead to more info. It seems the maintainers have other plans than getting this and the other PRs ready. Understandable, everyone's busy these days 🤷
There was a problem hiding this comment.
I'd suggest bumping the PR (by leaving a comment and tagging me) to get quicker reviews. But yeah, I've been pretty busy with other stuff recently.
I was hoping the XML would look something like this:
<Split>
<Sound>my sound 1</Sound>
<Sound>my sound 2</Sound>
</Split>
I'm not entirely sure how you do that with the SettingsHelper, but it should be possible. The Splits component might be a good reference, since that component has a list of columns in its settings.
Changes the Sound Effect component to allow multi-selection from the open file dialogue, modifies the PlaySound function to choose a random sound from said list.
Tested with: no sounds, one sound, varying numbers of chosen sounds.