Options page and time to next note indicator added #55
Conversation
ceakarsu
requested review from
andreasbalzer,
EnableIrelandAT and
muiriswoulfe
as code owners
|
The options page: |
|
The instrument selection page with note counts: |
|
The game page with time to note / playing indicator:
|
ProjectCoimbra.UWP/Project.Coimbra.Midi/Models/InstrumentInfo.cs
Outdated
Show resolved
Hide resolved
ProjectCoimbra.UWP/Project.Coimbra/Pages/InstrumentsPage.xaml.cs
Outdated
Show resolved
Hide resolved
ProjectCoimbra.UWP/Project.Coimbra/Pages/InstrumentsPage.xaml.cs
Outdated
Show resolved
Hide resolved
ProjectCoimbra.UWP/Project.Coimbra/Pages/InstrumentsPage.xaml.cs
Outdated
Show resolved
Hide resolved
ProjectCoimbra.UWP/Project.Coimbra.Midi/Models/InstrumentInfo.cs
Outdated
Show resolved
Hide resolved
| @@ -270,14 +305,16 @@ private void AddInstruments(MidiFile midi) | |||
|
|
|||
| if (this.Instruments.ContainsKey(currentEvent.Channel)) | |||
| { | |||
| if (!this.Instruments[currentEvent.Channel].Contains(programNumber)) | |||
| if (!this.Instruments[currentEvent.Channel].ProgramNumbers.Contains(programNumber)) | |||
There was a problem hiding this comment.
You might be able to use TryAdd here although I'm not sure it's available for the version of .NET we're using.
| else | ||
| { | ||
| this.InputControl.SetTimeToNextNote($"Playing{new string('.', (this.dotCounter / 5) + 1)}"); | ||
| this.dotCounter++; |
There was a problem hiding this comment.
Can use this.dotCounter = this.dotCounter + 1 % 15
| @@ -32,12 +33,17 @@ public sealed partial class GamePage : Page | |||
|
|
|||
| private readonly DispatcherTimer timer = new DispatcherTimer(); | |||
| private readonly TimeSpan noteDuration = TimeSpan.FromSeconds(5); | |||
| private readonly long msInASecond = (long)TimeSpan.FromSeconds(1).TotalMilliseconds; | |||
There was a problem hiding this comment.
I'd suggest millisecondsInASecond would be clearer
| @@ -29,6 +30,9 @@ public sealed partial class InstrumentsPage : Page | |||
|
|
|||
| private readonly MidiEngine midiEngine = MidiEngine.Instance; | |||
|
|
|||
| private readonly ResourceLoader resourceLoader = ResourceLoader.GetForCurrentView(); | |||
| private string waitingIndicatorResource = string.Empty; | |||
There was a problem hiding this comment.
You don't need to initalise this value as you're setting it in the constructor.
| @@ -130,7 +137,7 @@ private void ShowPlayersInstrumentInfo() | |||
| (x.Instrument > -1 | |||
| ? " - " + | |||
| Instruments[x.Instrument] | |||
| : " - Waiting...")) | |||
| : $" - {waitingIndicatorResource}")) | |||
There was a problem hiding this comment.
Are you calling this multiple times? If not, you don't need the class variable and you can just get the information within the method.
| @@ -22,7 +22,7 @@ Licensed under the MIT License. | |||
| </Grid.ColumnDefinitions> | |||
|
|
|||
| <StackPanel Grid.Row="0" Grid.ColumnSpan="2" Grid.Column="0"> | |||
| <TextBlock HorizontalAlignment="Center" Style="{StaticResource HeaderTextBlockStyle}" Padding="0,50,0,50" Text="Well done! What would you like to do next?" x:Uid="OptionsPage.Gig" /> | |||
| <TextBlock HorizontalAlignment="Center" Style="{StaticResource HeaderTextBlockStyle}" x:Uid="OptionsPage.Title" Padding="0,50,0,50" Text="Well done! What would you like to do next?" /> | |||
There was a problem hiding this comment.
I believe the tabs/spaces formatting is wrong here again
| @@ -329,4 +329,48 @@ | |||
| <data name="LaneSetupControl.KeysSelected" xml:space="preserve"> | |||
| <value>Keys {0} selected.</value> | |||
| </data> | |||
| </root> | |||
| <data name="OptionsPage.Title.Text" xml:space="preserve"> | |||
There was a problem hiding this comment.
Thanks for fixing this up!
| <value>Well done! What would you like to do next?</value> | ||
| <comment>The text of the Title on the Options Page</comment> | ||
| </data> | ||
| <data name="OptionsPage.ChangeNoteDurationButton.Content" xml:space="preserve"> |
There was a problem hiding this comment.
These resources should also be added to the "de-DE" file, which currently is identical. I'd suggest just copying and pasting this file on top of the other.
| @@ -329,24 +329,68 @@ | |||
| <data name="LaneSetupControl.KeysSelected" xml:space="preserve"> | |||
| <value>Keys {0} selected.</value> | |||
| </data> | |||
| <data name="OptionsPage.Title.Text" xml:space="preserve"> | |||
There was a problem hiding this comment.
The merge seems to have gone a bit strangely here. Ideally, the strings should be sorted alphabetically as I believe VS will automatically do this when it opens the file in the rich string editor. To avoid this, it's better to sort them now and avoid the churn each time the file is hit.
Added an Options Page at the end of the song with options to,
Also added a label that shows the remaining time to the next note to play (if it's more than 3 seconds away).
Added the note counts to the instrument selection page. The instruments are now ordered by how many notes they have. And the instruments with only 0 notes are not added to the list.
Will move the strings to resources and will try to improve the conditions to show the time to next note before I merge this.