official-stockfish / Stockfish

A free and strong UCI chess engine
https://stockfishchess.org/
GNU General Public License v3.0
11.33k stars 2.25k forks source link

RENAMING/REFORMATTING THREAD #1862

Closed mcostalba closed 5 years ago

mcostalba commented 5 years ago

Use this issue to post renaming suggestions, once in a while, if approved will be committed at once.

We will use a table to make it easier account pending renames.

ghost commented 5 years ago

rootNode should be named RootNode like PvNode.

mcostalba commented 5 years ago

@Chess13234 Is rootNode a compile time constant? If not, then rootNode is ok.

joergoster commented 5 years ago

In search.cpp, the curly brackets in lines 494 and 519 and the whole body inbetween, are wrongly indented. It's the // Do we have time for the next iteration? Can we stop searching now? stuff.

protonspring commented 5 years ago

The new pawns 8x8 psqt has ^M carriage return characters on each line.

Rocky640 commented 5 years ago

evaluate.cpp line 516 replace nonPawnEnemies = pos.pieces(Them) & ~pos.pieces(Them, PAWN); with nonPawnEnemies = pos.pieces(Them) & ~pos.pieces(PAWN);

miguel-l commented 5 years ago

In our main search<>() function, we have variables named captureCount and quietCount but in update_capture_stats and update_quiet_stats functions, we have them as captureCnt and quietsCnt. It would be nice to stick to one or the other.

protonspring commented 5 years ago

https://github.com/nickpelling/Stockfish/compare/31ac538...7f00d6c

@Rocky640 pointed out:

It seems that we are not consistent with this in sf code. Not a big deal but...

On one hand, we have many function declaration arguments using (you can search for Bitboard ExtMove Move StateInfo or Thread)

For example this, which will modify b inline Square pop_lsb(Bitboard* b)

But on the other hand, we have many function declaration arguments using & (you can search for bool& StateInfo& istringstream& istream& Bitboard& Square& or RootMoves&)

For example void do_castling(Color us, Square from, Square& to, Square& rfrom, Square& rto); which will modify to, rfrom and rto.

or PieceType min_attacker(const Bitboard* byTypeBB, Square to, Bitboard stmAttackers, Bitboard& occupied, Bitboard& attackers) { which will modify occupied, and attackers

protonspring commented 5 years ago

@joergoster says . . . regarding the KBNvK endgame. . .

Note that adding the non_pawn_material(strongSide) like in other endgames is also missing. I never cared to open a PR because of this, though.

protonspring commented 5 years ago

Some places we check if a move is MOVE_NONE with. . move == MOVE_NONE. other places we check if a move is MOVE_NONE with . . !move.

It would probably be good to make them all the same. For clarify, I prefer "move == MOVE_NONE" or similar.

protonspring commented 5 years ago

In the endgames, some methods use "verify_material" and others do not. Best to be consistent, either all use verify_material, or not.

mcostalba commented 5 years ago

Now there is a PR https://github.com/official-stockfish/Stockfish/pull/1894

mcostalba commented 5 years ago

Closing, please post your suggestions directly in #1894