-
-
Notifications
You must be signed in to change notification settings - Fork 200
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
add Gale #724
add Gale #724
Conversation
Thanks. I have some doubts though whether this is worth it, considering that this seems like a very special case and rather many new config values for a single variant, so I currently slightly tend towards not merging this. However, supporting file wildcards and rank 10 squares in the bitboard parsing of course would be useful, so that might be worth an independent PR. I would just suggest to use a |
Well, your project so up to you. The different wildcard for files looks doable. |
Well, I flipped the order in the commit message, but you know what I mean |
Ok, what about instead of four configuration rules, there was a single ConnectionRule config where different connection rules were added as needed.
Any other misc connection rule that was mutually exclusive with the other connection rules could be added here as well. This config is just an example, I am still only planning to add Gale in this Request. |
I prefer your current approach compared to the new suggestion, it feels more transparent and flexible. I think there is a bug in your current implementation though if I understand it correctly. It does not seem to check whether the starting zone is occupied, it just loops over all starting squares. That is also where I think the implementation could potentially be much more efficient. E.g., one could first determine the two And one could then beyond that probably also check the connectivity on a Bitboard level, basically the same as you do now, just not doing it per square, like
Not necessarily the best implementation, but might be a rough direction one could go towards to speed it up. Edit: Doing the same from both sides and doing a meet in the middle might potentially be more efficient, but haven't really thought much about it. |
|
Thanks, it looks good to me now. However, after merging the other PR this now seems to have merge conflicts to resolve. |
I think this solves the conflicts. |
No description provided.