Skip to content

WIP brain API#6968

Closed
Owen1212055 wants to merge 5 commits into
PaperMC:masterfrom
Owen1212055:brain-api
Closed

WIP brain API#6968
Owen1212055 wants to merge 5 commits into
PaperMC:masterfrom
Owen1212055:brain-api

Conversation

@Owen1212055
Copy link
Copy Markdown
Member

@Owen1212055 Owen1212055 commented Dec 1, 2021

This API is supposed to completely expose the "Brain" API that are used by certain entities such as goats, axolotls and villagers. However this api ALSO allows manually ticking brains of all entities.

This API is HEAVILY in WIP. I have created this pull request in order to collect as much feedback as I can here, so there is a lot of stuff like imports, access transformers, and paper comments I have to still do.

Some things I need help with:

  • Registering Custom Types
    • As of 1.18.2 you can no longer register stuff like this during runtime, how should I handle the registry freezing for registering custom memories/sensors/etc?
    • Memories
      • Memories that are serialized don't get loaded correctly, basically see the big comment in the Brain diff.
  • Is this level of exposure necessary?
    • Although there is a lot you have access to here, I worry that this may become quite a huge burden due to the amount of hacks that this requires.

I pushed a rough example in the test plugin, the pickled brain! See https://canary.discord.com/channels/289587909051416579/925530366192779286/989010753371652106 for the behavior.
ALL FEEDBACK WELCOME!

image
If you would like to use the brain debugger, use this fabric mod. This will allow the debugger to render.
enable-brain-debug-1.0-SNAPSHOT.zip

@Owen1212055 Owen1212055 mentioned this pull request Dec 1, 2021
Comment thread patches/api/0343-WIP-Brain-API.patch Outdated
@Owen1212055
Copy link
Copy Markdown
Member Author

private final MemoryKey<Boolean> IS_SCARED = Bukkit.createMemoryKey(NamespacedKey.fromString("scared", this), Boolean.class);
    private final SensorKey SCARIES_SENSOR =  Bukkit.createSensorKey(NamespacedKey.fromString("scary_mobs_finder", this));
    private final ActivityKey SCARED_ACTIVITY = Bukkit.createActivityKey(NamespacedKey.fromString("scared", this));

I updated how custom keys are created. This allowed me to get rid of the maps, and instead just do some instance checking in the utility classes (which have now been merged as well).

@Owen1212055
Copy link
Copy Markdown
Member Author

Owen1212055 commented Dec 21, 2021

I have included a new example that further shows what this system can use and have documented most of the api. Opinions appreciated. :)

@Owen1212055 Owen1212055 force-pushed the brain-api branch 2 times, most recently from cef7a46 to ae6f04d Compare January 3, 2022 01:52
@stale
Copy link
Copy Markdown

stale Bot commented Mar 5, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@maestro-denery
Copy link
Copy Markdown
Contributor

This will be a really nice API, I have a team which makes a "Vanilla with additions" server, so we develop our own APIs and libraries for "mod-like" development with plugins, we do many NMS hacks to reach "modded" experience for players, obviously making hacks is bad, I tried to make something like "Brain API" via VarHandles and reflection, but it looked very dirty, and probably the performance in this case would be very poor. so as a plugin developer I need fast and minimally hacky API for interacting with entity AI. I plan contribute in this WIP API, because I think that it will be useful for other people like me.

@maestro-denery
Copy link
Copy Markdown
Contributor

maestro-denery commented Mar 23, 2022

Hello everyone, I did some refactor of this draft API. (Owen knows about it) This refactor is about making better backward compatibility with Bukkit MemoryKey API, cleaner implementation and better maintainability of Brain API. I tested it, but anyway it can contain some bugs. Feel free to give a feedback about this refactor!

API changes:

  1. Created MemoryModuleType<U> API interface which has NamespacedKey getter and Optional<MemoryKey<U>> getter, this getter returns Optional having some value only if this MemoryModuleType is vanilla and represented in MemoryKey API.
  2. Created MemoryManager interface (singletone like BrainManager) where I transferred methods manipulating with memory (sensor, activity, behaviour methods aren't transferred) from BrainManager, edited them with MemoryModuleType and added additional setMemory() and getMemory() methods for manipulating with MemoryModuleType instead of MemoryKey.
  3. Changed usage of MemoryKey in BrainManager, Sensor and etc. to MemoryModuleType.
  4. Deleted methods for creation of custom MemoryKeys in Bukkit and Server and replaced them with creation of API MemoryModuleType both from Bukkit MemoryKey and NamespacedKey.

Impl details:

  1. MemoryManager has PaperMemoryManager implementation which has memory methods implemented and also contains two HashBiMaps, first BiMap contains registered custom types of API MemoryModuleTypes and second one is a cache for MemoryModuleTypes representing vanilla memories.
  2. API interface MemoryModuleType has PaperMemoryModuleType implementation containing NMS MemoryModuleType, its own unique namespaced key and optionally MemoryKey. This impl has two ways of creation, first one creates from NamespacedKey and second one creates from MemoryKey, first makes new NMS MemoryModuleType puts NamespacedKey and itself in registry defined in PaperMemoryManager and also puts new NMS MemoryModuleType into NMS Registry. I'll show later why NMS Registry is needed. Second gets value from the cache defined in PaperMemoryManager if it exists, if not, it gets NMS MemoryModuleType through CraftMemoryKey.fromMemoryKey(key) and inserts its NamespacedKey into API MemoryModuleType's one. And puts this instance to the cache and returns.
  3. I added in PaperBrainUtil three methods, first is getHandle() for getting NMS MemoryModuleType for API one, and toPaper() which gets API MemoryModuleTypes from NMS one. How? It is the place where NMS Registry helps. This method looks like:
public static PaperMemoryModuleType<?> toPaper(net.minecraft.world.entity.ai.memory.MemoryModuleType<?> nms) {
        // custom types actually register in NMS Registry, that's why we are getting them from it.
        final PaperMemoryModuleType<?> customMemoryType = (PaperMemoryModuleType<?>) PaperMemoryManager.INSTANCE.getTypeByKey(CraftNamespacedKey.fromMinecraft(Registry.MEMORY_MODULE_TYPE.getKey(nms)));
        // Here we are getting Vanilla type (from cache) if custom type doesn't exist.
        return customMemoryType == null ? PaperMemoryModuleType.of(CraftMemoryKey.toMemoryKey(nms)) : customMemoryType;
}

It is the main method which makes possible refactoring of everything else. And the third is handleIfVanilla(), this method operates with memory value and maps this value to NMS if Optional<MemoryKey<U>> presents in API MemoryModuleType, it is needed for proper manipulation with memory values of Vanilla MemoryModuleTypes in MemoryManager.

Code examples.

This is how this API MemoryModuleType creates:

public static final MemoryModuleType<List<Squid>> SQUID_CANDIDATES = Bukkit.createMemoryModuleType(NamespacedKey.fromString("squid_candidates"));
public static final MemoryModuleType<List<Parrot>> NEARBY_PARROTS = Bukkit.createMemoryModuleType(NamespacedKey.fromString("nearby_parrots"));
public static final MemoryModuleType<Location> MEETING_POINT = Bukkit.createMemoryModuleType(MemoryKey.MEETING_POINT);

This is how to manipulate with memory in this refactor:

    // Obtaining MemoryManager from BrainManager
    MemoryManager memoryManager = Bukkit.getBrainManager().getMemoryManager();
    // Getting memory value of given entity.
    List<Squid> squids = memoryManager.getMemory(entity, TestPlugin.SQUID_CANDIDATES).orElse(new ArrayList<>());
        if (target == null || target.isDead()) {
            if (squids.isEmpty()) {
                // Erasing it.
                memoryManager.eraseMemory(entity, TestPlugin.SQUID_CANDIDATES);
                memoryManager.eraseMemory(entity, TestPlugin.SQUID_RAGE);
                return;
            }

            target = squids.remove(0);
        } else {
            entity.getPathfinder().moveTo(target);
        }

Afterword.

As Owen said: "ALL FEEDBACK WELCOME!", I wanna see Opinions about this refactor, especially about its maintainability, and impl.

@Machine-Maker
Copy link
Copy Markdown
Member

Responding to @ExtraExtremeERA
Couple things here...

  1. That new interface MemoryModuleType<U>, I don't think it fits well unless you are just replacing all of MemoryKey with it, cause now there are two MemoryKey APIs essentially instead of just one. I prefer Owen's PaperMemoryKey (but that has its own problems too which ill comment on later)

  2. I like having concept of separating the Memory stuff into a separate interface, but I haven't really been a fan of the whole Manager interface. Why not just make that interface one that Entities extend directly and then we implement it in some PaperMemoryHolder interface that we make CraftLivingEntity extend?

@Owen1212055
Copy link
Copy Markdown
Member Author

  1. I like having concept of separating the Memory stuff into a separate interface, but I haven't really been a fan of the whole Manager interface. Why not just make that interface one that Entities extend directly and then we implement it in some PaperMemoryHolder interface that we make CraftLivingEntity extend?

Regarding this point, I would defo like your opinion on this. The issue is that the type of the entity is required in some cases, and in the future when most likely more entities are moved over to use the brain system it will cause the generic to be tossed everywhere. See: https://canary.discord.com/channels/289587909051416579/925530366192779286/925667741984243723

Ping me in contrib tho and we can discuss more, cause i'd like more feedback.

Copy link
Copy Markdown
Member

@Machine-Maker Machine-Maker left a comment

Choose a reason for hiding this comment

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

also a bunch of unneeded imports, line changes, stuff like that but those don't really matter right now.

Comment thread patches/api/0353-WIP-Brain-API.patch
Comment thread patches/api/0353-WIP-Brain-API.patch Outdated
Comment thread patches/server/0842-WIP-Brain-API.patch Outdated
+ Brain<?> brain = getBrain(livingEntity);
+
+ if (memoryKey instanceof PaperMemory<V>) {
+ brain.setMemoryWithExpiry(PaperBrainUtil.getHandle(memoryKey), value, expireIn);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

setMemoryWithExpiry expects a non null value

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hmm should I make a new expiry method instead that is not null then?

Copy link
Copy Markdown
Contributor

@yannicklamprecht yannicklamprecht Jun 25, 2022

Choose a reason for hiding this comment

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

Redis has a concept we could follow.

You can 1. set a non-null value and 2. set its expiry. When 0 it will directly expire. If possible I would not use a long in API instead a duration to make conversion easier and calculation a non issue.
Defining two constant durations is recommended as well.
Like EXPIRE_NOW (0) and REMOVE_EXPIRY (-1). Both as duration for sure.

Comment thread patches/server/0842-WIP-Brain-API.patch Outdated
Comment thread patches/server/0842-WIP-Brain-API.patch Outdated
Comment thread patches/server/0842-WIP-Brain-API.patch Outdated
Comment thread patches/server/0842-WIP-Brain-API.patch Outdated
Comment thread patches/server/0842-WIP-Brain-API.patch Outdated
@Owen1212055
Copy link
Copy Markdown
Member Author

Rebased to 1.19, redid the example, asking for feedback, you can now tick all entity brains, etc.

+ */
+public final class VanillaActivityKey implements ActivityKey {
+
+ public static final Map<NamespacedKey, ActivityKey> ACTIVITY_KEYS = new HashMap<>();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This field allow mutation for plugins which we don't really want here

@Owen1212055 Owen1212055 mentioned this pull request Dec 25, 2022
@fantahund
Copy link
Copy Markdown

Hey, would it be possible to add the BrainAPI for the camels? For example, that I can prevent the camels from lying down.

@maestro-denery
Copy link
Copy Markdown
Contributor

maestro-denery commented Dec 27, 2022

oh, nvm, I deleted my previous comment, because I forgot that it is the goal of this API for some reason. The only thing it needs to support camels is a rebase to put a BrainHolder interface in it. And if there are any new activities or memories or whatever for them there would be a need to implement them using code gen or whatever.

@Owen1212055
Copy link
Copy Markdown
Member Author

For it to be more properly integrated, this PR will need to be redone with all the new registry-related APIs.
Additionally, Mojang has redone its brain system's format to be significantly different than what this API proposes.

I am closing this as in general this will need to be rewritten, and don't have the time for that atm.

@Owen1212055 Owen1212055 closed this Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

7 participants