Skip to content

Comments

#2 ギルドの作成と解散#3

Open
taktakahashi wants to merge 2 commits intodevelopfrom
feature/cyc_2_create_guild
Open

#2 ギルドの作成と解散#3
taktakahashi wants to merge 2 commits intodevelopfrom
feature/cyc_2_create_guild

Conversation

@taktakahashi
Copy link
Collaborator

/sarcandra-guild init {guildName} ギルド作成 一人につき一つまで
/sarcandra-guild dissolve ギルド解散

@blackbracken blackbracken self-requested a review April 14, 2019 16:28
Copy link
Member

@blackbracken blackbracken left a comment

Choose a reason for hiding this comment

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

少し多くなってしまいましたがレビューしました

基本non-nullableにすることを心掛けるとより良いコードが書けるようになると思います

otherwise LGTM.


import java.util.*

class Guild(val masterUUID: UUID, val memberUUIDList: List<UUID>, val name: String)
Copy link
Member

Choose a reason for hiding this comment

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

このクラスはこのプラグインに居ていい奴だっけ?

Copy link
Member

Choose a reason for hiding this comment

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

あとこれdata classのほうがいいやつですね

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

とりあえずないとつらいから書いた

Copy link
Member

Choose a reason for hiding this comment

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

とりあえずで各プラグインに書いたクラスをあとでコアに移植するのと、
そもそもの使い方を事前にバチっと決めて最初にコアにクラス宣言しておくのどっちが楽か迷っているなう

@ryo-ymd
Copy link
Member

ryo-ymd commented Apr 14, 2019

このPRコメント多くて引くww

@taktakahashi taktakahashi force-pushed the feature/cyc_2_create_guild branch from a3a694d to 8478e49 Compare April 15, 2019 09:32
"INIT" -> {
guildContainer.createGuild(senderPlayer?.uniqueId, args[1])
true
if (args[1].isNullOrBlank()) {
Copy link
Member

Choose a reason for hiding this comment

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

条件式の真偽が逆になっていませんか

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.

3 participants