-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
base: master
Are you sure you want to change the base?
Changes from all commits
a5f1b84
a98e7c7
bc5f9bd
a3c9aae
a071c12
a34377d
1b5af30
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
@@ -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); | ||
opts.data.game.fen = new_fen; | ||
opts.data.treeParts[0].fen = new_fen; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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'); | ||
|
@@ -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; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ export default class EditorCtrl { | |
epSquare: Square | undefined; | ||
remainingChecks: RemainingChecks | undefined; | ||
rules: Rules; | ||
variant: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems that the unique point of this |
||
halfmoves: number; | ||
fullmoves: number; | ||
|
||
|
@@ -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 = () => {}; | ||
|
@@ -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); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
@@ -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}`; | ||
} | ||
|
@@ -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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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}`, | ||
|
@@ -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'], | ||
|
@@ -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)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is verbose and fragile. Instead we could have a |
||
ctrl.setRules((e.target as HTMLSelectElement).value as Rules); | ||
}, | ||
}, | ||
|
There was a problem hiding this comment.
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.