-
Notifications
You must be signed in to change notification settings - Fork 9
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
Dimensions Addition #1
base: master
Are you sure you want to change the base?
Conversation
… catches for teleporting within the same dimension
Any specific reason why you decided to rename the Main.java file to StayPut.java? |
I did so comparing to other plugins and saw that they used more defined
names. Particularly if that class needs to be called by another java
program.
…On Wed, Oct 31, 2018, 01:17 Zander ***@***.*** wrote:
Any specific reason why you decided to rename the Main.java file to
StayPut.java?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABiDuLTdV_urjmvI2ijVu-Jwoys0weEgks5uqVyZgaJpZM4YBJM6>
.
|
Is something amiss that you would like to review further? I add quite a bit to make the logic for dimensions, and can understand if you need more time. |
Apologies, wrong button =) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few things you should change in my opinion to make sure it is safe to put on a server.
Also change all the variables to camelCase so they are the same in the whole database. :)
Besides that, looks good! Especially for the first time PR'ing. 😄
return true; | ||
} | ||
|
||
protected void sendInfoMessage(CommandSender commandSender){ | ||
this.sendMessage(commandSender, "Available commands:"); | ||
this.sendMessage(commandSender, "/stayput reload - Reloads the config files"); | ||
this.sendMessage(commandSender, "/stayput listdimensions - Lists all the dimensions"); | ||
this.sendMessage(commandSender, "/stayput rebuildTables - DO NOT RUN THIS"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe don't add it here if you shouldn't run it? Or add a better message to know why people shouldn't run it.
} | ||
|
||
if(executedCommand.equals("rebuildTables")) { | ||
this.sendMessage(commandSender, "-- Deleting and Rebuilding Position --"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a permission check here? I assume not everyone should be allowed to run this command.
public void setYaw(Float yaw) { | ||
this.yaw = yaw; | ||
} | ||
|
||
public String getDimension_name() { return dimension_name; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may be a bit of nitpicking, but could you please format all the methods the same? So always a new line after the {
and the }
on a new line. Also make sure there is a new line between the methods.
double coordX = position.getCoordinate_x(); | ||
double coordY = position.getCoordinate_y(); | ||
double coordZ = position.getCoordinate_z(); | ||
double coordinate_x = position.getCoordinate_x(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the camelCase format for variables. This is meant for all the variables in your changes.
(And if you find any of mine, please change them to if you want to :))
What is the current status of this (and maybe other) PRs? |
Since our last conversation several months ago, sorry about that.
I've not since done the update to the standards you mentioned. I follow the
standard of snake casing variables and camel casing methods. Though I'm
going to conform to your style for this mod.
Then with that is done, I will have a PR ready for you. So some time
tonight.
…On Tue, 30 Apr 2019 at 00:31, Zander ***@***.***> wrote:
What is the current status of this (and maybe other) PRs?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAMIHOCIRCHXLJ6OBT5KUWTPS7YWPANCNFSM4GAESM5A>
.
|
I honestly hope I'm doing this correctly. First pull request of stable dimensions.