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

Panel#takeAndPlaceをStageに移動 #223

Merged
merged 7 commits into from
Jan 9, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions lib/panel.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,27 +27,25 @@ class Panel {
if (currentCount !== null) { // null means infinity
this.parts.set(blockName, currentCount + 1);
}
return true;
this.update();
}

/**
* panel上のblockを減らす
* @param {string} blockName - 消費するblock名
* @return {bool} 正しくblockを減らせたかどうか
* @return {undefined}
*/
take(blockName) {
if (!this.parts.has(blockName)) {
return false;
}
const currentCount = this.parts.get(blockName);
Copy link
Member

Choose a reason for hiding this comment

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

消すんじゃなくてassertにしたいかなー。 currentCount === 0 の場合も同様。

if (currentCount !== null) { // null means infinity
if (currentCount - 1 === 0) { // take the last block
Copy link
Member

Choose a reason for hiding this comment

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

currentCountが0のときに return false する必要がありそう

Copy link
Member Author

Choose a reason for hiding this comment

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

多分今のところ、currentCountが0という状況は発生し得ないのですが(そうなりそうな場合partsからkeyごと消される)、対応したほうがいいもんですかね

Copy link
Member

Choose a reason for hiding this comment

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

それを言うなら39行目の !this.parts.has(blockName) も発生し得ないような気が⋯⋯。それなら「正しくblockを減らせたかどうか」をreturnする仕様を消して、アサーションに置き換えたほうがよさそう

Copy link
Member Author

Choose a reason for hiding this comment

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

45行目を見てください。(これはもとからの仕様です)

Copy link
Member

Choose a reason for hiding this comment

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

あー、selectedがundifinedにならなくなったのか⋯⋯。それならむしろここでselectedにundefinedをセットして、takeAndPlaceにundefinedなら回転する仕様を追加して return false を撲滅するほうがよさそう。

Copy link
Member

Choose a reason for hiding this comment

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

(いずれにせよreturnされた値は使ってないんだし)

Copy link
Member Author

Choose a reason for hiding this comment

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

あ、39行目なんて通らないんじゃないかの意味がようやく分かった。(だーめ。)
undefinedにする案よいかもしれませんね。直前に操作した以外のブロックも回転するようになりますが、まあ。

this.selected = null;
this.parts.delete(blockName);
} else {
this.parts.set(blockName, currentCount - 1);
}
}
return true;
this.update();
}

update() {
Expand Down
9 changes: 3 additions & 6 deletions lib/stage.js
Original file line number Diff line number Diff line change
Expand Up @@ -400,17 +400,15 @@ class Stage {
const oldBlock = this.board.getBlock(x, y);
assert(oldBlock, 'oldBlock is invalid');

if (blockName === oldBlock.config.name) { // rotate the block
if (!blockName || blockName === oldBlock.config.name) { // rotate the block
if (oldBlock.config.rotatable) {
const rotate = (oldBlock.config.rotate + 1) % 4;
this.boardElement.placeBlock({x, y, type: blockName, rotate});
this.boardElement.placeBlock({x, y, type: oldBlock.config.name, rotate});
}
} else { // replace the block
// take the block from panel
if (blockName !== 'empty') {
if (!this.panel.take(blockName)) { // lack the block, can't place it
return;
}
this.panel.take(blockName);
}

// push the old block into panel
Expand All @@ -419,7 +417,6 @@ class Stage {
}

this.boardElement.placeBlock({x, y, type: blockName, rotate: 0});
this.panel.update();
}
}
}
Expand Down