-
-
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
Feat: choose 960 start position on the analysis board #16978
base: master
Are you sure you want to change the base?
Conversation
app/controllers/UserAnalysis.scala
Outdated
.decodeUriPath(urlFen) | ||
.filter(_.trim.nonEmpty) | ||
.orElse(get("fen")) | ||
.map(Fen.Full.clean) | ||
val parsed960: Option[Fen.Full] = input | ||
.ifTrue(variant.chess960) |
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.
this takes the FEN input
, if the variant is chess960
app/controllers/UserAnalysis.scala
Outdated
val parsed960: Option[Fen.Full] = input | ||
.ifTrue(variant.chess960) | ||
.orElse(get("position")) | ||
.flatMap(_.toIntOption.flatMap(Chess960.positionToFen)) |
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.
and then, if position
isn't defined, it attempts to convert the FEN into an Int?
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.
keeping .map(Fen.Full.clean)
on line 38 would help prevent that, as scala will then refuse to convert a Fen.Full to an Int
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.
I was trying to account for /chess960?poisiton={num}
as well as /chess960/positionNum
like we allow for fen
. But I realise that was probably unnecessary
Chess960.positionNumber(inputFen | variant.initialFen) | ||
) // no input fen or num defaults to standard start position | ||
) | ||
val decodedFen: Option[Fen.Full] = chess960PositionNum.flatMap(Chess960.positionToFen).orElse(inputFen) |
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.
To allow /chess/960?position={num}
or /chess960/{fen}
(which could be a starting fen). Like we currently do while linking to a start position. That should probably also be changed to the num format for brevity.
But the fentoposition followed by the positiontofen doesn't feel right to me.
Closes #1662