Progress on Loki

[niel5946]

Making moves
I have finished writing the methods, position::make_move() and position::undo_move(). They can be found here.
No optimizations have been made yet, so I think it'll be possible to squeeze more performance out of them, but right now I just want to make sure they are bug free. Therefore, this day will be spent writing a perft method and testing different positions.
Overall, I am happy with their implementations. It's nothing special, but the code is cleaner and more compact than the old version

[niel5946]

First succesful perft results!
I have spent the entire day fixing bugs in the Loki-refactor while perft testing, and I just got it to work for all positions on the CPW Perft Results page!
One bug that took me a while to figure out was that since I wanted to reduce copying of move-lists, I had my move-generator class hold a move_list_t and return a reference when having generated the moves for the position. This turned out to (of course) get over-written whenever perft made a move, generated all moves for the new position, and then un-made said move. At that point, the move-list would be useless for the first perft caller, since it now was a reference to the child-node's move-list.
My solution was to just have an array of move_list_t's, and then write to/return a reference to one of those based on the position's ply - thus, only sibling nodes share a move_list_t& now, which works for single-threaded code. In case of multiple threads, a move_generator per position object will of course be necessary.

I haven't perft tested enough positions to be confident in the move generator, but for the initial position to depth 6, the refactored code manages ~33Mnps on my Ryzen 9 5900X, whereas the Loki 3.5.0-x64 release binary only got ~22Mnps. I consider this a great success!

[algerbrex]

First succesful perft results!
I have spent the entire day fixing bugs in the Loki-refactor while perft testing, and I just got it to work for all positions on the CPW Perft Results page!
One bug that took me a while to figure out was that since I wanted to reduce copying of move-lists, I had my move-generator class hold a move_list_t and return a reference when having generated the moves for the position. This turned out to (of course) get over-written whenever perft made a move, generated all moves for the new position, and then un-made said move. At that point, the move-list would be useless for the first perft caller, since it now was a reference to the child-node's move-list.
My solution was to just have an array of move_list_t's, and then write to/return a reference to one of those based on the position's ply - thus, only sibling nodes share a move_list_t& now, which works for single-threaded code. In case of multiple threads, a move_generator per position object will of course be necessary.

I haven't perft tested enough positions to be confident in the move generator, but for the initial position to depth 6, the refactored code manages ~33Mnps on my Ryzen 9 5900X, whereas the Loki 3.5.0-x64 release binary only got ~22Mnps. I consider this a great success!

Congrulations Niels! Loki has been a great inspiration for my own engine and I look forward to see how you'll continue to improve, and I look forward to when I can find the time and motivation to do my own "great refactoring" for Blunder.

I haven't had time to look through your code well yet, but what did you find to be the biggest efficiency improvements in your perft results? Of course perft speed is still tangential to the primary purpose of a chess engine, but I'm still not convinced I've made Blunder's move generator as efficient as I could make it, and that this is primarily due to me not being a very good programmer.

[niel5946]

Congrulations Niels! Loki has been a great inspiration for my own engine and I look forward to see how you'll continue to improve, and I look forward to when I can find the time and motivation to do my own "great refactoring" for Blunder.

Thank you very much!
It's always great to hear that one's work does not just benefit oneself

I haven't had time to look through your code well yet, but what did you find to be the biggest efficiency improvements in your perft results? Of course perft speed is still tangential to the primary purpose of a chess engine, but I'm still not convinced I've made Blunder's move generator as efficient as I could make it, and that this is primarily due to me not being a very good programmer.

At the current moment, I haven't really done a bigger comparison between the old code and the refactor, other than checking NPS. Despite of that, I am pretty sure it is one or both of the below improvements:

• Reducing the amount of variables created. This is especially true for the move lists in move_generator and the move stack (where the position's history is stored) in position. In both of these examples, I now write directly to a pre-allocated structure instead of creating a new one. A small struct allocation - even when being stack-allocated - is of course not a lot, but when speaking about millions of these a second, I am sure it makes a big difference.
• Making it easier for the compiler to inline and optimize. This is just a lot of small code improvements by using templates, constexpr and noexcept methods.

One thing that I just realized, is that these NPS numbers aren't 100% comparable, since the old code also keeps track of and updates the position's zobrist hash, which the new one doesn't yet. In spite of this, I am still confident that the refactored code is faster (XOR'ing a 64-bit number a couple of times when making a move probably wont make a world of difference), but the perft-NPS will be more precise when hashkey-updates are introduced. I plan to implement this in one of the following days.

[tcusr]

i've been browsing your code quickly and the only advice i have for you is try not to use a C++ feature just because you can, for example here a simple ternary operator is enough. also the extensive use of smart pointers is a bit weird (at least for me, if i'm not missing anything).
but nonetheless the code looks much better, good luck.

[dangi12012]

And now more PERFT test positions for you:
I used this to uncover many many many more bugs in my code before it was ready:

Code: Select all

std::string Positions[] =
	{
		std::string("rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1 ;D1 20 ;D2 400 ;D3 8902 ;D4 197281 ;D5 4865609 ;D6 119060324"),
		std::string("r3k2r/p1ppqpb1/bn2pnp1/3PN3/1p2P3/2N2Q1p/PPPBBPPP/R3K2R w KQkq - 0 1 ;D1 48 ;D2 2039 ;D3 97862 ;D4 4085603 ;D5 193690690"),
		std::string("4k3/8/8/8/8/8/8/4K2R w K - 0 1 ;D1 15 ;D2 66 ;D3 1197 ;D4 7059 ;D5 133987 ;D6 764643"),
		std::string("4k3/8/8/8/8/8/8/R3K3 w Q - 0 1 ;D1 16 ;D2 71 ;D3 1287 ;D4 7626 ;D5 145232 ;D6 846648"),
		std::string("4k2r/8/8/8/8/8/8/4K3 w k - 0 1 ;D1 5 ;D2 75 ;D3 459 ;D4 8290 ;D5 47635 ;D6 899442"),
		std::string("r3k3/8/8/8/8/8/8/4K3 w q - 0 1 ;D1 5 ;D2 80 ;D3 493 ;D4 8897 ;D5 52710 ;D6 1001523"),
		std::string("4k3/8/8/8/8/8/8/R3K2R w KQ - 0 1 ;D1 26 ;D2 112 ;D3 3189 ;D4 17945 ;D5 532933 ;D6 2788982"),
		std::string("r3k2r/8/8/8/8/8/8/4K3 w kq - 0 1 ;D1 5 ;D2 130 ;D3 782 ;D4 22180 ;D5 118882 ;D6 3517770"),
		std::string("8/8/8/8/8/8/6k1/4K2R w K - 0 1 ;D1 12 ;D2 38 ;D3 564 ;D4 2219 ;D5 37735 ;D6 185867"),
		std::string("8/8/8/8/8/8/1k6/R3K3 w Q - 0 1 ;D1 15 ;D2 65 ;D3 1018 ;D4 4573 ;D5 80619 ;D6 413018"),
		std::string("4k2r/6K1/8/8/8/8/8/8 w k - 0 1 ;D1 3 ;D2 32 ;D3 134 ;D4 2073 ;D5 10485 ;D6 179869"),
		std::string("r3k3/1K6/8/8/8/8/8/8 w q - 0 1 ;D1 4 ;D2 49 ;D3 243 ;D4 3991 ;D5 20780 ;D6 367724"),
		std::string("r3k2r/8/8/8/8/8/8/R3K2R w KQkq - 0 1 ;D1 26 ;D2 568 ;D3 13744 ;D4 314346 ;D5 7594526 ;D6 179862938"),
		std::string("r3k2r/8/8/8/8/8/8/1R2K2R w Kkq - 0 1 ;D1 25 ;D2 567 ;D3 14095 ;D4 328965 ;D5 8153719 ;D6 195629489"),
		std::string("r3k2r/8/8/8/8/8/8/2R1K2R w Kkq - 0 1 ;D1 25 ;D2 548 ;D3 13502 ;D4 312835 ;D5 7736373 ;D6 184411439"),
		std::string("r3k2r/8/8/8/8/8/8/R3K1R1 w Qkq - 0 1 ;D1 25 ;D2 547 ;D3 13579 ;D4 316214 ;D5 7878456 ;D6 189224276"),
		std::string("1r2k2r/8/8/8/8/8/8/R3K2R w KQk - 0 1 ;D1 26 ;D2 583 ;D3 14252 ;D4 334705 ;D5 8198901 ;D6 198328929"),
		std::string("2r1k2r/8/8/8/8/8/8/R3K2R w KQk - 0 1 ;D1 25 ;D2 560 ;D3 13592 ;D4 317324 ;D5 7710115 ;D6 185959088"),
		std::string("r3k1r1/8/8/8/8/8/8/R3K2R w KQq - 0 1 ;D1 25 ;D2 560 ;D3 13607 ;D4 320792 ;D5 7848606 ;D6 190755813"),
		std::string("4k3/8/8/8/8/8/8/4K2R b K - 0 1 ;D1 5 ;D2 75 ;D3 459 ;D4 8290 ;D5 47635 ;D6 899442"),
		std::string("4k3/8/8/8/8/8/8/R3K3 b Q - 0 1 ;D1 5 ;D2 80 ;D3 493 ;D4 8897 ;D5 52710 ;D6 1001523"),
		std::string("4k2r/8/8/8/8/8/8/4K3 b k - 0 1 ;D1 15 ;D2 66 ;D3 1197 ;D4 7059 ;D5 133987 ;D6 764643"),
		std::string("r3k3/8/8/8/8/8/8/4K3 b q - 0 1 ;D1 16 ;D2 71 ;D3 1287 ;D4 7626 ;D5 145232 ;D6 846648"),
		std::string("4k3/8/8/8/8/8/8/R3K2R b KQ - 0 1 ;D1 5 ;D2 130 ;D3 782 ;D4 22180 ;D5 118882 ;D6 3517770"),
		std::string("r3k2r/8/8/8/8/8/8/4K3 b kq - 0 1 ;D1 26 ;D2 112 ;D3 3189 ;D4 17945 ;D5 532933 ;D6 2788982"),
		std::string("8/8/8/8/8/8/6k1/4K2R b K - 0 1 ;D1 3 ;D2 32 ;D3 134 ;D4 2073 ;D5 10485 ;D6 179869"),
		std::string("8/8/8/8/8/8/1k6/R3K3 b Q - 0 1 ;D1 4 ;D2 49 ;D3 243 ;D4 3991 ;D5 20780 ;D6 367724"),
		std::string("4k2r/6K1/8/8/8/8/8/8 b k - 0 1 ;D1 12 ;D2 38 ;D3 564 ;D4 2219 ;D5 37735 ;D6 185867"),
		std::string("r3k3/1K6/8/8/8/8/8/8 b q - 0 1 ;D1 15 ;D2 65 ;D3 1018 ;D4 4573 ;D5 80619 ;D6 413018"),
		std::string("r3k2r/8/8/8/8/8/8/R3K2R b KQkq - 0 1 ;D1 26 ;D2 568 ;D3 13744 ;D4 314346 ;D5 7594526 ;D6 179862938"),
		std::string("r3k2r/8/8/8/8/8/8/1R2K2R b Kkq - 0 1 ;D1 26 ;D2 583 ;D3 14252 ;D4 334705 ;D5 8198901 ;D6 198328929"),
		std::string("r3k2r/8/8/8/8/8/8/2R1K2R b Kkq - 0 1 ;D1 25 ;D2 560 ;D3 13592 ;D4 317324 ;D5 7710115 ;D6 185959088"),
		std::string("r3k2r/8/8/8/8/8/8/R3K1R1 b Qkq - 0 1 ;D1 25 ;D2 560 ;D3 13607 ;D4 320792 ;D5 7848606 ;D6 190755813"),
		std::string("1r2k2r/8/8/8/8/8/8/R3K2R b KQk - 0 1 ;D1 25 ;D2 567 ;D3 14095 ;D4 328965 ;D5 8153719 ;D6 195629489"),
		std::string("2r1k2r/8/8/8/8/8/8/R3K2R b KQk - 0 1 ;D1 25 ;D2 548 ;D3 13502 ;D4 312835 ;D5 7736373 ;D6 184411439"),
		std::string("r3k1r1/8/8/8/8/8/8/R3K2R b KQq - 0 1 ;D1 25 ;D2 547 ;D3 13579 ;D4 316214 ;D5 7878456 ;D6 189224276"),
		std::string("8/1n4N1/2k5/8/8/5K2/1N4n1/8 w - - 0 1 ;D1 14 ;D2 195 ;D3 2760 ;D4 38675 ;D5 570726 ;D6 8107539"),
		std::string("8/1k6/8/5N2/8/4n3/8/2K5 w - - 0 1 ;D1 11 ;D2 156 ;D3 1636 ;D4 20534 ;D5 223507 ;D6 2594412"),
		std::string("8/8/4k3/3Nn3/3nN3/4K3/8/8 w - - 0 1 ;D1 19 ;D2 289 ;D3 4442 ;D4 73584 ;D5 1198299 ;D6 19870403"),
		std::string("K7/8/2n5/1n6/8/8/8/k6N w - - 0 1 ;D1 3 ;D2 51 ;D3 345 ;D4 5301 ;D5 38348 ;D6 588695"),
		std::string("k7/8/2N5/1N6/8/8/8/K6n w - - 0 1 ;D1 17 ;D2 54 ;D3 835 ;D4 5910 ;D5 92250 ;D6 688780"),
		std::string("8/1n4N1/2k5/8/8/5K2/1N4n1/8 b - - 0 1 ;D1 15 ;D2 193 ;D3 2816 ;D4 40039 ;D5 582642 ;D6 8503277"),
		std::string("8/1k6/8/5N2/8/4n3/8/2K5 b - - 0 1 ;D1 16 ;D2 180 ;D3 2290 ;D4 24640 ;D5 288141 ;D6 3147566"),
		std::string("8/8/3K4/3Nn3/3nN3/4k3/8/8 b - - 0 1 ;D1 4 ;D2 68 ;D3 1118 ;D4 16199 ;D5 281190 ;D6 4405103"),
		std::string("K7/8/2n5/1n6/8/8/8/k6N b - - 0 1 ;D1 17 ;D2 54 ;D3 835 ;D4 5910 ;D5 92250 ;D6 688780"),
		std::string("k7/8/2N5/1N6/8/8/8/K6n b - - 0 1 ;D1 3 ;D2 51 ;D3 345 ;D4 5301 ;D5 38348 ;D6 588695"),
		std::string("B6b/8/8/8/2K5/4k3/8/b6B w - - 0 1 ;D1 17 ;D2 278 ;D3 4607 ;D4 76778 ;D5 1320507 ;D6 22823890"),
		std::string("8/8/1B6/7b/7k/8/2B1b3/7K w - - 0 1 ;D1 21 ;D2 316 ;D3 5744 ;D4 93338 ;D5 1713368 ;D6 28861171"),
		std::string("k7/B7/1B6/1B6/8/8/8/K6b w - - 0 1 ;D1 21 ;D2 144 ;D3 3242 ;D4 32955 ;D5 787524 ;D6 7881673"),
		std::string("K7/b7/1b6/1b6/8/8/8/k6B w - - 0 1 ;D1 7 ;D2 143 ;D3 1416 ;D4 31787 ;D5 310862 ;D6 7382896"),
		std::string("B6b/8/8/8/2K5/5k2/8/b6B b - - 0 1 ;D1 6 ;D2 106 ;D3 1829 ;D4 31151 ;D5 530585 ;D6 9250746"),
		std::string("8/8/1B6/7b/7k/8/2B1b3/7K b - - 0 1 ;D1 17 ;D2 309 ;D3 5133 ;D4 93603 ;D5 1591064 ;D6 29027891"),
		std::string("k7/B7/1B6/1B6/8/8/8/K6b b - - 0 1 ;D1 7 ;D2 143 ;D3 1416 ;D4 31787 ;D5 310862 ;D6 7382896"),
		std::string("K7/b7/1b6/1b6/8/8/8/k6B b - - 0 1 ;D1 21 ;D2 144 ;D3 3242 ;D4 32955 ;D5 787524 ;D6 7881673"),
		std::string("7k/RR6/8/8/8/8/rr6/7K w - - 0 1 ;D1 19 ;D2 275 ;D3 5300 ;D4 104342 ;D5 2161211 ;D6 44956585"),
		std::string("R6r/8/8/2K5/5k2/8/8/r6R w - - 0 1 ;D1 36 ;D2 1027 ;D3 29215 ;D4 771461 ;D5 20506480 ;D6 525169084"),
		std::string("7k/RR6/8/8/8/8/rr6/7K b - - 0 1 ;D1 19 ;D2 275 ;D3 5300 ;D4 104342 ;D5 2161211 ;D6 44956585"),
		std::string("R6r/8/8/2K5/5k2/8/8/r6R b - - 0 1 ;D1 36 ;D2 1027 ;D3 29227 ;D4 771368 ;D5 20521342 ;D6 524966748"),
		std::string("6kq/8/8/8/8/8/8/7K w - - 0 1 ;D1 2 ;D2 36 ;D3 143 ;D4 3637 ;D5 14893 ;D6 391507"),
		std::string("6KQ/8/8/8/8/8/8/7k b - - 0 1 ;D1 2 ;D2 36 ;D3 143 ;D4 3637 ;D5 14893 ;D6 391507"),
		std::string("K7/8/8/3Q4/4q3/8/8/7k w - - 0 1 ;D1 6 ;D2 35 ;D3 495 ;D4 8349 ;D5 166741 ;D6 3370175"),
		std::string("6qk/8/8/8/8/8/8/7K b - - 0 1 ;D1 22 ;D2 43 ;D3 1015 ;D4 4167 ;D5 105749 ;D6 419369"),
		std::string("6KQ/8/8/8/8/8/8/7k b - - 0 1 ;D1 2 ;D2 36 ;D3 143 ;D4 3637 ;D5 14893 ;D6 391507"),
		std::string("K7/8/8/3Q4/4q3/8/8/7k b - - 0 1 ;D1 6 ;D2 35 ;D3 495 ;D4 8349 ;D5 166741 ;D6 3370175"),
		std::string("8/8/8/8/8/K7/P7/k7 w - - 0 1 ;D1 3 ;D2 7 ;D3 43 ;D4 199 ;D5 1347 ;D6 6249"),
		std::string("8/8/8/8/8/7K/7P/7k w - - 0 1 ;D1 3 ;D2 7 ;D3 43 ;D4 199 ;D5 1347 ;D6 6249"),
		std::string("K7/p7/k7/8/8/8/8/8 w - - 0 1 ;D1 1 ;D2 3 ;D3 12 ;D4 80 ;D5 342 ;D6 2343"),
		std::string("7K/7p/7k/8/8/8/8/8 w - - 0 1 ;D1 1 ;D2 3 ;D3 12 ;D4 80 ;D5 342 ;D6 2343"),
		std::string("8/2k1p3/3pP3/3P2K1/8/8/8/8 w - - 0 1 ;D1 7 ;D2 35 ;D3 210 ;D4 1091 ;D5 7028 ;D6 34834"),
		std::string("8/8/8/8/8/K7/P7/k7 b - - 0 1 ;D1 1 ;D2 3 ;D3 12 ;D4 80 ;D5 342 ;D6 2343"),
		std::string("8/8/8/8/8/7K/7P/7k b - - 0 1 ;D1 1 ;D2 3 ;D3 12 ;D4 80 ;D5 342 ;D6 2343"),
		std::string("K7/p7/k7/8/8/8/8/8 b - - 0 1 ;D1 3 ;D2 7 ;D3 43 ;D4 199 ;D5 1347 ;D6 6249"),
		std::string("7K/7p/7k/8/8/8/8/8 b - - 0 1 ;D1 3 ;D2 7 ;D3 43 ;D4 199 ;D5 1347 ;D6 6249"),
		std::string("8/2k1p3/3pP3/3P2K1/8/8/8/8 b - - 0 1 ;D1 5 ;D2 35 ;D3 182 ;D4 1091 ;D5 5408 ;D6 34822"),
		std::string("8/8/8/8/8/4k3/4P3/4K3 w - - 0 1 ;D1 2 ;D2 8 ;D3 44 ;D4 282 ;D5 1814 ;D6 11848"),
		std::string("4k3/4p3/4K3/8/8/8/8/8 b - - 0 1 ;D1 2 ;D2 8 ;D3 44 ;D4 282 ;D5 1814 ;D6 11848"),
		std::string("8/8/7k/7p/7P/7K/8/8 w - - 0 1 ;D1 3 ;D2 9 ;D3 57 ;D4 360 ;D5 1969 ;D6 10724"),
		std::string("8/8/k7/p7/P7/K7/8/8 w - - 0 1 ;D1 3 ;D2 9 ;D3 57 ;D4 360 ;D5 1969 ;D6 10724"),
		std::string("8/8/3k4/3p4/3P4/3K4/8/8 w - - 0 1 ;D1 5 ;D2 25 ;D3 180 ;D4 1294 ;D5 8296 ;D6 53138"),
		std::string("8/3k4/3p4/8/3P4/3K4/8/8 w - - 0 1 ;D1 8 ;D2 61 ;D3 483 ;D4 3213 ;D5 23599 ;D6 157093"),
		std::string("8/8/3k4/3p4/8/3P4/3K4/8 w - - 0 1 ;D1 8 ;D2 61 ;D3 411 ;D4 3213 ;D5 21637 ;D6 158065"),
		std::string("k7/8/3p4/8/3P4/8/8/7K w - - 0 1 ;D1 4 ;D2 15 ;D3 90 ;D4 534 ;D5 3450 ;D6 20960"),
		std::string("8/8/7k/7p/7P/7K/8/8 b - - 0 1 ;D1 3 ;D2 9 ;D3 57 ;D4 360 ;D5 1969 ;D6 10724"),
		std::string("8/8/k7/p7/P7/K7/8/8 b - - 0 1 ;D1 3 ;D2 9 ;D3 57 ;D4 360 ;D5 1969 ;D6 10724"),
		std::string("8/8/3k4/3p4/3P4/3K4/8/8 b - - 0 1 ;D1 5 ;D2 25 ;D3 180 ;D4 1294 ;D5 8296 ;D6 53138"),
		std::string("8/3k4/3p4/8/3P4/3K4/8/8 b - - 0 1 ;D1 8 ;D2 61 ;D3 411 ;D4 3213 ;D5 21637 ;D6 158065"),
		std::string("8/8/3k4/3p4/8/3P4/3K4/8 b - - 0 1 ;D1 8 ;D2 61 ;D3 483 ;D4 3213 ;D5 23599 ;D6 157093"),
		std::string("k7/8/3p4/8/3P4/8/8/7K b - - 0 1 ;D1 4 ;D2 15 ;D3 89 ;D4 537 ;D5 3309 ;D6 21104"),
		std::string("7k/3p4/8/8/3P4/8/8/K7 w - - 0 1 ;D1 4 ;D2 19 ;D3 117 ;D4 720 ;D5 4661 ;D6 32191"),
		std::string("7k/8/8/3p4/8/8/3P4/K7 w - - 0 1 ;D1 5 ;D2 19 ;D3 116 ;D4 716 ;D5 4786 ;D6 30980"),
		std::string("k7/8/8/7p/6P1/8/8/K7 w - - 0 1 ;D1 5 ;D2 22 ;D3 139 ;D4 877 ;D5 6112 ;D6 41874"),
		std::string("k7/8/7p/8/8/6P1/8/K7 w - - 0 1 ;D1 4 ;D2 16 ;D3 101 ;D4 637 ;D5 4354 ;D6 29679"),
		std::string("k7/8/8/6p1/7P/8/8/K7 w - - 0 1 ;D1 5 ;D2 22 ;D3 139 ;D4 877 ;D5 6112 ;D6 41874"),
		std::string("k7/8/6p1/8/8/7P/8/K7 w - - 0 1 ;D1 4 ;D2 16 ;D3 101 ;D4 637 ;D5 4354 ;D6 29679"),
		std::string("k7/8/8/3p4/4p3/8/8/7K w - - 0 1 ;D1 3 ;D2 15 ;D3 84 ;D4 573 ;D5 3013 ;D6 22886"),
		std::string("k7/8/3p4/8/8/4P3/8/7K w - - 0 1 ;D1 4 ;D2 16 ;D3 101 ;D4 637 ;D5 4271 ;D6 28662"),
		std::string("7k/3p4/8/8/3P4/8/8/K7 b - - 0 1 ;D1 5 ;D2 19 ;D3 117 ;D4 720 ;D5 5014 ;D6 32167"),
		std::string("7k/8/8/3p4/8/8/3P4/K7 b - - 0 1 ;D1 4 ;D2 19 ;D3 117 ;D4 712 ;D5 4658 ;D6 30749"),
		std::string("k7/8/8/7p/6P1/8/8/K7 b - - 0 1 ;D1 5 ;D2 22 ;D3 139 ;D4 877 ;D5 6112 ;D6 41874"),
		std::string("k7/8/7p/8/8/6P1/8/K7 b - - 0 1 ;D1 4 ;D2 16 ;D3 101 ;D4 637 ;D5 4354 ;D6 29679"),
		std::string("k7/8/8/6p1/7P/8/8/K7 b - - 0 1 ;D1 5 ;D2 22 ;D3 139 ;D4 877 ;D5 6112 ;D6 41874"),
		std::string("k7/8/6p1/8/8/7P/8/K7 b - - 0 1 ;D1 4 ;D2 16 ;D3 101 ;D4 637 ;D5 4354 ;D6 29679"),
		std::string("k7/8/8/3p4/4p3/8/8/7K b - - 0 1 ;D1 5 ;D2 15 ;D3 102 ;D4 569 ;D5 4337 ;D6 22579"),
		std::string("k7/8/3p4/8/8/4P3/8/7K b - - 0 1 ;D1 4 ;D2 16 ;D3 101 ;D4 637 ;D5 4271 ;D6 28662"),
		std::string("7k/8/8/p7/1P6/8/8/7K w - - 0 1 ;D1 5 ;D2 22 ;D3 139 ;D4 877 ;D5 6112 ;D6 41874"),
		std::string("7k/8/p7/8/8/1P6/8/7K w - - 0 1 ;D1 4 ;D2 16 ;D3 101 ;D4 637 ;D5 4354 ;D6 29679"),
		std::string("7k/8/8/1p6/P7/8/8/7K w - - 0 1 ;D1 5 ;D2 22 ;D3 139 ;D4 877 ;D5 6112 ;D6 41874"),
		std::string("7k/8/1p6/8/8/P7/8/7K w - - 0 1 ;D1 4 ;D2 16 ;D3 101 ;D4 637 ;D5 4354 ;D6 29679"),
		std::string("k7/7p/8/8/8/8/6P1/K7 w - - 0 1 ;D1 5 ;D2 25 ;D3 161 ;D4 1035 ;D5 7574 ;D6 55338"),
		std::string("k7/6p1/8/8/8/8/7P/K7 w - - 0 1 ;D1 5 ;D2 25 ;D3 161 ;D4 1035 ;D5 7574 ;D6 55338"),
		std::string("3k4/3pp3/8/8/8/8/3PP3/3K4 w - - 0 1 ;D1 7 ;D2 49 ;D3 378 ;D4 2902 ;D5 24122 ;D6 199002"),
		std::string("7k/8/8/p7/1P6/8/8/7K b - - 0 1 ;D1 5 ;D2 22 ;D3 139 ;D4 877 ;D5 6112 ;D6 41874"),
		std::string("7k/8/p7/8/8/1P6/8/7K b - - 0 1 ;D1 4 ;D2 16 ;D3 101 ;D4 637 ;D5 4354 ;D6 29679"),
		std::string("7k/8/8/1p6/P7/8/8/7K b - - 0 1 ;D1 5 ;D2 22 ;D3 139 ;D4 877 ;D5 6112 ;D6 41874"),
		std::string("7k/8/1p6/8/8/P7/8/7K b - - 0 1 ;D1 4 ;D2 16 ;D3 101 ;D4 637 ;D5 4354 ;D6 29679"),
		std::string("k7/7p/8/8/8/8/6P1/K7 b - - 0 1 ;D1 5 ;D2 25 ;D3 161 ;D4 1035 ;D5 7574 ;D6 55338"),
		std::string("k7/6p1/8/8/8/8/7P/K7 b - - 0 1 ;D1 5 ;D2 25 ;D3 161 ;D4 1035 ;D5 7574 ;D6 55338"),
		std::string("3k4/3pp3/8/8/8/8/3PP3/3K4 b - - 0 1 ;D1 7 ;D2 49 ;D3 378 ;D4 2902 ;D5 24122 ;D6 199002"),
		std::string("8/Pk6/8/8/8/8/6Kp/8 w - - 0 1 ;D1 11 ;D2 97 ;D3 887 ;D4 8048 ;D5 90606 ;D6 1030499"),
		std::string("n1n5/1Pk5/8/8/8/8/5Kp1/5N1N w - - 0 1 ;D1 24 ;D2 421 ;D3 7421 ;D4 124608 ;D5 2193768 ;D6 37665329"),
		std::string("8/PPPk4/8/8/8/8/4Kppp/8 w - - 0 1 ;D1 18 ;D2 270 ;D3 4699 ;D4 79355 ;D5 1533145 ;D6 28859283"),
		std::string("n1n5/PPPk4/8/8/8/8/4Kppp/5N1N w - - 0 1 ;D1 24 ;D2 496 ;D3 9483 ;D4 182838 ;D5 3605103 ;D6 71179139"),
		std::string("8/Pk6/8/8/8/8/6Kp/8 b - - 0 1 ;D1 11 ;D2 97 ;D3 887 ;D4 8048 ;D5 90606 ;D6 1030499"),
		std::string("n1n5/1Pk5/8/8/8/8/5Kp1/5N1N b - - 0 1 ;D1 24 ;D2 421 ;D3 7421 ;D4 124608 ;D5 2193768 ;D6 37665329"),
		std::string("8/PPPk4/8/8/8/8/4Kppp/8 b - - 0 1 ;D1 18 ;D2 270 ;D3 4699 ;D4 79355 ;D5 1533145 ;D6 28859283"),
		std::string("n1n5/PPPk4/8/8/8/8/4Kppp/5N1N b - - 0 1 ;D1 24 ;D2 496 ;D3 9483 ;D4 182838 ;D5 3605103 ;D6 71179139"),
		std::string("8/2p5/3p4/KP5r/1R3p1k/8/4P1P1/8 w - - 0 1 ;D1 14 ;D2 191 ;D3 2812 ;D4 43238 ;D5 674624 ;D6 11030083"),
		std::string("rnbqkb1r/ppppp1pp/7n/4Pp2/8/8/PPPP1PPP/RNBQKBNR w KQkq f6 0 3 ;D1 31 ;D2 570 ;D3 17546 ;D4 351806 ;D5 11139762;D6 244063299")
	};

[benb]

i've been browsing your code quickly and the only advice i have for you is try not to use a C++ feature just because you can, for example here a simple ternary operator is enough. also the extensive use of smart pointers is a bit weird (at least for me, if i'm not missing anything).
but nonetheless the code looks much better, good luck.

I find myself allergic to the ternary operator (if conciseness were THAT important we could use use APL), but regardless of that, thanks to C++ there's a lot being done in these lines by the compiler, so that less object code gets executed by the processor.

This is a template, and gets instantiated with a BISHOP or a ROOK. If it's not one of those, it'll give the compile-time error on line 30.

The index_size code in the BISHOP instantiation will load a register with 512, and the ROOK instantiation (resulting in another, similar block of object code, which some may argue is wasteful) will load with 4096. Offhand I don't think you can do this in C without a lot of source code duplication.

My main/only complaint with this code is these two values are magic numbers (as used in general programming, no connection to bitboards) and should be defined something like this, to spell out what they represent:

Code: Select all

const uint16_t Bishop_index_size =  512;
const uint16_t Rook_index_size   = 4096;

Does anyone want to start a code review thread? I have some close-to-working 0x88 based code that I've yet to put on Github.

[tcusr]

i've been browsing your code quickly and the only advice i have for you is try not to use a C++ feature just because you can, for example here a simple ternary operator is enough. also the extensive use of smart pointers is a bit weird (at least for me, if i'm not missing anything).
but nonetheless the code looks much better, good luck.

I find myself allergic to the ternary operator (if conciseness were THAT important we could use use APL), but regardless of that, thanks to C++ there's a lot being done in these lines by the compiler, so that less object code gets executed by the processor.

This is a template, and gets instantiated with a BISHOP or a ROOK. If it's not one of those, it'll give the compile-time error on line 30.

The index_size code in the BISHOP instantiation will load a register with 512, and the ROOK instantiation (resulting in another, similar block of object code, which some may argue is wasteful) will load with 4096. Offhand I don't think you can do this in C without a lot of source code duplication.

but none of that happens, the variable is constexpr and all occurrences of index_size will be "replaced" by the actual number. also to check if _Pce is a rook or bishops should be done with concepts, like he does in other parts of the code.

Does anyone want to start a code review thread? I have some close-to-working 0x88 based code that I've yet to put on Github.

that would be nice, i'm going to release my engine too and i'd really appreciate if other people could help me improve it.

[niel5946]

i've been browsing your code quickly and the only advice i have for you is try not to use a C++ feature just because you can, for example here a simple ternary operator is enough. also the extensive use of smart pointers is a bit weird (at least for me, if i'm not missing anything).
but nonetheless the code looks much better, good luck.

I appreciate your comments
I agree that the lambda is a little overkill when compared to the ternary operator. However, I don't think that an extensive use of smart pointers are bad or weird. This is primarily for the reason that it's more modern, and less bug-prone. With normal heap-allocated memory, you have to always keep track of which things you have allocated and which you have freed. I have previously experienced a lot of bugs from either calling free() prematurely or never, causing a memory leak.
Additionally, smart pointers have nearly no overhead. Of course std::shared_ptr does, but I intend to pass it by reference, which will not make it increment the reference count.

My main/only complaint with this code is these two values are magic numbers (as used in general programming, no connection to bitboards) and should be defined something like this, to spell out what they represent:

Code: Select all

const uint16_t Bishop_index_size =  512;
const uint16_t Rook_index_size   = 4096;

Does anyone want to start a code review thread? I have some close-to-working 0x88 based code that I've yet to put on Github.

You are right about the two constants - I'll put them in their own variables soon.
And yes, a code review thread sounds like a good idea!

[dangi12012]

I agree that the lambda is a little overkill when compared to the ternary operator. However, I don't think that an extensive use of smart pointers are bad or weird. This is primarily for the reason that it's more modern, and less bug-prone. With normal heap-allocated memory, you have to always keep track of which things you have allocated and which you have freed. I have previously experienced a lot of bugs from either calling free() prematurely or never, causing a memory leak.

https://github.com/BimmerBass/Loki/blob ... ndex.h#L35

I think the questioner misunderstood what this line of code does. It is not a lambda - it is a compiletime (constexpr) constant initialized by a lambda.
This could be done with a constexpr function but this is much cleaner so your compiletime initializer is anonymous.
The runtime cost is ZERO and its the same as writing two classes - one with const 4096 and one with const 512.

Same thing can be done with Template specialisation and std::enable_if - but this is less readable.

Use constexpr/consteval where it makes sense.
consteval - forces compiletime execution of a function (the function does not exist after compilation)
constexpr - forces compiletime initialisation/execution in some, but not all cases
const - once initialized cannot be changed
static - initialized once