Kaya - pull request! #5

Open
wants to merge 6 commits into
from

Conversation

Projects
None yet
2 participants
@tomodo
Collaborator

tomodo commented Feb 20, 2012

I've made the moves list numbering for shogi games follow the shogi style (move number shows turns taken rather than ply). I fixed a bug that would break communication with the shogi engines (GNU and GPS) if you pressed undo while it was still thinking.

@pcapriotti

This comment has been minimized.

Show comment Hide comment
@pcapriotti

pcapriotti Feb 22, 2012

This is nice, but it should really go into the Shogi plugin. Can you please add a numbering_style method to Game, with the current style as default, and override it for Shogi? I don't want game-specific logic to creep into generic classes like MoveList.

This is nice, but it should really go into the Shogi plugin. Can you please add a numbering_style method to Game, with the current style as default, and override it for Shogi? I don't want game-specific logic to creep into generic classes like MoveList.

This comment has been minimized.

Show comment Hide comment
@tomodo

tomodo Feb 23, 2012

Owner

Thanks for your tips! The way you suggest is much better! I tried to implement it

219c806

is acceptable?

Owner

tomodo replied Feb 23, 2012

Thanks for your tips! The way you suggest is much better! I tried to implement it

219c806

is acceptable?

This comment has been minimized.

Show comment Hide comment
@pcapriotti

pcapriotti Mar 5, 2012

I added a couple of comments to your commit. I'm not sure if you get a notification for those.

I added a couple of comments to your commit. I'm not sure if you get a notification for those.

@pcapriotti

This comment has been minimized.

Show comment Hide comment
@pcapriotti

pcapriotti Feb 24, 2012

This doesn't seem to be needed here.

This doesn't seem to be needed here.

@pcapriotti

This comment has been minimized.

Show comment Hide comment
@pcapriotti

pcapriotti Feb 24, 2012

Can you make this an instance method and instantiate this class in Game constructors?

Can you make this an instance method and instantiate this class in Game constructors?

This comment has been minimized.

Show comment Hide comment
@tomodo

tomodo Mar 6, 2012

Owner

I don't know how to do that, sorry.

Owner

tomodo replied Mar 6, 2012

I don't know how to do that, sorry.

@pcapriotti

This comment has been minimized.

Show comment Hide comment
@pcapriotti

pcapriotti Feb 24, 2012

This should also be an instance method.

This should also be an instance method.

@pcapriotti

This comment has been minimized.

Show comment Hide comment
@pcapriotti

pcapriotti Feb 24, 2012

This is also not needed.

This is also not needed.

@pcapriotti

This comment has been minimized.

Show comment Hide comment
@pcapriotti

pcapriotti Feb 24, 2012

Looks good, otherwise.

Looks good, otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment