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

Make the application runnable in UNIX. #22

Merged
merged 1 commit into from
Mar 15, 2017

Conversation

JunielKatarn
Copy link
Contributor

This change implements a MuteNAudioSongPlayer, which mocks lacks any _waveform member or implementation.

Works on Mac OS X (10.2) and Linux (Ubuntu).

The song will successfully load and display controls and score, but the 'Play' functionality is just a placeholder for now.
screenshot from 2017-03-06 00-15-59
screenshot from 2017-03-06 00-16-59
screenshot from 2017-03-06 00-34-46

Note: class MockSongPlayer was also implemented, but its usage broke the app due to indexing errors related to some control's Max and Min values.

@JunielKatarn
Copy link
Contributor Author

Makes progress in #16.

@JunielKatarn
Copy link
Contributor Author

Hi @maxton. Any issues with this PR?

@maxton
Copy link
Owner

maxton commented Mar 11, 2017

Yes. Instead of refactoring JammitNAudioSongPlayer, you should only have added one class: MockSongPlayer, which implements ISongPlayer. In the future there will probably only be one song player, probably using PortAudio. So there's no need to pollute the class hierarchy in such a way.

The MockSongPlayer class should mostly replicate the external behavior of JammitNAudioSongPlayer so that the other components can be tested; it should respond to Play/Pause/Stop calls, it should use a background thread to increment the Position when in the Play state, and should expose the channels in the same way as JammitNAudioSongPlayer does. This way things such as animation can be tested using this class without audio output.

@JunielKatarn
Copy link
Contributor Author

I did implement the a mock audio player, but it crashed the application due to Max/Min value bindings for the UI controls.

I'll revisit it and show you where the exceptions originate. Maybe you can shed some light into how to correctly mock the offending methods?

@JunielKatarn
Copy link
Contributor Author

@maxton See the current MockSongPlayer implementation.
If used, loading a song fails with the following exception:

System.ArgumentOutOfRangeException occurred
  HResult=0x80131502
  Message=Value of '1' is not valid for 'Value'. 'Value' should be between 'Minimum' and 'Maximum'.
  Source=System.Windows.Forms
  StackTrace:
   at System.Windows.Forms.TrackBar.set_Value(Int32 value)
   at Jammit.Controls.SeekBar.set_SamplePosition(Int64 value) in D:\Build\jamnet\mono\Controls\SeekBar.cs:line 29
   at Jammit.SongWindow.TimerTick(Object sender, EventArgs e) in D:\Build\jamnet\mono\Forms\SongWindow.cs:line 52
   at System.Windows.Forms.Timer.OnTick(EventArgs e)
   at System.Windows.Forms.Timer.TimerNativeWindow.WndProc(Message& m)
   at System.Windows.Forms.NativeWindow.DebuggableCallback(IntPtr hWnd, Int32 msg, IntPtr wparam, IntPtr lparam)
   at System.Windows.Forms.UnsafeNativeMethods.DispatchMessageW(MSG& msg)
   at System.Windows.Forms.Application.ComponentManager.System.Windows.Forms.UnsafeNativeMethods.IMsoComponentManager.FPushMessageLoop(IntPtr dwComponentID, Int32 reason, Int32 pvLoopData)
   at System.Windows.Forms.Application.ThreadContext.RunMessageLoopInner(Int32 reason, ApplicationContext context)
   at System.Windows.Forms.Application.ThreadContext.RunMessageLoop(Int32 reason, ApplicationContext context)
   at System.Windows.Forms.Application.Run(Form mainForm)
   at Jammit.Program.Main() in D:\Build\jamnet\mono\Program.cs:line 19

Line:
trackBar1.Value = _samples == 0 ? 1 : (int)(trackBar1.Maximum * _samplePos / _samples);

You mention the MockSongPlayer should increment the position in a background thread. Is this expcted to fix the issue (seems to be a division by zero).
If so, how can the mock instance tell what's the current position of the track, without using the WaveOut?

@JunielKatarn
Copy link
Contributor Author

Simplified the changes. Builds and runs on OS X. Does not intrude with the current Windows code base.

@maxton
Copy link
Owner

maxton commented Mar 14, 2017

Looks good. Can you squash those commits into one?

@JunielKatarn JunielKatarn reopened this Mar 15, 2017
@JunielKatarn
Copy link
Contributor Author

@maxton Squashed, and ready to merge.

@maxton maxton merged commit c8355d0 into maxton:master Mar 15, 2017
@JunielKatarn JunielKatarn deleted the mono_mocksongplayer branch March 15, 2017 08:53
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.

2 participants