Skip to content

Conversation

@anarchuser
Copy link
Contributor

This shall resolve #301

@anarchuser anarchuser self-assigned this Oct 9, 2020
@anarchuser anarchuser force-pushed the refactor/move-validation branch 2 times, most recently from 60fbcc2 to 883ad6b Compare October 9, 2020 12:02
@anarchuser anarchuser requested a review from xeruf October 9, 2020 12:14
In turn, refactor them a bit to pass fields as parameters, instead of
raw coordinates and colors
One can choose to have them throw as before, or return a `MoveMistake`
enum which allows for switching of the specific issue in question.
@anarchuser anarchuser force-pushed the refactor/move-validation branch from fe63a47 to e5baeac Compare October 9, 2020 12:17
Copy link
Contributor

@SKoschnicke SKoschnicke left a comment

Choose a reason for hiding this comment

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

I think the following methods can be removed as they are private and not called in the class:

  • getAllPossibleMoves
  • getPossibleStartMoves
  • getAllMoves

Everthing else looks good to me!

Copy link
Contributor

@SKoschnicke SKoschnicke left a comment

Choose a reason for hiding this comment

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

Nice!

@anarchuser anarchuser merged commit b21d30d into master Oct 22, 2020
@anarchuser anarchuser deleted the refactor/move-validation branch October 22, 2020 09:28
@xeruf
Copy link
Member

xeruf commented Oct 22, 2020

Wieder so ein PR wo squashen einen unnötig großen commit erzeugt... Ich glaube ich muss die convention anpassen, ich bin andere arbeitsstile gewohnt ^^

@anarchuser
Copy link
Contributor Author

Es sind ein paar kleine Änderungen und ein großer Commit.
Commit 38721b1 ist alleine für den Großteil der Änderungen verantwortlich.
Also ja, die anderen kleinen Änderungen hätten in separaten Commits sein können, aber das hätte nicht viel an der Größe des Commits geändert.

anarchuser added a commit that referenced this pull request Nov 23, 2020
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.

GameRuleLogic methods should be public

4 participants