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

Ply limit alert and prevent premoves in stalemates / insufficient material positions #16679

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

johndoknjas
Copy link
Contributor

@johndoknjas johndoknjas commented Dec 27, 2024

Resolves #16667 and #16683

From what I understand, dests cannot be an empty string when there's a connection/lag issue with the server. In those cases dests would be undefined. So the if condition for an alert added by this PR should(?) only affect the ply limit case.

@johndoknjas johndoknjas marked this pull request as draft December 27, 2024 17:38
@SergioGlorias
Copy link
Contributor

From what I know, when the legal limit arrives, it doesn't even allow selecting any piece, but when the lichess limit is mentioned (600 ply)
the person can make the movements, but soon after the server deletes the movements

@johndoknjas
Copy link
Contributor Author

johndoknjas commented Dec 28, 2024

@SergioGlorias For me the study board actually lets me make premoves in stalemates / no mating material positions. E.g.:
image

Better behaviour would be as you said though, so maybe that should be an issue itself.

@johndoknjas johndoknjas changed the title Alert the user when no more moves allowed in a chapter Ply limit alert and prevent premoves in stalemates / insufficient material positions Dec 29, 2024
@ornicar
Copy link
Collaborator

ornicar commented Dec 29, 2024

The reason why the server sends san, dests and fen is to save the client the time of processing chess positions.

If we start calling parseFen, setupPosition and isEnd on every move, then the whole point is defeated, and we might as well stop sending fens and dests from the server.

Which is something to consider. But in the meantime let's not have do both.

@@ -574,7 +575,8 @@ export default class AnalyseCtrl {
}

onPremoveSet = () => {
if (this.study) this.study.onPremoveSet();
if (this.node.dests === '') alert('Too many moves for a lichess board.');
Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a weird assumption to make and one that would probably backfire. Can we use the ply number instead?

@@ -353,7 +353,8 @@ export default class AnalyseCtrl {
config.movable!.color = color;
}
config.premovable = {
enabled: config.movable!.color && config.turnColor !== config.movable!.color,
enabled:
config.movable!.color && config.turnColor !== config.movable!.color && !this.currPosition().isEnd(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the point of analysis premoves again? Since we can move all pieces.

I suppose it's only useful in specific applications like "learn from your mistakes" where only one side is playable. That would be a better thing to check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ornicar I think the main point is when the connection to the server is poor. E.g., if you open a study board and turn off wifi, the next move should be a premove.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Study: Warning message that you have reached the ply limit in a chapter
3 participants