Minebench / Tresor

An experimental VaultAPI compatible abstraction layer for Bukkit/Spigot/Paper plugins
https://ci.minebench.de/job/Tresor/
GNU General Public License v3.0
8 stars 3 forks source link

NPC Draft #26

Open IllusionTheDev opened 7 months ago

IllusionTheDev commented 7 months ago

I made a pretty rough NPC system. This is some serious 3AM code, so I'm expecting some changes.

Closes https://github.com/Minebench/Tresor/issues/23


IssueHunt Summary ### Referenced issues This pull request has been submitted to: - [#23: NPC](https://oss.issuehunt.io/repos/121785670/issues/23) ---
IllusionTheDev commented 7 months ago

@Phoenix616 I've gone ahead and pushed some changes. Let me know if this resolves some of your feedback.

Phoenix616 commented 7 months ago

Looks good overall, I like the approach with the interaction object also including the item as that's probably what one wants most often. (Although I guess that opens one up to main and offhand issues but I feel like if that is important someone could just get it from the player directly)

One feature that I think might also be interesting to have is per-player NPCs (seeing as Citizens supports that naively it seems like something one would want to) as well as maybe the ability to get the raw entities connected to the NPC? (Not sure if that is even something needed or good to add here though seeing as some plugins might be doing client-side entities...)

IllusionTheDev commented 7 months ago

Looks good overall, I like the approach with the interaction object also including the item as that's probably what one wants most often. (Although I guess that opens one up to main and offhand issues but I feel like if that is important someone could just get it from the player directly)

Citizens doesn't provide a way to get the hand used in the interaction. I feel like it should be an implementation detail.

One feature that I think might also be interesting to have is per-player NPCs (seeing as Citizens supports that naively it seems like something one would want to) as well as maybe the ability to get the raw entities connected to the NPC? (Not sure if that is even something needed or good to add here though seeing as some plugins might be doing client-side entities...)

Not every NPC plugin allows for client-sided NPCs. We should probably put that behind a Feature enum

Phoenix616 commented 7 months ago

Yeah, hand makes sense to keep internally and only expose a generic left or right "interaction" as most probably don't care (or even know :S) about the both hand interaction stuff.

As for the client-side NPCs: I really don't know how common that is tbh., I don't know an example of that myself but a method to get the NPC entity(ies) is probably a must. (Even if it just returns an empty list for plugins that use client-sided ones)

IllusionTheDev commented 7 months ago

As for the client-side NPCs: I really don't know how common that is tbh., I don't know an example of that myself but a method to get the NPC entity(ies) is probably a must. (Even if it just returns an empty list for plugins that use client-sided ones)

I'm unsure if a list fits this scenario. We'd be better off coming up with a Model system in the future. Maybe a nullable entity getter?