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

Disable 960 castling in analysis + add 960 in board editor for issue #12926 #13453

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 38 additions & 1 deletion ui/analyse/src/ctrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { make as makeSocket, Socket } from './socket';
import { nextGlyphSymbol } from './nodeFinder';
import { opposite, parseUci, makeSquare, roleToChar } from 'chessops/util';
import { Outcome, isNormal } from 'chessops/types';
import { parseFen } from 'chessops/fen';
import { parseFen, makeFen, parseCastlingFen } from 'chessops/fen';
import { Position, PositionError } from 'chessops/chess';
import { Result } from '@badrap/result';
import { setupPosition } from 'chessops/variant';
Expand Down Expand Up @@ -128,6 +128,11 @@ export default class AnalyseCtrl {
makeStudy?: typeof makeStudyCtrl,
) {
this.data = opts.data;
if (opts.data.game.variant.key === 'standard' || opts.data.game.variant.key === 'fromPosition') {
const new_fen = this.returnStandardCastlingFen(opts.data.game.fen);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please mind the project naming conventions. new_fen => newFen. More occurences in the pull request.

opts.data.game.fen = new_fen;
opts.data.treeParts[0].fen = new_fen;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overwriting the fen coming from the server... Maybe it should be fixed on the server side instead, then?

}
this.element = opts.element;
this.trans = opts.trans;
this.treeView = treeViewCtrl('column');
Expand Down Expand Up @@ -468,6 +473,38 @@ export default class AnalyseCtrl {
'/' +
encodeURIComponent(fen).replace(/%20/g, '_').replace(/%2F/g, '/');
}
returnStandardCastlingFen(fen: Fen): Fen {
let new_fen = fen;
const setup = parseFen(fen).unwrap();
let castlingPart = '-';
const iterable = setup.unmovedRooks[Symbol.iterator]();
const w_castling_squares = [];
const b_castling_squares = [];
const kings_in_starting = {
//check if white king on its starting square
w: setup.board.king.has(4) && setup.board.white.has(4),
//check same thing for black king
b: setup.board.king.has(60) && setup.board.black.has(60),
};
let result = iterable.next();
//populate arrays to contain possible black and white castling squares
while (!result.done) {
if (setup.board.black.has(result.value)) b_castling_squares.push(result.value);
else if (setup.board.white.has(result.value)) w_castling_squares.push(result.value);
result = iterable.next();
}
//generate new fen if necessary
if ((kings_in_starting['w'] && w_castling_squares) || (kings_in_starting['b'] && b_castling_squares)) {
castlingPart =
'K'.repeat(w_castling_squares.includes(7) ? 1 : 0) +
'Q'.repeat(w_castling_squares.includes(0) ? 1 : 0) +
'k'.repeat(b_castling_squares.includes(63) ? 1 : 0) +
'q'.repeat(b_castling_squares.includes(56) ? 1 : 0);
}
setup.unmovedRooks = parseCastlingFen(setup.board, castlingPart).unwrap();
new_fen = makeFen(setup);
return new_fen;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yikes. That code certainly deserves its own file and tests. Should probably be in chessops too, as you mentioned.


userNewPiece = (piece: cg.Piece, pos: Key): void => {
if (crazyValid(this.chessground, this.node.drops, piece, pos)) {
Expand Down
46 changes: 42 additions & 4 deletions ui/editor/src/ctrl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export default class EditorCtrl {
epSquare: Square | undefined;
remainingChecks: RemainingChecks | undefined;
rules: Rules;
variant: string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems that the unique point of this variant member is to tell whether we have chess960 when rules=chess.
So maybe it should be a boolean?

halfmoves: number;
fullmoves: number;

Expand Down Expand Up @@ -60,6 +61,9 @@ export default class EditorCtrl {
this.castlingToggles = { K: false, Q: false, k: false, q: false };
const params = new URLSearchParams(location.search);
this.rules = this.cfg.embed ? 'chess' : lichessRules((params.get('variant') || 'standard') as VariantKey);
this.variant = this.cfg.embed
? 'Standard'
: this.getVariantFromParams(params.get('variant') || 'Standard');
this.initialFen = (cfg.fen || params.get('fen') || INITIAL_FEN).replace(/_/g, ' ');

this.redraw = () => {};
Expand All @@ -78,7 +82,22 @@ export default class EditorCtrl {
this.options.onChange?.(fen);
this.redraw();
}

private getVariantFromParams(paramVariant: string) {
switch (paramVariant) {
case 'standard':
return 'Standard';
case 'chess960':
return 'Chess 960';
case 'threeCheck':
return 'Three-check';
case 'kingOfTheHill':
return 'King of the Hill';
case 'racingKings':
return 'Racing Kings';
default:
return paramVariant.charAt(0).toUpperCase() + paramVariant.slice(1);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why we need to re-hardcode all variant names here.

}
private castlingToggleFen(): string {
let fen = '';
for (const toggle of CASTLING_TOGGLES) {
Expand Down Expand Up @@ -132,13 +151,29 @@ export default class EditorCtrl {
}

makeAnalysisUrl(legalFen: string, orientation: Color = 'white'): string {
const variant = this.rules === 'chess' ? '' : lichessVariant(this.rules) + '/';
const variant =
this.rules === 'chess'
? this.variant == 'Chess 960'
? 'chess960/'
: ''
: lichessVariant(this.rules) + '/';
return `/analysis/${variant}${urlFen(legalFen)}?color=${orientation}`;
}

makeEditorUrl(fen: string, orientation: Color = 'white'): string {
if (fen === INITIAL_FEN && this.rules === 'chess' && orientation === 'white') return this.cfg.baseUrl;
const variant = this.rules === 'chess' ? '' : '?variant=' + lichessVariant(this.rules);
if (
fen === INITIAL_FEN &&
this.rules === 'chess' &&
orientation === 'white' &&
this.variant === 'Standard'
)
return this.cfg.baseUrl;
const variant =
this.rules === 'chess'
? this.variant == 'Chess 960'
? '?variant=chess960'
: ''
: '?variant=' + lichessVariant(this.rules);
const orientationParam = variant ? `&color=${orientation}` : `?color=${orientation}`;
return `${this.cfg.baseUrl}/${urlFen(fen)}${variant}${orientationParam}`;
}
Expand Down Expand Up @@ -203,6 +238,9 @@ export default class EditorCtrl {
else if (!this.remainingChecks) this.remainingChecks = RemainingChecks.default();
this.onChange();
}
setVariant(variant: string): void {
this.variant = variant;
}

setOrientation(o: Color): void {
this.options.orientation = o;
Expand Down
7 changes: 6 additions & 1 deletion ui/editor/src/view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ function variant2option(key: Rules, name: string, ctrl: EditorCtrl): VNode {
{
attrs: {
value: key,
selected: key == ctrl.rules,
selected: key == ctrl.rules && name == ctrl.variant,
},
},
`${ctrl.trans.noarg('variant')} | ${name}`,
Expand All @@ -81,6 +81,7 @@ function variant2option(key: Rules, name: string, ctrl: EditorCtrl): VNode {

const allVariants: Array<[Rules, string]> = [
['chess', 'Standard'],
['chess', 'Chess 960'],
['antichess', 'Antichess'],
['atomic', 'Atomic'],
['crazyhouse', 'Crazyhouse'],
Expand Down Expand Up @@ -222,6 +223,10 @@ function controls(ctrl: EditorCtrl, state: EditorState): VNode {
attrs: { id: 'variants' },
on: {
change(e) {
const option = (e.target as HTMLSelectElement).item(
(e.target as HTMLSelectElement).selectedIndex,
);
if (option) ctrl.setVariant(option.text.slice(10));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is verbose and fragile. Instead we could have a ['chess960', 'Chess 960'] option and set the ctrl.rule and ctrl.isChess960 flags accordingly

ctrl.setRules((e.target as HTMLSelectElement).value as Rules);
},
},
Expand Down
Loading