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 #145 #146

Merged
merged 2 commits into from Jun 5, 2018

Conversation

Projects
None yet
2 participants
@NQNStudios
Copy link
Contributor

commented Jun 5, 2018

I figured this might be a good first issue to tackle. After some testing, I found that the problem was a silent error in hashing the path of the sequence being shuffled. All hashes were evaluating to 0 because charCodeAt[i] attempts to array-index the function, charCodeAt(), returning undefined and making these line fail to sum anything:

for (var i = 0, l = seqPathStr.length; i < l; i++) {
    sequenceHash += seqPathStr.charCodeAt[i] || 0;
}

Which is why the same length sequence always ended up producing the same seed.

@y-lohse

This comment has been minimized.

Copy link
Owner

commented Jun 5, 2018

That's fantastic, thank you very much! Absolutely looks like that would be the problem.

Only thing before we merge, can you please discard the changes made to the ink.js template files? I'll update them once we publish a new version :)

@NQNStudios

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2018

Done. Yeah, I figured I had done something wrong when my PR had >200 line changes after I only changed 2 characters, haha!

@y-lohse

This comment has been minimized.

Copy link
Owner

commented Jun 5, 2018

Yeah no worries haha. Thanks again!

@y-lohse y-lohse merged commit eb235b4 into y-lohse:master Jun 5, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@NQNStudios NQNStudios deleted the NQNStudios:fix-shuffle-lists branch Jul 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.