Disable mob drop multiplication for horses

[TealNerd]

No description provided.

[ttk2]

thank you, also Donkeys?

On Sat, Apr 11, 2015 at 6:20 PM biggestnerd notifications@github.com
wrote:

---

You can view, comment on, or merge this pull request online at:

#21
Commit Summary

• Update Humbug.java

File Changes

• M src/main/java/com/untamedears/humbug/Humbug.java
  https://github.com/Civcraft/Humbug/pull/21/files#diff-0 (2)

Patch Links:

• https://github.com/Civcraft/Humbug/pull/21.patch
• https://github.com/Civcraft/Humbug/pull/21.diff

—
Reply to this email directly or view it on GitHub
#21.

[TealNerd]

donkeys are horses and bukkit is stupid

[ttk2]

that's been established.

On Sat, Apr 11, 2015 at 6:21 PM biggestnerd notifications@github.com
wrote:

donkeys are horses and bukkit is stupid

—
Reply to this email directly or view it on GitHub
#21 (comment).

[ttk2]

BTW main server drop rate is 1 until this goes online tomorrow.

On Sat, Apr 11, 2015 at 6:26 PM justin kilpatrick <
kilpatrickjustin@gmail.com> wrote:

that's been established.

On Sat, Apr 11, 2015 at 6:21 PM biggestnerd notifications@github.com
wrote:

donkeys are horses and bukkit is stupid

—
Reply to this email directly or view it on GitHub
#21 (comment).

[TealNerd]

but yeah you should merge this so it can build

[ttk2]

going to have someone do a quick review, looks good to me though, but I know nothing about humbug

[TealNerd]

its just the one line that checks if the mob is a player so it doesnt dupe player drops. i added horses to the if statement

[Pentom]

Code Review: http://www.reddit.com/r/Civcraft/comments/32bqoe/morning_changelog_20150412/cq9pvxw

TLDR: Feature loses a miniscule bit of functionality that it had before (previoulsy it wiped out horse mob xp drops but now it allows them). It has no noticeable affect on civcraft since xp is disabled (validated explicitly and stated in review). I've suggested that this functionality be either reintroduced for horses to keep what we have now or removed. I would not necessarily push for this as a pre-req for acceptance of this pull though since I think adding the feature back in for horses is stupid since I would prefer it to be gone for all mob types. I've added issue 22 to track this bad configuration option so this pull -can- be accepted without any changes as long as we accept that horses will drop useless xp orbs. #22

TLDR TLDR: It appears fine to accept with a minor useless problem that may or may not be fixed.

[ttk2]

Erocs just set the multiplier for horses specifically to 1 seems to work
for now. But it makes cases where other people using this feature could
face unintended consequences. Suggested correction involves having the
horse multiplier continue to exist but not fall under general. Or we could
actually differentiate between user stuff on a horse and its drops.

On Sun, Apr 12, 2015, 9:01 AM Pentom notifications@github.com wrote:

Code Review:
http://www.reddit.com/r/Civcraft/comments/32bqoe/morning_changelog_20150412/cq9pvxw

TLDR: Feature loses a miniscule bit of functionality that it had before
(previoulsy it wiped out horse mob xp drops but now it allows them). It has
no noticeable affect on civcraft since xp is disabled (validated explicitly
and stated in review). I've suggested that this functionality be either
reintroduced for horses to keep what we have now or removed. I would not
necessarily push for this as a pre-req for acceptance of this pull though
since I think adding the feature back in for horses is stupid since I would
prefer it to be gone for all mob types. I've added issue 22 to track this
bad configuration option so this pull -can- be accepted without any changes
as long as we accept that horses will drop useless xp orbs. #22
#22

TLDR TLDR: It appears fine to accept with a minor useless problem that may
or may not be fixed.

—
Reply to this email directly or view it on GitHub
#21 (comment).

[rourke750]

Does this need to be merged?

[CivcraftBot]

Can one of the admins verify this patch?

[ttk2]

I think erocs just set the multiplier to one manually, it perhaps should be
that way by default instead of having a hard coded override.

On Sat, Apr 18, 2015 at 11:14 PM CivcraftBot notifications@github.com
wrote:

Can one of the admins verify this patch?

—
Reply to this email directly or view it on GitHub
#21 (comment).