PaperMC / Paper

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

Missing API for accessing targets of mobs using new AI brain system #6862

Open Warriorrrr opened 2 years ago

Warriorrrr commented 2 years ago

Expected behavior

That getTarget() returns the entity the axolotl is currently targeting, and that setTarget(null) clears targeting.

Observed/Actual behavior

getTarget() returns null, even when the axolotl is attacking another entity. Setting the target to null also does nothing.

Steps/models to reproduce

Reproduced this using the following code:

@EventHandler
    public void onAxolotlAttack(EntityDamageByEntityEvent event) {
        if (event.getDamager() instanceof Axolotl axolotl) {
            if (axolotl.getTarget() != null)
                getLogger().info(String.format("Target type: %s", axolotl.getTarget().getType().name()));
            else
                getLogger().info("Target is null");

            axolotl.setTarget(null);
        }
    }

The output will be the "Target is null" message each time, and the axolotl continues attacking.

Plugin and Datapack List

Plugins (1): TestPlugin

There are 2 data packs enabled: [vanilla (built-in)], [file/bukkit (world)] There are no more data packs available

Paper version

This server is running Paper version git-Paper-359 (MC: 1.17.1) (Implementing API version 1.17.1-R0.1-SNAPSHOT) (Git: bc43f40) You are running the latest version

Other

No response

Machine-Maker commented 2 years ago

I’m not 100%, but I think this is works as intended. The axolotl, and all the newer entities do not use the same AI system as older mobs. They have a memory/brain system. There are methods to get and set memories on entities, and I think there is one for target entity.

lynxplay commented 2 years ago

I’m not 100%, but I think this is works as intended. The axolotl, and all the newer entities do not use the same AI system as older mobs. They have a memory/brain system. There are methods to get and set memories on entities, and I think there is one for target entity.

There sadly is not, the attack_target memory key has not been mapped to the API as of right now, possibly due to it containing a living entity instead of a UUID (which ANGRY_AT uses).

Machine-Maker commented 2 years ago

Ok, well there’s an open PR that adds them then.

lynxplay commented 2 years ago

Besides the fact that the PR needs a rebase, the question at least for me would be whether the [getTarget](https://papermc.io/javadocs/paper/1.17/org/bukkit/entity/Mob.html#getTarget()) and [setTarget](https://papermc.io/javadocs/paper/1.17/org/bukkit/entity/Mob.html#setTarget()) methods should be re-implemented for mobs that use the brain framework instead of goal/target selectors to properly match their described behaviour,

Machine-Maker commented 2 years ago

Besides the fact that the PR needs a rebase, the question at least for me would be whether the [getTarget](https://papermc.io/javadocs/paper/1.17/org/bukkit/entity/Mob.html#getTarget()) and [setTarget](https://papermc.io/javadocs/paper/1.17/org/bukkit/entity/Mob.html#setTarget()) methods should be re-implemented for mobs that use the brain framework instead of goal/target selectors to properly match their described behaviour,

The PR has been rebased now, and I don't think those methods should be re-implemented. They aren't the same thing, and the inhertiance would be wrong. LivingEntities can have memories, but only Mobs have that target field. So if you moved the get/setTarget methods to LivingEntity, you'd be adding that to some entities for which it has 0 meaning. Some documentation clarification should be somewhere, maybe a whole big paragraph explaining that some entities use a totally different AI system.

lynxplay commented 2 years ago

Oh sorry if I wasn't clear enough. I was merely suggesting implementing the set/getTarget methods for any Mob such as the Axolotl or the Goat to use the brain/memory keys to get and set the target instead of continuing to use the goal/targetfinders.

Basically as plain as (don't flame me if this isn't actually valid code):

public class CraftAxolotl {
  ....
  public LivingEntity getTarget() {
    return this.getMemoryKey(ATTACK_TARGET);
  }
  ....
}

or maybe in some form of CraftBrainMob superclass.

Machine-Maker commented 2 years ago

That might be fine for the getTarget method, but I don't think it would for setTarget. simply setting the memory for attack_target doesn't have the same effect as setTarget() on a mob that doesnt use memories. maybe that's fine, but I don't want them to get mixed up.

lynxplay commented 2 years ago

Makes sense :+1: So I guess we can close this as working as intended ?

Machine-Maker commented 2 years ago

Well, I guess since there isn't an actual way to do it (pending that PR), it can stay open and be closed by that maybe