Skip to content

Conversation

@Y2Kwastaken
Copy link
Contributor

@Y2Kwastaken Y2Kwastaken commented Feb 15, 2025

The goal of this Pull Request is to add a basic level of support for Point of Interest in the API. This more used within entity AI, however, there can be exposed a useful detect Point of interests in the world. I want to flesh this out if possible, however I also want testing to be done on the current API to make sure it works as intended.

I will work more on testing and seeing if this API can be fleshed out more in the coming days/weeks.

@lynxplay lynxplay added type: feature Request for a new Feature. scope: api labels Feb 16, 2025
@Y2Kwastaken
Copy link
Contributor Author

Y2Kwastaken commented Feb 16, 2025

Is there any way in specific we should handle this case

Just tested this, and will I get the expected result for Villager workstation (specifically farmer).
I do not get the result for beehives.
For beehives it gets only found for occupancy ANY, and returns null for the other two ones. Regardless if it is empty, 1 / 2 Bees or full.

I got this comment on spigot a few months ago. Do we either

  1. specify the discrepancy in the API documentation
  2. each PoiType internal would run through something like CraftMenus but for PaperPoiTypes etc that will map to some Predicate that can make up for the non straightforward discrepancies. This isn't super straightforward and kind of annoying, but it does "fix" some things internals is a bit uneven about.

@Owen1212055
Copy link
Member

I think that documenting the fact that some POITypes are used more as "markers" for the block rather than storing occupancy would be useful.

Could maybe even add a method onto the poi type class if it has 0 max tickets so people can check themselves.

*
* @return true if this PoiType is acting as a "marker" otherwise false
*/
boolean isMarker();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Owen1212055 thoughts on this method? I think I might need to tweak the docs a bit more on this one. I also changed the comment on the PoiType interface to go into more depth about what this method entails.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe instead we could say "hasOcccupants" whether or not this POI can hold occupants. Reuses terminology

@Y2Kwastaken Y2Kwastaken marked this pull request as ready for review May 2, 2025 21:49
@Y2Kwastaken Y2Kwastaken requested a review from a team as a code owner May 2, 2025 21:49
@lynxplay lynxplay moved this from Changes required to PR Party candidate in Paper PR Queue Dec 20, 2025
.map(record -> PaperPoiSearchResult.from(record, this))
.toList();
/*
with Moonrise:
Copy link
Contributor

Choose a reason for hiding this comment

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

Yea, we need a feature hook for this and then moonrise can patch that over.
I don't think we need a moonrise PR for that, moonrise does not maintain the feature hook class in its repo.

@github-project-automation github-project-automation bot moved this from PR Party candidate to Changes required in Paper PR Queue Jan 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: api type: feature Request for a new Feature.

Projects

Status: Changes required

Development

Successfully merging this pull request may close these issues.

4 participants