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

Random pattern #32

Merged
merged 2 commits into from
Mar 10, 2014
Merged

Random pattern #32

merged 2 commits into from
Mar 10, 2014

Conversation

p-lambert
Copy link
Contributor

Currently, there are 17 available patterns. The last one, “chevrons” has index 16, thus it will be never randomly picked, as generate_pattern uses the 21th hexadecimal character from the hash (which will be 15, at maximum) to pick up a pattern.

Thank you very much for this gem!

@jasonlong
Copy link
Owner

Wow, unbelievable. I remember counting these towards the end (taking into account the 0-based indexing) and must have miscounted.

At this point, I think I'm more inclined to remove one of the patterns and keep the pattern choice based on a single SHA value. I'd probably choose to remove triangles_rotated since it's very similar to triangles.

@p-lambert Would you like to update your PR to do this? If not, I'm happy to take care of it. We'll need to update the README as well. Either way, thanks for pointing this out!

@p-lambert
Copy link
Contributor Author

@jasonlong, I’ll be glad to help. just to be sure if I got it right: should I leave generate_pattern like it was before (since the map call will be no longer needed) and just remove (including README) the aforementioned pattern?

@jasonlong
Copy link
Owner

Yeah, that's what I was thinking. Thanks!

On Sun, Mar 9, 2014 at 5:27 PM, Pedro Lambert notifications@github.com
wrote:

@jasonlong, I’ll be glad to help. just to be sure if I got it right: should I leave generate_pattern like it was before (since the map call will be no longer needed) and just remove (including README) the aforementioned pattern?

Reply to this email directly or view it on GitHub:
#32 (comment)

@jasonlong
Copy link
Owner

Looks good, thx @p-lambert.

jasonlong pushed a commit that referenced this pull request Mar 10, 2014
@jasonlong jasonlong merged commit 05f56fd into jasonlong:master Mar 10, 2014
btmills added a commit to btmills/geopattern that referenced this pull request Mar 10, 2014
In the previous version, there were 17 patterns, and the last one,
chevrons, was never randomly displayed. This commit removes the
rotatedTriangles pattern and replaces it with chevrons.

Ref: jasonlong/geo_pattern#32
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.

None yet

2 participants