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

Add two pingpong sequence modes for whitewhale. #4

Closed
wants to merge 1 commit into from

Conversation

@GoneCaving
Copy link
Contributor

@GoneCaving GoneCaving commented Oct 24, 2016

Per this thread:
http://llllllll.co/t/white-whale-ping-pong-playback-mode/4816/41

mPing: 1,2,3,4,3,2,1,2...
mPingRep: 1,2,3,4,4,3,2,1,1,...

Seems to be working, pattern & series mode checked. I've not tested on a smaller grid, and i think there may be a clash with the keys from the left hand side? Happy to make any further changes to address this? (drop one mode?). It would help to know what the "FIXME" around line 882 relate to.

w.wp[pattern].step_mode = mForward;
else if(x == LENGTH-1)
}

This comment has been minimized.

@genericwoods

genericwoods Oct 25, 2016

Minor: it looks like the alignment of these closing braces is kind of off.

This comment has been minimized.

@GoneCaving

GoneCaving Oct 25, 2016
Author Contributor

Really? Looks ok here.

This comment has been minimized.

@genericwoods

genericwoods Oct 25, 2016

Just to make sure we’re seeing the same thing:
screen shot 2016-10-25 at 11 25 48 am

Also appears to be off on lines 310-343

w.wp[pattern].step_mode = mPing;
w.wp[pattern].ping_dir = mPingFwd;
}
else if(x == LENGTH-5) {

This comment has been minimized.

@genericwoods

genericwoods Oct 25, 2016

I mentioned this in our DM convo, but I think we can clean up some of the existing logic here (that’s made more confusing with each playback mode added) by using if(x == LENGTH-mPingRep) here (we may need to explicitly assign numerical values to each of the enums, forgive me that I’m a bit rusty on vanilla C programming)

This comment has been minimized.

@GoneCaving

GoneCaving Oct 25, 2016
Author Contributor

The enums are 0 up, so should be easy enough. If we don't mind the overhead of setting the mPingFwd regardless of mode, we might be able to combine all of the modes into a single block (LENGTH-x).

This comment has been minimized.

@genericwoods

genericwoods Oct 25, 2016

Even better, that seems like a good idea to me

@genericwoods
Copy link

@genericwoods genericwoods commented Oct 25, 2016

To capture my response on the Lines forum, I think dropping the repeating mode makes the most sense (retains consistency with the larger grid, and keeps the code simpler)

@GoneCaving
Copy link
Contributor Author

@GoneCaving GoneCaving commented Oct 25, 2016

OK, abandoning this, and will adopt the comments above (and hopefully address the tabs vs spaces issue).

@GoneCaving GoneCaving closed this Oct 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants