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

Goscorer integration and stone removal phase overhaul #162

Closed
wants to merge 14 commits into from
Closed

Conversation

anoek
Copy link
Member

@anoek anoek commented Jun 8, 2024

This patch updates the stone removal phase in a number of ways:

  • Switches our scorer to use https://github.com/lightvector/goscorer which does automatic dame detection and proper seki scoring for Japanese rules
  • Adds support for highlighting locations that need sealing before proper scoring can be done
  • Alters the toggling of dead stones in the stone removal phase to make it less error prone
  • Removes the ability to manually flag locations as dame. Moving forward if there is a problem with dame detection, it's considered a bug.
  • Adds a red X to removed stones for clarity

image

@anoek anoek marked this pull request as draft June 9, 2024 12:18
});
return encodeMoves(moves);
export function sortMoves(moves: string, width: number, height: number): string;
export function sortMoves(moves: JGOFMove[], width: number, height: number): JGOFMove[];
Copy link
Contributor

Choose a reason for hiding this comment

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

It could be nice to allow the caller to omit the width/height arguments in the JGOFMove signature, given that they aren't used.

Suggested change
export function sortMoves(moves: JGOFMove[], width: number, height: number): JGOFMove[];
export function sortMoves(moves: JGOFMove[], width?: number, height?: number): JGOFMove[];

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch thanks!

public toggleSingleGroupRemoval(
x: number,
y: number,
force_removal: boolean = false,
Copy link
Contributor

@benjaminpjones benjaminpjones Jun 9, 2024

Choose a reason for hiding this comment

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

If I'm reading correctly, the only way to trigger this is via shift-click. Is there a mobile-friendly escape hatch as well?

And are we that confident in the algorithm to prevent people from removing groups (without the shift key)

Copy link
Member Author

Choose a reason for hiding this comment

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

Long press, I think that trickles down starting from the GobanSVG and GobanCanvas code

src/GoEngine.ts Outdated Show resolved Hide resolved
@anoek anoek closed this Jun 10, 2024
@anoek anoek deleted the goscorer branch June 21, 2024 12:21
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.

2 participants