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

Prodrums implementation #33

Closed
wants to merge 13 commits into from
Closed

Prodrums implementation #33

wants to merge 13 commits into from

Conversation

justmoen
Copy link

@justmoen justmoen commented Mar 7, 2015

Enabled MIDI drums and tested with Roland HD-1.
Fullscreen was disabled.
Added advanced audio setting to disable Jurgen taunting.
Implemented Pro Drums for Megalight V4 only.

@mdsitton
Copy link
Member

mdsitton commented Mar 7, 2015

Looked through it, so far looks alright. Ill try to test it here in a bit.

@justmoen
Copy link
Author

justmoen commented Mar 7, 2015

Thank you for taking a look at it. I realize that you are doing a complete
rewrite of FoFiX. I have another commit for today that will allow for Pro
drums with lefty mode.

Cheers,
Justin
On Mar 7, 2015 2:24 PM, "Matthew Sitton" notifications@github.com wrote:

Looked through it, so far looks alright. Ill try to test it here in a bit.


Reply to this email directly or view it on GitHub
#33 (comment).

@mdsitton
Copy link
Member

mdsitton commented Mar 7, 2015

Yeah, I am slowly working on it. Not everyone is on board with doing a rewrite, which is why i haven't really made it much of a priority.

Alright commit away, the project needs some activity.

if drumTrackName:
self.drumTrack = Audio.StreamingSound(self.engine.audio.getChannel(3), drumTrackName)
if drumTrackNames:
for drumTrackName in drumTrackNames:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this loop instead of what you have you could use enumerate()

for i, drumTrackName in enumerate(drumTrackNames):

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried what you have proposed. It actually cost me an extra line of code
because I have to maintain the index outside of the try.

i = 3
try:
if drumTrackNames:
for j, drumTrackName in enumerate(drumTrackNames):
i = j + 3

self.drumTracks.append(Audio.StreamingSound(self.engine.audio.getChannel(i),
drumTrackName))
i += 1

On Sat, Mar 7, 2015 at 3:18 PM, Matthew Sitton notifications@github.com
wrote:

In fofix/game/Song.py
#33 (comment):

     try:
  •        if drumTrackName:
    
  •            self.drumTrack = Audio.StreamingSound(self.engine.audio.getChannel(3), drumTrackName)
    
  •        if drumTrackNames:
    
  •            for drumTrackName in drumTrackNames:
    

In this loop instead of what you have you could use enumerate()

for i, drumTrackName in enumerate(drumTrackNames):


Reply to this email directly or view it on GitHub
https://github.com/fofix/fofix/pull/33/files#r25999696.

@mdsitton
Copy link
Member

Are you planning to continue working on this?

@justmoen
Copy link
Author

I am done with the prodrums portion. Do you have any idea where to begin
porting to opengles? I have never worked on video programming.
On Mar 15, 2015 3:20 PM, "Matthew Sitton" notifications@github.com wrote:

Are you planning to continue working on this?


Reply to this email directly or view it on GitHub
#33 (comment).

@mdsitton
Copy link
Member

If you are finished i have a few things to mention. First i would move the pro drum option to be a track option in the setlist when your playing drums instead of a character option. I also haven't been able to test it, but does your code force people to play the pro drums notes when in pro mode yet?

Also a port to OpenGL ES 1.0 might be possible, but porting to OpenGL ES 2.0+ would be extremely difficult.

@justmoen
Copy link
Author

It does not force them, it just shows them whether to play a tom or
cymbal. Further work is required to distinguish 8 separate mappings for
drum input.

It should be a player preference. You cannot play prodrums unless the
player has pro drums. The drum track should not determine which drum kit
to use.
On Mar 15, 2015 4:01 PM, "Matthew Sitton" notifications@github.com wrote:

If you are finished i have a few things to mention. First i would move the
pro drum option to be a track option in the setlist when your playing drums
instead of a character option. I also haven't been able to test it, but
does your code force people to play the pro drums notes when in pro mode
yet?

Also a port to OpenGL ES 1.0 might be possible, but porting to OpenGL ES
2.0+ would be extremely difficult.


Reply to this email directly or view it on GitHub
#33 (comment).

@mdsitton
Copy link
Member

I can see why you did it that way, but in the long run i don't think that is the proper place to put it. I'm not going to merge it until i can get some others input on the pull request. I've asked stump to look at the pull request also.

@justmoen
Copy link
Author

It is a step forward. Feel free to change it. I am just enjoying playing
with pro drums for the first time.
On Mar 26, 2015 12:45 PM, "Matthew Sitton" notifications@github.com wrote:

I can see why you did it that way, but in the long run i don't think that
is the proper place to put it. I'm not going to merge it until i can get
some others input on the pull request. I've asked stump to look at the pull
request also.


Reply to this email directly or view it on GitHub
#33 (comment).

@mdsitton
Copy link
Member

We really need to redo the whole controller/input system before any of that can be changed though.

@erodozer
Copy link
Member

erodozer commented Apr 1, 2015

Could you please separate out commits 518b786 and 4ebbe30 into a separate pull request, as their existence is not directly relevant to pro drums, however they do appear to be welcome fixes.

Also clean up commit fca6e24 and 0c54bd3 as commit reverts are not necessary inside of pull requests unless they are a reversion against master.

Aside from that, could you also squash some commits to make the pull request cleaner and overall easier to see the end result of what has actually changed.

@mdsitton
Copy link
Member

I am going to go ahead and close this pull request. Since It has been about 6 months.

@justmoen if you still wish to resubmit it with the feedback @nhydock gave applied feel free. I can take another look at merging it then.

@mdsitton mdsitton closed this Sep 16, 2015
@mdsitton
Copy link
Member

@justmoen if your not planning on pursuing this pull request anymore. Ill probably re implement some of these changes since there looks to be some useful code in this still...

@justmoen
Copy link
Author

Yes, please reuse if you like. Any chance this project will ever compile
for ARM?

Justin
On Oct 25, 2015 11:47 PM, "Matthew Sitton" notifications@github.com wrote:

@justmoen https://github.com/justmoen if your not planning on pursuing
this pull request anymore. Ill probably re implement some of these changes
since there looks to be some useful code in this still...


Reply to this email directly or view it on GitHub
#33 (comment).

@mdsitton
Copy link
Member

I'd certainly like to try to get it working on arm. I could probably get a raspberrypi and try to get things going on it, but even with that the game would need to work under opengl es.

Long term I think it would be cool to make it work on android, but a LOT of stuff has to happen before that happens. (Move away from pygame, figure out python4android, opengl es, etc. ).

So not impossible, might just be a long long time, and considering FoFiX 4.0 has already been in development for 6 years.... yeah really long time lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants