PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
9.73k stars 2.27k forks source link

Add Mob Goal API #2617

Closed MiniDigger closed 2 years ago

MiniDigger commented 4 years ago

Just like the Mob Pathfinding API wrapps the nms.Navigation, I propose an API that wrapps the PathfinderGoal interfaces

Goals:

Implementation idea:

dealing with vanilla goals is more difficult, there are 185 subclasses of Goal in 1.14.4... I would say we structure them only wrap direct subclasses. For example, there is a AvoidEntityGoal, that has 5 subclasses (Cat, panda, wolf, ocelot and rabit). for a plugin, its enough to expose that super class goal. similarly, move to block goal has 9 subclasses, etc. that would not allow getting a full list of goal for an entity tho... I would say for now we can try to avoid this issue and implement a simple way where the getGoals methods return a VanillaGoal that has a name (class name), so they can be identified and removed if needed.

I would step up and implement such an api against master/1.13, since thats where we at dyescape need it, and hope that somebody else can assist in porting it to 1.14.

any feedback, especially considering the handling of vanilla goals, is greatly appreciated. I plan to start working on this in like 12 hours.

MiniDigger commented 4 years ago
22:06 <+DiscordBot> <e​le​ct​ro​ni​cc​at​> That's basically what I was kinda thinking of doing
22:06 <+DiscordBot> <e​le​ct​ro​ni​cc​at​> Would just need a wrapper class for nms -> cb and a wrapper for plugin impls
22:06 <+DiscordBot> <e​le​ct​ro​ni​cc​at​> (ANd then a bit of glue so that people can actually shove them in/pull them out)
yannicklamprecht commented 4 years ago

For documentation: I would at least use: _your_plugin_name:spawn_healingarea, _minecraft:move_toblock, _minecraft:avoidentity and so on

Another idea would be (Could hardly depend on Mojang): Better would be, when Mojang decides to get rid of the GoalConstructor with more than an entity or better using a ctor (Entity, GlobalGoalAttributeStorage). That the goal gets all their data from that entity and the GlobalGoalAttributeStorage. Like how they did it with the Attributes for entities.

We can use the above idea to overload the constructor of each PathfinderGoal and implementing something like that.

....
public SpecificPathfinderGoal(Entity entity, GlobalGoalAttributeStorage storage){
 this(
   entity,
   storage.getOrDefault(new NamespacedKey("specific", "max_radius_move_to_block"), 6),
   storage.getOrDefault(new NamespacedKey("specific", "max_attack_radius"), 20)
);
}

public SpecificPathfinderGoal(Entity entity, int maxRadiusMoveToBlock, int maxAttackRadius){
 // the Minecraft SpecificGoal constructor
}
...

NamespacedKey's should be initialised elsewhere, so that a plugin can be used for it.

We can override the values by using

Server#getGlobalAttributeStorage()
GlobalAttributeStorage#set(new NamespacedKey("specific", "max_radius_move_to_block"), 30);

For improvement of generics I would recommend some Class like the GameRuleKey's in Bukkit.

GlobalAttributeStorageKey<Integer> SPECIFIC_MAX_RADIUS_MOVE_TO_BLOCK = new GlobalAttributeStorageKey<>(new NamespacedKey("specific", "max_radius_move_to_block"),Integer.class);

Possibly adding a default value in that key which is initialised by the developer creating this key (hardcoded, config, etc.) or by MinecraftServer defaults if easy doable.

MiniDigger commented 4 years ago

went ahead and implemented a first draft (without yannick ideas)

https://gist.github.com/MiniDigger/f98e612946c6dbebe09913b85ae06bfa

theminecoder commented 4 years ago

Looking good so far. Would it make sense to also expose the navigator as part of this so people can make use of it in goals?

jaqobb commented 4 years ago

Looking good so far. Would it make sense to also expose the navigator as part of this so people can make use of it in goals?

Isn't this patch enough for a navigation API? https://github.com/PaperMC/Paper/blob/master/Spigot-API-Patches/0147-Mob-Pathfinding-API.patch

theminecoder commented 4 years ago

Ah yes, seems it is. Must have skipped that when looking at the long list of patches :P

blablubbabc commented 4 years ago

I attempted something similar in the past as well. I even ended up creating interfaces with short descriptions for all the different vanilla PathfinderGoals and per-mob goal lists to allow plugins to identify individual goals (when they want to remove a specific goal / AI task from a mob). But methods/parameters and some kind of factory was still missing (this was taking way too long to do alone, so I eventually gave up / paused the effort on that). Registering and running custom pathfinder goals was working fine though. I also remember that while implementing this I noticed some inconsistencies with how minecraft's goal selector picks the goal to run (I think it ignored the task priorities in some cases, which might be confusing / cause issues when plugins register their own behaviors) which lead me to basically re-implement that slightly differently..

One issue with identifying goals by their (class) name is that mobs can use multiple different instances of the same pathfinder goal (eg. 'avoid mob A', 'avoid mob B', ..). So one actually needs a name / namespaced_key per goal instance rather than per goal type.

For reference: https://hub.spigotmc.org/stash/users/blablubbabc/repos/bukkit/compare/diff?sourceBranch=refs%2Fheads%2Fentity-ai&targetRepoId=11 https://hub.spigotmc.org/stash/users/blablubbabc/repos/craftbukkit/compare/diff?sourceBranch=refs%2Fheads%2Fentity-ai&targetRepoId=12 All vanilla pathfinder goals and their categories / activation flags: https://pastebin.com/Gchet1m8

Another open question was whether mobs should keep their goals / state when transfering between worlds. In vanilla minecraft they simply recreate the mob and thereby freshly setup the default goals, which might be confusing when used by plugins (for which the entity before and after the world transfer is considered the same). But implementing this might be tricky..

And one would need to think about whether to automatically remove custom plugin-provided behaviors again when the corresponding plugin gets disabled. This will likely require tracking when plugins add goals to mobs, or lazily check if the plugin is still enabled before running or returning a goal / AI task.

I am wondering though how minecraft's villager behaviors added with MC 1.14 would fit into this. I haven't checked this out in detail yet, but I assume they are basically another AI system, separate to the previous PathfinderGoal system used by all the other mobs. Not sure whether adding custom goals to villagers would therefore cause conflicts.

MiniDigger commented 4 years ago

wow, thanks, thats some great work. I am not sure if such a diff is too large for paper to maintain. I was just going with namespaced keys, since those can be auto generated.

I'll look more closely at the changes tomorrow, especially what you did to the goal selector.

I noticed that vanilla uses multiple goals of the same type on some entities, I guess we would need to return a list of goals for cases like this, not sure yet.

For switching worlds I am actually not sure how my PR handles that, since the mob goals instance is bound to the CraftMob instance. needs testing.

didn't look at villagers (or really anything) in 1.14 either, so that needs investigation too.

biels commented 4 years ago

Any news on that? Has this (or something similar) been finally implemented somewhere (Paper or Spigot)? Looks like the patches are almost there in this issue!

MiniDigger commented 4 years ago

@biels see https://github.com/PaperMC/Paper/pull/2619 its close to being merged

MiniDigger commented 4 years ago

most of this has shipped with #2619, only adding vanilla goals is missing

EOT3000 commented 4 years ago

Villagers do not use goals. I've looked through and apparently they use Behaviors and Activities, since I haven't seen any MobGoals in the villager classes, and stuff like this:

    public static final Activity CORE = a("core");
    public static final Activity IDLE = a("idle");
    public static final Activity WORK = a("work");
    public static final Activity PLAY = a("play");
    public static final Activity REST = a("rest");
    public static final Activity MEET = a("meet");
    public static final Activity PANIC = a("panic");
    public static final Activity RAID = a("raid");
    public static final Activity PRE_RAID = a("pre_raid");
    public static final Activity HIDE = a("hide");

exists

CaptureBehavior

aikar commented 4 years ago

Yes, this API only covers the Legacy AI system for MC. We can explore adding the New AI system in 1.16 or 1.17 once its shown signs of stabilizing.

orblazer commented 3 years ago

Hello @MiniDigger, in PaperVanillaGoal we have invalid start, stop and tick. This call the same method in handle but she need call respectively c, d, e (in nms 1_16_R2). So we need make the methods start2, stop2 and tick2 like shouldActivate.

Laarryy commented 2 years ago

@MiniDigger I'm not sure of the status of the 'New AI System' but I figure you're the best to poke - if this is no longer required, please do me the distinct honor of closing this issue and marking it as stale or superseded.

MiniDigger commented 2 years ago

I have no idea how the new brain stuff works, what is missing on this issue is creating vanilla goals without touching internals, which idk we will ever tackle before mojang refactors their stuff...

Owen1212055 commented 2 years ago

Closing as generally being able to expose all of the vanilla is not feasible, and with vanilla no longer adding more entities to the goal system it is hard to predict what will happen with those goals anyways.

The majority of this issue was already implemented, and so if you do indeed want to use vanilla logic in your goals it is recommended that you simply implement it yourself with API calls.