Skip to content

disguise api#11793

Closed
yannicklamprecht wants to merge 2 commits into
PaperMC:mainfrom
yannicklamprecht:feature/disguise-api-new
Closed

disguise api#11793
yannicklamprecht wants to merge 2 commits into
PaperMC:mainfrom
yannicklamprecht:feature/disguise-api-new

Conversation

@yannicklamprecht
Copy link
Copy Markdown
Contributor

pre hard fork PR: #10478

relates to #10584

Adds a simple disguise api that also makes sure that invalid entity data, not belonging to the type that is faked, is filtered.
It also adds a server generator module to reduce maintenance of the entity data filter.

tldr; The API provides a way to disguise any entity as another or a player with having the possibility to interact with the entity like with any others. The big advantage is that you've the whole vanilla logic and Mob Goal and Pathfinding included without hassle.

Testing the changes can be done using the following snippet:
https://gist.github.com/yannicklamprecht/639acba127a54f5617e1281f099b7aff

Effective loc:

600 loc in patches
3k+ loc generated filter classe
400 loc generator code copied from api-generator
100 loc for generating the filter class.

@yannicklamprecht yannicklamprecht requested a review from a team as a code owner December 23, 2024 19:48
@yannicklamprecht yannicklamprecht mentioned this pull request Dec 23, 2024
9 tasks
@kennytv kennytv added the type: feature Request for a new Feature. label Dec 24, 2024
@lynxplay
Copy link
Copy Markdown
Contributor

Given we already chatted about this on other channels, I'll be closing this PR as we do not believe such an API is the right fit for the main server API, especially when looking towards evolving this API continuously (outside of the version-specific values) and the general upkeep such an API includes.

We do however acknowledge that such functionality is essential for a lot of developers out there and that implementing such functionality purely via a plugin is cumbersome and limiting. To that degree, we'd offer you the following next steps if you are interested:

  1. Move this API to a separate plugin, for now under your own namespace.
  2. Getting the generator to work outside of the server environment is something we might be able to help with via discord. In this case, only the vanilla remapped jar + libs
    should be needed, which mache might help with?
  3. To skip potentially slow reflective calls, we'd accept "internal" API for your API as a feature patch along the line of io.papermc.paper.network. ChannelInitializeListenerHolder. It being a feature patch means we do not need to update it to newer versions during upgrades immediately. This way, the server can operate
    without potential slowdowns but can be "enabled" for the disguise API if your plugin/api is installed.

@lynxplay lynxplay closed this Dec 26, 2024
@yannicklamprecht yannicklamprecht deleted the feature/disguise-api-new branch December 26, 2024 16:48
@yannicklamprecht
Copy link
Copy Markdown
Contributor Author

That's not gonna happen. With the planned setup we can just go back to using citizens.

@lynxplay
Copy link
Copy Markdown
Contributor

Also fair if that is not something you are interested in.
Thank you anyway for the work you put into the PR 👍

@kennytv
Copy link
Copy Markdown
Member

kennytv commented Dec 27, 2024

To clarify, we would accept a base level of (internal) API to allow for easier custom entity or NPC creation, but properly representing all of what the "target" of this PR implies is a lot of maintenance and sort of magic, unstable API, for a use case that's pretty brittle too. Hence it doesn't really fit into our API module in its entirety. If the goal is just to spawn player entities, then the scope of the API itself should reflect that and ideally avoid the full-blown entity data generation

@Leguan16
Copy link
Copy Markdown
Contributor

Being able to create basic fake entities/players without relying on a plugin would be very nice. Sometimes many features certain plugins provide are just too much for a simple use case and thus a potential waste of resources.

I don't really understand why this PR was suddenly (given the fact that most of the talking happened internally it seems to have happened out of nowhere) closed with lynxplay's comment that said "it would add a bunch of maintenance work (which is understandable) and doesn't really fit into the API" and later clarifying that a more trivial and internal api is more fitting and thus more likely to be accepted.

Especially powerful apis like this is what many people like about paper. Simple ways to create powerful plugins.

That being said I hope someone tries to add something like this in a more trivial way.

@lynxplay
Copy link
Copy Markdown
Contributor

We can expose powerful APIs that wrap/enable modification of the server for things like data components / registry modification without much hesitation because they are entirely new systems.

Disguising an entity as another is a terribly complex API if implemented fully. The issue of such an API in the paper repository is that, if you glanced at the required API of existing alternatives (e.g. citizens or lib disguise or whatever) people tend to do more with their disguised/npc entities than just spawning them in.
The existing types surrounding entities (org.bukkit.entity.Entity) were never designed for wrapping anything that isn't an actual entity in the world, making them a poor way to allow consumers to do such down the line changes (think, I want my phantom disguised as a shulkerbox to open up mid flight by calling Shulker#setPeek).

My only hope for something like this would be mojang moving entities into a data component setup similar to ItemStacks, which would mean we have to/should setup a new entity data API anyway, which we could then design with things like this in mind. But for now, merging an API that is bound to be limited due to the very nature of existing entity data API is just setting up for failure. The alternative that is just implementing very internal-like entity API for the sake of the disguise API feels even worse as we'll set us up with two failing APIs once mojang makes entities data component driven.

We could have 100% communicated it better, the discussion on this was indeed internal and very late (given the old PR was open since april) but github PRs are not really a nice place for real-time discussions with multiple team members, I am sorry my message did not cover all our talking points, the above are at least my opinions.
The PR was still very useful, specifically the generator and synced data id collection will be something that needs to be done for any down the line approaches to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feature Request for a new Feature.

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

4 participants