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

MC7000: Fix off-by-one indexing in the controller script #4902

Merged
merged 1 commit into from
Aug 21, 2022

Conversation

fwcd
Copy link
Member

@fwcd fwcd commented Aug 19, 2022

Almost the entire script indexes arrays with deckNumber (which ranges from 1 to 4) rather than deckOffset (which ranges from 0 to 3). This works since JavaScript implicitly lets the user assign to out-of-bounds indices:

var xs = [0, 1, 2, 3];
xs[4] = 4;
// xs == [0, 1, 2, 3, 4]

Since most of the script doesn't care about the initial values, this bug doesn't affect the actual functionality much, since JS is happy to use indices 1-4 instead and ignore the value at index 0.

This behavior is subtle and unintuitive, especially with regards to the default values, therefore this PR fixes the script to use the correct indexing (from 0 to 3).

@ferranpujolcamins
Copy link
Contributor

Hi @fwcd , thanks for improving this details! I agree it's clearer now.

LGTM

@ferranpujolcamins ferranpujolcamins merged commit 4d96124 into mixxxdj:2.3 Aug 21, 2022
@fwcd fwcd deleted the mc7000-indexing-fixes branch August 21, 2022 15:46
@holopansel holopansel mentioned this pull request Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants