Skip to content

Conversation

@Wild1145
Copy link
Member

This was the best way of doing it that I can think of, think formatting is correct.

Tested and worked locally.

Copy link

Choose a reason for hiding this comment

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

There should be a space before the curly brace.

Copy link

Choose a reason for hiding this comment

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

This needs to be indented.

src/config.yml Outdated
Copy link
Member

Choose a reason for hiding this comment

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

There already is an "address" field, which also contains the port. Could you force IP that hostname instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about it, I didnt know exactly what the IP was being used for, so decided to create the Hostname & port field, but yeah, that shouldnt be a big thing to change.

@JeromSar
Copy link
Member

JeromSar commented Nov 1, 2014

@Wild1145 Sorry to be a buzzkill but you should probably replace solely the hostname if the port is 25565.

@JeromSar JeromSar added this to the Release 4.3 milestone Nov 1, 2014
@Wild1145
Copy link
Member Author

Wild1145 commented Nov 1, 2014

Already done.

As for the :25565 bit, it might not be needed, but I thought to include it as I had no way to test without properly.

@JeromSar
Copy link
Member

JeromSar commented Nov 1, 2014

@Wild1145 Could you modify it so the port is omitted? I'll give you a subdomain if you need to test the feature.

@Wild1145
Copy link
Member Author

Wild1145 commented Nov 1, 2014

@JeromSar I just tested it without the port, it works fine as I thought it might. Pushing now.

@JeromSar
Copy link
Member

JeromSar commented Nov 1, 2014

@Wild1145 That's not exactly what I meant. The port should be included in the address field, but the kick message should have the port omitted if it is 25565.

@Wild1145
Copy link
Member Author

Wild1145 commented Nov 1, 2014

@JeromSar Ahh, I see what you mean. I think this solves the problem anyway, because you wouldnt put :25565 on the end of play.totalfreedom.me unless you needed to. You would want the port included any other reason.

If you want I can put an if statement in there to remove the port if it is 25565, but I doubt you would put the port in there now unless you were not on default port.

@JeromSar
Copy link
Member

JeromSar commented Nov 1, 2014

@Wild1145 You're right, this is an alright solution. The port could be resolved with the Bukkit API anyway. Did you test everything?

@Wild1145
Copy link
Member Author

Wild1145 commented Nov 1, 2014

@JeromSar I tested it as much as I could locally, and it seems to work as expected.

@Wild1145
Copy link
Member Author

Wild1145 commented Nov 1, 2014

@WickedGamingUK I did that originally, however that involved having the 2 extra fields. I didnt want to mess with the current address system so just implemented this using it. It could be switched around later if its deemed appropriate, but this does work as it should.

JeromSar added a commit that referenced this pull request Nov 1, 2014
@JeromSar JeromSar merged commit 0aa0bae into TotalFreedom:master Nov 1, 2014
@JeromSar
Copy link
Member

JeromSar commented Nov 1, 2014

Thanks for contributing!

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