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

fixed: Display latest stone for ghost dead stone #401

Merged
merged 3 commits into from
Nov 3, 2018

Conversation

zsalch
Copy link
Contributor

@zsalch zsalch commented Oct 14, 2018

Issue: #301

Display latest stone for ghost dead stone

@@ -384,7 +384,8 @@ private void drawBranch() {

for (int i = 0; i < Board.BOARD_SIZE; i++) {
for (int j = 0; j < Board.BOARD_SIZE; j++) {
if (Lizzie.board.getData().stones[Board.getIndex(i, j)] != Stone.EMPTY) continue;
// Display latest stone for ghost dead stone
// if (Lizzie.board.getData().stones[Board.getIndex(i, j)] != Stone.EMPTY) continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just remove the line and don't put a comment...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently some test is ok, but has not more tests. I want to temporarily reserve it so that can suggest changes here when there is a problem.
Of course, I'm ok with remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. So you propose to keep it commented out until we are sure that it doesn't break something else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this change can solve some problems, but it also changes the display of the stones. I think we should be cautious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have committed a new change for fix the stone double shadow issue when branch

@OlivierBlanvillain
Copy link
Contributor

LGTM

@zsalch zsalch merged commit 84d8709 into featurecat:next Nov 3, 2018
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