-
-
Notifications
You must be signed in to change notification settings - Fork 74
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
Jean/party mode pull request #336
Conversation
Audio being mandatory, it makes more sense to output an error since the song won't work at all.
The custom control shows the selected flag, or "Mixed Values" or "None" if relevant, and selection happens in a custom dialog showing all possible options as toggles.
Party Mode has two options: "Teams" and "Free for All". Currently "Teams" isn't working, although the UI has been started. "Free for All" will work to automatically create rounds to match and make all added users play. Eventually it would be nice to have a tournament mode for the "Free for All" mode, so that winning players are matched against each others, until there is only one winner. "Free for All" will only work if there are exactly two microphones enabled - support for more players/microphones needs to be added. The flow works as follow: - Participants can be added, either using existing Player Profiles, or entering new guest names through text fields - Based on the number of participants, the number of rounds will be calculated - Song selection can be manual, one random song, or a subset of random songs so that users have a bit more choices - in team mode, we should let users from each team to choose the song in turns - there is an infinite joker button that will reselect (a) new song(s), eventually the number of joker per player/teams should be modifiable (including an "infinite" value) - For each round, users can define: - the winning condition (still work in progress, only the highest score will properly work) - a game modifier that will alter the gameplay based on a condition - A "Versus" screen shows the randomly matched players, as well as the winning condition and modifiers for the round, if any - Once the song ends, the "Versus" screen is shown again for the next round
public enum EPartySongSelection | ||
{ | ||
RandomSong, // a random song is picked | ||
RandomSubset, // a random subset of songs is picked, and players choose which one to sing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Nice idea. Playlists could be reused as user created subsets.
- Players should be able to define which subsets qualify for the random selection
{ | ||
public enum ETriggerType | ||
{ | ||
Infinite, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Better call it
Always
?- "When does it trigger" =>
Always
- "When does it trigger" =>
Infinite, | ||
AfterTime, | ||
UntilTime, | ||
TimeRange, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- "When does it trigger" =>
InTimeRange
?- Or just
TimeRange
, I am not sure, whatever
- Or just
{ | ||
RandomSong, // a random song is picked | ||
RandomSubset, // a random subset of songs is picked, and players choose which one to sing | ||
PlayersChoose // players choose the song |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The other values use a noun at the end. Thus, I'd suggest
PlayerChoice
or justManual
.
|
||
// TODO use this to filter the randomly selected songs | ||
[Flags] | ||
public enum EPartySongFiltering |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It should be possible to select one or more values for each EPartySongFiltering
- How to combine multiple values?
- Probably
any
value of selected category, butevery
selected category at least one.
- Probably
- For example, two artists and one genre selected => all songs of the genre that are performed by one of the artists?
- How to combine multiple values?
- The question is, how can users select sets of songs?
- Maybe this needs a different data structure, e.g. a list of song-lists. And each song-list is filled by a song search.
- Example: User searches for
green day
, then has a button to add all found songs as song-list. User searches for1990<=Year&&Year<=1999
, then again adds all found songs as song-list.
@@ -84,6 +87,9 @@ public static SongSelectSceneControl Instance | |||
[Inject(UxmlName = R.UxmlNames.songIndexButton)] | |||
private Button songIndexButton; | |||
|
|||
[Inject(UxmlName = R.UxmlNames.buttonJoker)] | |||
private Button buttonJoker; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- It is a button, should be called
jokerButton
. In general, most defining stuff at the end of the name.
@@ -93,6 +94,19 @@ private SongMeta SelectedSongMeta | |||
|
|||
private void Start() | |||
{ | |||
Selection.Subscribe(selection => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- What are you trying to achieve? I think the coroutine is not needed
- For example, UniRx has methods to do stuff when the value has been stable some time ( search
.Throttle(
)
- For example, UniRx has methods to do stuff when the value has been stable some time ( search
if (sceneData.IsPartyMode) | ||
{ | ||
// TODO intermediary scene that sums up all the rounds after last round | ||
SceneNavigator.Instance.LoadScene(PartyModeManager.CurrentRoundData.isLastRound ? EScene.MainScene : EScene.PartyModeVersus); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The proper target scene should be selected directly from SingScene instead of redirecting here.
@@ -274,8 +295,230 @@ private void Start() | |||
{ | |||
timeBarControl?.UpdateTimeValueLabel(songAudioPlayer.PositionInSongInMillis, songAudioPlayer.DurationOfSongInMillis); | |||
})); | |||
|
|||
// Modifiers apply callbacks | |||
foreach (SingModifier modifier in sceneData.SingModifiers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- There is naturally quite a lot new logic in this class. I'd suggest to create a dedicated
PartyModeControl
for it.SingSceneControl
can then delegate to this.
// Disable warning about fields that are never assigned, their values are injected. | ||
#pragma warning disable CS0649 | ||
|
||
// Scene that shows before a round in Party Mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I think this is a good idea. Players can prepare, change mics, etc. in this scene.
- However, maybe this could be combined with the round results?
- After each round, the results are shown AND next players can prepare. I don't think we need a dedicated scene for this.
Thanks for your work! I still have some conceptual doubts for the party modes. Team vs free-for-all
A knock-out system is a neat idea. PlayerProfiles vs. guest user names
Round count vs. player count
This should be separated. 5 people may want to play 3 rounds. Song selection from subset of songs See my comment above:
Versus scene vs. round result scene
Translations
|
"Multiple values" => We should implement some chips component to select multiple values. It should be visible, which values have been selected. |
The party settings UI need some description text field, for example to explain win conditions. |
I suggest trying to find a way of allowing contributions get merged while you have your fork with features that you keep private for now. |
I will close this as obsolete after #433. @jean-moreno feel free to create a new ticket if you have further feature requests. |
What does this PR do?
Implement party modes.
Current approach:
Additional Notes
@jean-moreno this is a draft PR for your branch such that I can review and give feedback.
I created a new branch for the PR. Thus, you can continue development on your original branch and when ready, merge it to this branch.