[B+C] Add a way to induce mob breeding, adds BUKKIT-5687

[mcmonkey4eva]

The Issue:

There is not a way to induce mob breeding, or check whether they are trying to breed.
An animal is considered to be breeding when it has been given food (EG: wheat) and is looking for another animal of its species to mate with and produce a baby animal.

Justification for this PR:

This new API feature will be useful for any plugin that wishes to induce mob breeding. An example case would be an NPC Farmer - without a way to make mobs breed, it would have to be faked with workarounds, EG manually spawning heart particles and manually spawning baby entities.

PR Breakdown:

Bukkit-side:

Adds the functions in Animals to induce breeding / check breeding attempt.

CraftBukkit-side:

Implements the functions in CraftAnimals to induce breeding / check breeding attempt. Mostly just wrapping some NMS functions.

Testing Results and Materials:

Any default plugin or pre-existing plugin can simply run the functions and output results.
I personally tested by inducing two cows to breed and observing their isBreeding state throughout the process.

Quick example code would be a command that executes:

for (Entity entity: player.getNearbyEntities(20, 20, 20)) {
    if (entity.getType() instanceof Animals) {
        ((Animals)entity).setBreeding(true);
    }
}

Relevant PR(s):

CraftBukkit: Bukkit/CraftBukkit#1400

JIRA Ticket:

BUKKIT-5687 - https://bukkit.atlassian.net/browse/BUKKIT-5687

[Wolvereness]

When I look at the javadoc, it explains nothing. Breeding is stateful, and it is more than binary.

• Is breeding the ability to attract a mob using food?
• Is breeding the hearts above their head?
• Is breeding when they are moving toward each other?
• Is breeding where they are close enough and are spawning a baby?
• Is breeding inaccessible after they spawned a baby?
• Does breeding wear off?
• Does breeding always spawn a baby?

This is an empty husk of an APi, relying on an in-depth knowledge of what the respective obfuscated method does. TL;DR: No one can use this but you.

Edit: Oh, the ticket needs to describe these kinds of things too - as it is, it's too ambiguous to use.

[mcmonkey4eva]

@Wolvereness - How does this explanation sound:
An animal is considered to be breeding when it has been given food (EG: wheat) and is looking for another animal of its species to mate with and produce a baby animal.?

[Wolvereness]

• Does it wear off?
• Does it spontaneously change once it finds a mate, or once it has a baby?
• Does it still return true under either of those conditions?

/edit:

• Why is there a Player parameter?
• Does it affect anything?

[mcmonkey4eva]

@Wolvereness I added answers to everything to the javadocs.
Is there anything more I can clarify / edit / reformat?

[mcmonkey4eva]

@ Anyone - is there anything more I can adjust here? Anything keeping it from being pullable? Or....?

[turt2live]

The javadocs are not wrapped correctly (78 characters per line, then hit enter, provided words don't get cut off weirdly).

As well the javadocs explain what breeding is in kind of a wrong sense. In Minecraft an animal is not "born", it is "spawned due to animal attraction". As well the javadocs contain a lot of scenarios that don't need to be there (such as use cases for the Player argument).

Here are some questions I have thus far:

• Why is the Player reference required? What if I want to make 200 cows all be ready for love because it's raining?
• Should there not be a stateful system as well? Considering the current javadocs state effectively 3 states (not looking for love, looking for love, and staring intently at a partner to produce a baby animal) should there not be a way to, say, force two entities to enter the third state (provided multiple conditions are met, such as same species and distance - which would need documentation).

For now though, that is all I have to comment on.

//edit://

Another question:

• Can the timeout for looking for a mate be changed?

[mcmonkey4eva]

@turt2live I thought the rules were no-80-char-cutoffs. Fixed.
The player reference is... for exactly what was written, specifying the player gives the player EXP and anything a plugin might given. Not sure how to clarify it in the docs.
The third state is (trying to induce breeding) happens if the breeding is set true and there's another mob nearby. It's all automatic, trying to force it wouldn't accomplish anything. Should I specify in the docs that it's an automated process?

The timeout could be changed... it requires pulling a file from mc-dev and adding functions... but that I can do.

[turt2live]

The 80 column limit applies to only code. JavaDocs are not code, they are documentation.

I understand that the player reference is used to reward the player, however I'm asking if it's possible to support no player being defined for some potential cases.

I also understand it's an automatic process, but again I'm asking if it's possible to skip the middle stage for the purposes of a method like LivingEntity breedWith(LivingEntity entity). This could be used for a few scenarios as well and may be desirable API if it can be added without rewriting a large portion of the existing server code.

I think that adding a variable timeout could be useful as well, but I'm unsure of what people who would actually use this new API would think about it. Looks like some community involvement is needed ;)

[mcmonkey4eva]

@turt2live
I added a note that the player can safely be null.
I just tried added a breedWith - it doesn't work without some heavy rewriting. The function b that should induce mating just makes the animal run around very quickly. A breedWith doesn't seem all that useful... it would just spawn a baby entity, and award XP / statistics / etc. as needed to the breeder player.
The timeout has been added. It was pretty straight forward and not at all problematic to add, no reason not to.

[keir-nellyer]

Maybe add another method without the player argument
On 14 Jul 2014 21:29, "mcmonkey" notifications@github.com wrote:

@turt2live https://github.com/turt2live
I added a note that the player can safely be null.
I just tried added a breedWith - it doesn't work without some heavy
rewriting. The function b that should induce mating just makes the animal
run around very quickly. A breedWith doesn't seem all that useful... it
would just spawn a baby entity, and award XP / statistics / etc. as needed
to the breeder player.
The timeout has been added. It was pretty straight forward and not at all
problematic to add, no reason not to.

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

[mcmonkey4eva]

@iKeirNez There... is one.

[Wolvereness]

@mcmonkey4eva #1085 (comment)

@iKeirNez There... is one.

public void setBreeding(boolean breeding, Player player, int timeout);

public void setBreeding(boolean breeding);

Doesn't look like it...

[mcmonkey4eva]

public void setBreeding(boolean breeding);
No player argument.

Or did you want one with timeout but no player?

[keir-nellyer]

A timeout method without the player argument would be more consistent.
On 15 Jul 2014 00:11, "mcmonkey" notifications@github.com wrote:

public void setBreeding(boolean breeding);
No player argument.

Or did you want one with timeout but no player?

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

[mcmonkey4eva]

@iKeirNez Added

[TheBizii]

Wait. Is this Minecraft plugin or Bukkit.jar for eclipse plugin development?

[turt2live]

#1085 (comment)

Wait. Is this Minecraft plugin or Bukkit.jar for eclipse plugin development?

This is a request to add code to the Bukkit project. If it gets added, it will be included in the Bukkit.jar so that other developers may use the code added in their plugins.

[Aaron1011]

It might be useful to have an event for breeding, so that plugins would have a way to get notified/cancel it when it happens through the API (instead of through a PlayerInteractEvent with a food)

[keir-nellyer]

@Aaron1011 That sounds like a separate PR as I think there should be an event for that anyway (API induced or not)

[turt2live]

@iKeirNez I would disagree. It makes more sense to push a breeding API in one piece over pushing it in multiple.

[Aaron1011]

@turt2live: That was my reasoning as well. I've seen other merged PRs that added in an API along with the corresponding event.

[turt2live]

You may want to review #791 while working on this as well.

[mcmonkey4eva]

@turt2live I've added the difference I saw - experience get/set functions.

[JeromSar]

As per java conventions. This should be xp in lowercase.

[mcmonkey4eva]

Will fix.

[feildmaster]

There's at least 2 other events that have been in bleeding, that look and feel more productive than the event you provide: (At least, I thought there were 2...)

This is, of course, an old commit. So it's missing xp handling. I'd have to look at your implementation for "breeder" though before I can comment on it.

https://github.com/Bukkit/Bukkit-Bleeding/compare/feildmaster/BreedingAPI

[mcmonkey4eva]

@feildmaster What is my event missing that the others provide?

[turt2live]

@mcmonkey4eva Yours is missing Animals[] getParents()

[turt2live]

@mcmonkey4eva A couple of issues I ran into while testing:

• From what I can tell, breeding should be a visual thing in-game, but it's not (using setBreeding)
• The baby in the event is always null
• After a call to setBreeding after some breeding has been completed, isBreeding returns false regardless of timeout.

There may be more issues, but those are the ones I found right away. Please provide proper testing materials and fully test your PR please.

[mcmonkey4eva]

getParents added

[mcmonkey4eva]

After a call to setBreeding after some breeding has been completed, isBreeding returns false regardless of timeout.
I've doc'd timeout as: how many ticks until the breeding state automatically cancels. The default is 600.
IE, that's how long the mob is still 'breeding', not the timeout for when you can breed again.
That'd be done by setting the animal's age to 0 / via (ageable).setBreed(true), a pre-existing function.

[turt2live]

So either way it's broken...

I've tested that entering Interger.MAX_VALUE doesn't actually do anything besides flash a temporary breeding status. I've also tested using setBreeding after breeding has occured, and that also fails to trigger a breeding status.

[mcmonkey4eva]

I've also tested using setBreeding after breeding has occured, and that also fails to trigger a breeding status. as it should. setBreeding only works when the entity canBreed.

[turt2live]

I don't think it should do that then. If a plugin wants an animal to be ready for breeding, it should be able to do so with this one method call. Modifying the age isn't really a decent solution.

[mcmonkey4eva]

Fair enough, I'll make it force breedability

[feildmaster]

Why is this an Ageable?

Can an Animal spawn a non-animal?

[mcmonkey4eva]

Ask Mojang. The internals use Ageables for the child entities. It might be something set up for modding/future edits where animals can spawn non-animals.

[feildmaster]

1- The baby should be the main entity, this event is about the baby. The parents/how it got created are secondary.

2- Instead of providing "breeder" to the constructor of the event, you should rather add a getBreeder in the Animal api. After that use the api to store an internal value that the event can't edit for a set state. I'm unsure of a set, because of how you worked your setBreeding methods.

3- Allowing the breeder to be modified in the event shouldn't happen. Events should be considered "read or cancel" except for special data like xp. If they want to change the breeder they're able to do so after canceling the event and calling their new event

4- After reading your implementation on breedWith... I don't like it. There's no reason to have it.

[mcmonkey4eva]

@feildmaster - all 4 done.
@turt2live - Figured out why the baby was able to be null and patched that.