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

1.1.0 #163

Open
wants to merge 69 commits into
base: master
Choose a base branch
from
Open

1.1.0 #163

wants to merge 69 commits into from

Conversation

LarsArtmann
Copy link
Member

No description provided.

@LarsArtmann LarsArtmann added bug Something isn't working enhancement New feature or request labels Jun 10, 2019
@LarsArtmann LarsArtmann requested a review from DevSnox June 10, 2019 21:28
@LarsArtmann LarsArtmann self-assigned this Jun 10, 2019
@LarsArtmann
Copy link
Member Author

@DevSnox i need a review

@LarsArtmann LarsArtmann changed the title Fixed gradle and some imports 1.1.0 Jun 18, 2019
@LarsArtmann LarsArtmann changed the title 1.1.0 WIP 1.1.0 Jun 19, 2019
@LarsArtmann LarsArtmann moved this from In progress to In QS in RELEASE-1.1 Jun 21, 2019
@LarsArtmann LarsArtmann requested review from DevSnox and removed request for DevSnox June 21, 2019 15:36
@LarsArtmann
Copy link
Member Author

1.1.0 is Done

@LarsArtmann LarsArtmann changed the title WIP 1.1.0 1.1.0 Jun 21, 2019
}

fun Entity.isNotInGameWorld() = world.isNotGameWorld()
fun World.isNotGameWorld() = this != gameWorld
Copy link
Member

Choose a reason for hiding this comment

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

#isGameWorld would be better, wouldn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't need isGameWorld in the src anywhere

players.forEach { it.save() }
}

fun Entity.isNotInGameWorld() = world.isNotGameWorld()
Copy link
Member

Choose a reason for hiding this comment

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

#isInGameWorld would be better, wouldn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't need isGameWorld in the src anywhere

Copy link
Member

Choose a reason for hiding this comment

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

Yeah sure but why using directly using a negative boolean?

Copy link
Member Author

Choose a reason for hiding this comment

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

So i don't need to ride ! for each function call

Copy link
Member

Choose a reason for hiding this comment

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

Larger names for no reason.
It makes sense for having those positive because there is an operator check if negative "!" but no one if it's positive.
So the base value of a boolean returning method should be always of positive value.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you want, i can add a unused positive function, should i?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah sure cause "empty" is a negativ statment.

Copy link
Member Author

Choose a reason for hiding this comment

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

isEmpty also exits

Copy link
Member

Choose a reason for hiding this comment

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

I think we should discuss about that 😂

Copy link
Member Author

Choose a reason for hiding this comment

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

Than call me :D

fun findPlanet(planet: UniqueID): OfflinePlanet?

fun findPlayerOrCreate(owner: Owner, planet: UniqueID): OfflinePlayer
fun findPlanetOrCreate(planet: UniqueID, owner: Owner): OfflinePlanet
Copy link
Member

Choose a reason for hiding this comment

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

#findOrCreatePlanet() would be correct

Copy link
Member Author

Choose a reason for hiding this comment

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

Das habe ich aufgrund von "findPlanet" gemacht (wegen den Vorschlägen)

fun findPlayer(owner: Owner): OfflinePlayer?
fun findPlanet(planet: UniqueID): OfflinePlanet?

fun findPlayerOrCreate(owner: Owner, planet: UniqueID): OfflinePlayer
Copy link
Member

Choose a reason for hiding this comment

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

#findOrCreatePlayer() would be correct

Copy link
Member Author

Choose a reason for hiding this comment

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

Das habe ich aufgrund von "findPlayer" gemacht (wegen den Vorschlägen)

Copy link
Member Author

@LarsArtmann LarsArtmann left a comment

Choose a reason for hiding this comment

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

Gute arbeit! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
RELEASE-1.1
  
In QS
Development

Successfully merging this pull request may close these issues.

None yet

2 participants