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

Change straight algorithm #389

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Change straight algorithm #389

wants to merge 1 commit into from

Conversation

Aurelius7309
Copy link
Member

The current algorithm for straights is pretty bad with custom ranks that have split paths. This proposes a new one that works correctly in cases where the previous one failed.
Example: Given ranks 10->11->12->13 with SMODS.Ranks['10'].next = {'Jack', '11'}.

  • 9,10,J,Q is a valid 4-finger straight
  • 9,10,J,11,12 should also be a valid 4-finger straight but isn't because the current algorithm gets trapped in the first path

Originally tried to base an implementation of quantum ranks on this but realized this would be inefficient as candidate straights can be invalidated at each step and checking a given hand with quantum ranks for containing a straight is exponential in the length of the straight.

Also builds prev tables in ranks when injected. Not used here but figured it might be useful to have

@Aurelius7309
Copy link
Member Author

Performance is about the same, shortcut is slower with large amounts of cards selected. Measured around 2.5ms with shortcut vs. 0.8ms without, while the previous implementation takes barely any hit at all

@DigitalDetective47
Copy link
Contributor

are prev tables built automatically by reversing the next tables, or do they have to be specified manually?

@Aurelius7309
Copy link
Member Author

Builds prev tables automatically, but I didn't end up using them here

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