Add entity API

[lonevox]

PLEASE READ THE GUIDELINES BEFORE MAKING A CONTRIBUTION

• Please check if the PR fulfills these requirements

• [x] The commit message are well described
• [ ] Docs have been added / updated (for features or maybe bugs which were noted). If not, please update the needed documentation here. This is not mandatory
• [x] All changes have fully been tested

• What kind of change does this PR introduce? (Bug fix, feature, ...) Feature.

• What is the current behavior? (You can also link to an open issue here) Calling EnvironmentDetectorPeripheral.scanEntities() gives only a small amount of information on each entity. I felt like this could be used to a greater potential.

• What is the new behavior (if this is a feature change)? Instead of getting a table with a small amount of data when calling EnvironmentDetectorPeripheral.scanEntities(), UUIDs for entities are now the only thing that is returned. These can be used with the new entity API. For example, entity.getNBT(uuid) returns the NBT data of the entity with the given UUID. The entity API itself can be disabled in config, as well as the individual functions within it that could be deemed exploitative on servers. A PlayerDetectorPeripheral.getPlayerUUID(username) Lua function has also been added so that you can make use of the entity API when using a PlayerDetectorPeripheral. I have also expanded the amount of entity information you get when calling LuaConverter.completeEntityToLua(entity). This function can be called via the entity API with entity.getData(uuid).

• Does this PR introduce a breaking change? (What changes might users need to make in their scripts due to this PR?) Yes, any calls to EnvironmentDetectorPeripheral.scanEntities() will need to use the entity API to get the data that it used to give you. Calls to AutomataEntityHandPlugin.searchAnimals() now have position information in a table with the key relativePos. This was a side-effect of cleaning up LuaConverter.completeEntityToLua(entity) to not require an ItemStack to be passed to it.

• Other information: I can make a PR for the docs if this PR is to be merged.

[lonevox]

Here's a script that I've been testing with: https://pastebin.com/uCH39VEm

[SirEndii]

Before I start reviewing

Since this adds a wider palette of features and due to the fact that this includes a breaking change, could you change the target branch to dev/0.8 so this features will be implemented to the next major version?

[lonevox]

Done

[SirEndii]

Greetings

The build seems to fail, I am not sure if the issues came with the change of the target branch, but they are there Could you fix them? I would also recommend running gradle check afterward to check for checkstyle rule violations.

You can either open the build log from teamcity by clicking on "Details" or you just could try to build it locally

[lonevox]

My bad, it builds now

[SirEndii]

You still didn't fix the checkstyle violations Either check the teamcity build log or run gradle check locally

[SirEndii]

Foremost, thanks for the PR and I really like your idea I also discussed that internally with some collaborators, and we agree that switching to an LuaAPI based entity api would be a nice addition to AP

The changes look good, but I didn't inspect them too deeply. I will take some time this week to review your changes and test them a bit for me

[tehgreatdoge]

Would it have made more sense to implement this with peripheral methods instead of polluting the global namespace? You could return a userdata from the scan method, return all the information by default, or just add another method to the peripheral for looking up data by uuid

[lonevox]

Would it have made more sense to implement this with peripheral methods instead of polluting the global namespace?

Originally I started with adding a getNBT(UUID) function to EnvironmentDetectorPeripheral, since I didnt want to put the huge NBT information in the small table returned by scanEntities, but if you think about it it makes no sense to have a getNBT function in EnvironmentDetectorPeripheral. It was either have a massive table returned or separate this into an Entity API which can have more fine-grained functions, so I opted for the later. It also isn't clear which functions in different peripherals return what entity data, but if you get a UUID from EnvironmentDetectorPeripheral.scanEntities and a UUID from PlayerDetectorPeripheral.getPlayerUUID, you know that Entity.getData(UUID) will return the same type of information for both entities.

You could return a userdata from the scan method

It was already the case before that some functions returned different entity data than others for no real reason. scanEntities only gave you like 10 fields per entity so it wasn't very useful, whereas searchAnimals gave you a lot of information, but still didn't include NBT data. It is easier to maintain and more consistent if this is all in one place.

return all the information by default

You could return all data, but there is a lot of information available now. It feels almost wrong to lump almost all information on the entity into one huge table. This would be NBT, almost all runtime data, and all persistent data. If you just need to get the positions of entities then this is all useless information.

or just add another method to the peripheral for looking up data by uuid

Something like getEntityData(UUID) wouldn't make sense in any one peripheral

[lonevox]

After looking at some of my changes again though, I think it would make sense for AutomataEntityHandPlugin.searchAnimals and AutomataEntityTransferPlugin.getCapturedAnimal to both return UUIDs instead of the result of completeEntityToLuaWithShearable to be consistent with my other changes

[SirEndii]

Would it have made more sense to implement this with peripheral methods instead of polluting the global namespace? You could return a userdata from the scan method, return all the information by default, or just add another method to the peripheral for looking up data by uuid

I am currently discussing that a bit with some of the CC community in the CC discord. I know that additions to the global API are not welcome in the community. AP already does unconventional things that I want to eliminate in 0.8, and I'd hate to add more.

[lonevox]

I am currently discussing that a bit with some of the CC community in the CC discord. I know that additions to the global API are not welcome in the community. AP already does unconventional things that I want to eliminate in 0.8, and I'd hate to add more.

Damn thats unfortunate, I wasn't aware of that. It's an issue of usability and code quality vs global pollution then. It's a shame that an entity API isn't available in CC itself.

I guess a rough compromise could be standardizing the entity data returned in different parts of this mod. So pretty much chucking everything into one huge table, but making sure that happens consistently in every peripheral function where an entity is retrieved. It would be worth documenting this standard entity table somewhere.

Another alternative is turning the Player Detector peripheral into an Entity Detector peripheral. All functionality of the entity API could be put in the Entity Detector. scanEntities could be removed from the Environment Detector and put into the Entity Detector.

[SirEndii]

Another alternative is turning the Player Detector peripheral into an Entity Detector peripheral. All functionality of the entity API could be put in the Entity Detector. scanEntities could be removed from the Environment Detector and put into the Entity Detector.

I support that idea Adding all the entity related features to one block would be the ideal compromise imo

you could also participate in the small discussion in the #modmaking chat on the cc discord

[SirEndii]

One idea I like is to still add all the entity API functions to the environmental detector but to separate them a bit, put them into a table which you can retrieve using getEntityAPI. This would be simpler than "adding the API to the global environment, but only when the peripheral is attached" imo

So maybe you want to go with that?

I will also convert this to a draft until we agree on a method

[lonevox]

One idea I like is to still add all the entity API functions to the environmental detector but to separate them a bit, put them into a table which you can retrieve using getEntityAPI. This would be simpler than "adding the API to the global environment, but only when the peripheral is attached" imo

So maybe you want to go with that?

I will also convert this to a draft until we agree on a method

For the idea of turning the Player Detector into an Entity Detector: Player Detector has four player-specific things: the playerClick, playerJoin, playerLeave events, and the getOnlinePlayers function. Everything else can be made to work for any entity (1 event and 10 functions). The getPlayers... functions currently return the names of players, but they could become getEntities... functions and return entity UUIDs instead. The functions migrated from the EntityAPI like getName(UUID) or getNBT(UUID) would have checks within them to see if the entity with the UUID is still within the range of the Entity Detector so that these can't be abused to get information about entities anywhere.

I think a getEntityAPI on the Environment Detector is a possibility, although you'd then have almost duplicate functions across peripherals (such as EnvironmentDetector.EntityAPI.getPos and PlayerDetector.getPlayerPos). An Entity Detector could replace all functions that get entity data in all other peripherals.

The biggest consideration with the potential Entity Detector is if its okay for it to have the 4 player-specific things. These 4 things cant really be removed, so its just if it makes sense or not. If it doesn't make sense, then it might not make sense to turn the Player Detector into an Entity Detector. I'm not sure, I'd like your opinion.

[SirEndii]

Merging the two peripherals is an interesting idea, but I don't really like it. Merging them would create one peripheral with maybe too many functions imo which wouldn't keep it simple

Developing the Player Detector and the Environment Detector separately would be my way to go. I would simply accept the duplicates, as there are apparently only 2 functions (getPlayerPos and getOnlinePlayers)

Keeping the player related functions (isPlayerInX... getPlayersInX...) and events to the player detector would make it simpler for the user imo to just have player related functions in one dedicated block while there would be the more advanced entity api in the Environment Detector

[lonevox]

Sweet. I'm going away for a week but I'll work on that when I get back

[SirEndii]

Greetings @lonevox Do you have any plans on continuing this?