Stockfish: RENAMING/REFORMATTING THREAD

Created on 11 Dec 2018  路  12Comments  路  Source: official-stockfish/Stockfish

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.

Most helpful comment

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.

All 12 comments

rootNode should be named RootNode like PvNode.

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

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.

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

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

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.

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

@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.

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.

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

Closing, please post your suggestions directly in #1894

Was this page helpful?
0 / 5 - 0 ratings

Related issues

GBeauregard picture GBeauregard  路  7Comments

d3vv picture d3vv  路  4Comments

nguyenpham picture nguyenpham  路  4Comments

BKSpurgeon picture BKSpurgeon  路  6Comments

rayoh123 picture rayoh123  路  5Comments