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

Add Time To Opponent #97

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Add Time To Opponent #97

wants to merge 6 commits into from

Conversation

MartinREMY42
Copy link
Owner

No description provided.

@MartinREMY42 MartinREMY42 marked this pull request as ready for review October 20, 2021 20:12
turn: 16,
scorePlayerZero: null,
scorePlayerOne: null,
// remainingTimes ??
Copy link
Collaborator

Choose a reason for hiding this comment

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

TODO caché ?

@@ -1004,6 +1072,11 @@ describe('OnlineGameWrapperComponent of Quarto:', () => {
expect(wrapper.chronoOneGlobal.stop).toHaveBeenCalled();
expect(wrapper.notifyTimeoutVictory).toHaveBeenCalled();
}));
it('when resigning, lastMoveTime must be upToDate then remainingMs');
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing test ?

@@ -1004,6 +1072,11 @@ describe('OnlineGameWrapperComponent of Quarto:', () => {
expect(wrapper.chronoOneGlobal.stop).toHaveBeenCalled();
expect(wrapper.notifyTimeoutVictory).toHaveBeenCalled();
}));
it('when resigning, lastMoveTime must be upToDate then remainingMs');

it('when winning move is done, remainingMs at last turn of opponent must be');
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing test?

it('when resigning, lastMoveTime must be upToDate then remainingMs');
it('when winning move is done, remainingMs at last turn of opponent must be');
});
describe('AddTime functionnalities', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

feature? :D Ou simplement describe('AddTime') est assez clair

});
describe('AddTime functionnalities', () => {
it('should allow to add local time to opponent', fakeAsync(async() => {
// Given an onlineGameComponent
Copy link
Collaborator

Choose a reason for hiding this comment

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

chipotage: OnlineGameComponent (use the right capitalization for class names)

spyOn(partDAO, 'update').and.callThrough();
tick(1);

// when local countDownComponent emit addTime
Copy link
Collaborator

Choose a reason for hiding this comment

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

CountDownComponent

spyOn(partDAO, 'update').and.callThrough();
tick(1);

// when local countDownComponent emit addTime
Copy link
Collaborator

Choose a reason for hiding this comment

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

emits

expect(wrapper.chronoZeroGlobal.remainingMs).toBe((30 * 60 * 1000) + (5 * 60 * 1000));
tick(wrapper.joiner.maximalMoveDuration * 1000);
}));
it('should postponed the timeout of chrono and not only change displayed time', fakeAsync(async() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

postpone + when missing

@@ -1370,6 +1560,37 @@ describe('OnlineGameWrapperComponent of Quarto:', () => {
expect(wrapper.getUpdateType(update)).toBe(UpdateType.REQUEST);
tick(wrapper.joiner.maximalMoveDuration * 1000 + 1);
}));
it('Request.LocalTimeAdded + one remainingMs modified = UpdateType.REQUEST', fakeAsync(async() => {
await prepareStartedGameFor({ pseudo: 'creator', verified: true });
wrapper.currentPart = new Part({
Copy link
Collaborator

Choose a reason for hiding this comment

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

y'a pas moyen d'avoir les champs non important séparés des champs important ici pour rendre ça plus clair ?

Comment on lines +78 to +79
// if the distribution would capture all seeds
// the move is legal but the capture is forbidden and cancelled
Copy link
Collaborator

Choose a reason for hiding this comment

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

est-ce qu'on part sur cette convention pour du multiligne intra-fonction au final ? (si t'as changé le /* ... */ j'imagine que c'est que c'était perturbant !)

// when asking list moves
const moves: AwaleMove[] = minimax.getListMoves(node);

// then length should be one and move be the only legal
Copy link
Collaborator

Choose a reason for hiding this comment

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

the length should be one => there should be only one move ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ou simplement: the only move should be the only legal move

@@ -61,6 +61,30 @@ describe('AwaleRules', () => {
const node: AwaleNode = new MGPNode(null, move, resultingState);
expectToBeVictoryFor(rules, node, Player.ONE, minimaxes);
});
it('should not do mansoon when possible distribution', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

monsoon

const status: AwaleLegalityStatus = rules.isLegal(move, state);
const resultingState: AwaleState = rules.applyLegalMove(move, state, status);

// then, since other player can actually distribute, he do not mansoon all his pieces
Copy link
Collaborator

Choose a reason for hiding this comment

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

monsoon + texte genré !

@@ -76,6 +100,30 @@ describe('AwaleRules', () => {
// then the move is illegal
expect(status.legal.reason).toBe(AwaleFailure.SHOULD_DISTRIBUTE());
});
it('should allow mansoon on one last capture', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

monsoon

Comment on lines 47 to 51
if (state.pieceInHand !== QuartoPiece.NONE) {
this.pieceInHand = state.pieceInHand;
} else {
this.pieceInHand = null;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm quelle est l'utilité d'avoir pieceInHand : QuartoPiece | null si on a QuartoPiece.NONE pour représenter l'absence de pièce ?

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