Skip to content
This repository has been archived by the owner on Aug 31, 2019. It is now read-only.

Allow custom underwater arrow distances #175

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ShinyDialga
Copy link
Contributor

As of 1.8, arrows have been severely nerfed underwater. They can only go about 7 blocks in water compared to 1.7 being about 20 blocks. This patch allows you to change the float that determines how far the arrow goes underwater. The default setting in the bukkit.yml is 0.6, as is the default in vanilla. The variable in the config appears as arrow-water-distance: 0.6 I found 0.86 (I found the value of cos(pi/6) to be very similar (0.86602540378) but I'm not sure if there's a correlation) to be very replicative of 1.7's arrow behavior of 20 blocks maximum. There is also code (thanks to @jedediah's moving TNT patch) that sends velocity and teleport packets to the client to show the true location of the arrow, as it appears to be a little off without it.

I attempted something at this patch before, but didn't really care enough until this thread came out: https://oc.tc/forums/topics/55d1f8475f35b9034600069d

Here's a youtube video showing 1.8 arrow behavior vs actions with the patch:
https://www.youtube.com/watch?v=EhLiampr_kw

/**
+ * How far should arrows go in water?
+ */
+ public float getArrowWaterDistance();

Choose a reason for hiding this comment

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

Why not have a setter too? Seems a bit silly to have sportbukkit options not mutable, that is the purpose of plugins.

Choose a reason for hiding this comment

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

Also that comment isn't the best. "Distance arrow moves in water" would probably be better, I don't think questions are the best for describing a method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was simply following previous commits structures. The commit for water-pushes-tnt was a bukkit.yml option as well. Not sure what's the need for a setter as this isn't a case you'd change often. The comment for it was also in the form of a question.

Choose a reason for hiding this comment

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

Just because someone else does it that way doesn't make it right. I requested (and never got) a setter for that commit, and having a question as a javadoc is pretty awful at best.

Even if you don't think it would be changed much, the whole point of the API is to give the plugins ways to interact with the server. If we didn't want to change it there would be little point to an API in the first place.

Copy link
Member

Choose a reason for hiding this comment

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

The method is a wrapper for a configuration value. It should not have a setter. Plugins generally should not mutate config values.

Choose a reason for hiding this comment

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

Is there any real justification behind that? What makes a config value so sacred that a plugin can't add functionality to interact with it? Setters exist for numerous other config values, so why should sportbukkit's additions be any different? All values that can be mutable should be mutable through the API, rather than requiring reflection to manipulate them. The configs should just provide a default.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants