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

Fix non-existing pattern error #11

Merged
merged 2 commits into from
Nov 14, 2015

Conversation

hillerstorm
Copy link
Collaborator

Tested against remember_by_stalker_of_fyllecell303_.xm.

The pattern at the last song position in player.xm.songpats is 26 which
doesn't exist in player.xm.patterns.
This causes p in the following line to return undefined resulting in
multiple errors being logged:

var p = player.xm.patterns[player.cur_pat];
var r = p[player.cur_row];

To jump to the last pattern instantly set a breakpoint in xm.js:play(),
run the following code in the debugger and then resume:

player.cur_songpos=29;player.cur_pat=25;player.cur_row=0

@hillerstorm hillerstorm mentioned this pull request Nov 13, 2015
@a1k0n
Copy link
Owner

a1k0n commented Nov 14, 2015

Ah, that's annoying. Thanks for adding, and nice tests! I am gonna have dinner and take a closer look.

My only concern is that it's an infinite loop if the song has length 0, but it might bail out earlier in that case. Not sure.

@a1k0n
Copy link
Owner

a1k0n commented Nov 14, 2015

This makes sense and looks good. Would you mind adding a test for the song_looppos logic?

@hillerstorm
Copy link
Collaborator Author

Sure, I'll add a test or two and rebase

a1k0n added a commit that referenced this pull request Nov 14, 2015
@a1k0n a1k0n merged commit 0f86a8d into a1k0n:master Nov 14, 2015
@a1k0n
Copy link
Owner

a1k0n commented Nov 14, 2015

Awesome. Thanks!

@hillerstorm hillerstorm deleted the fix_out_of_range_pattern_error branch June 5, 2020 08:59
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