Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Common Protection API Integration // (Optional) FLAN Monitor Trust Permission #2058

Open
Hellscaped opened this issue Jan 11, 2025 · 8 comments
Labels
enhancement An extension of a feature or a new feature.

Comments

@Hellscaped
Copy link

If you haven't heard of it before, or haven't looked into it much:
The Common Protection API is a Patbox library that allows easy interop for protection mods and mods like CC.
It abstracts checks for entity protection, area protection, block protection, etc.
Majority of claim mods on fabric register a provider with it to make it easy to use.

CC: Tweaked hasn't implemented any of the checks to see if it can do an action against the CPAPI, but it checks against spawn protection.
Ideally it should check against the CPAPI to prevent players from using turtles/modems to circumvent protection.

Second thing: FLAN Monitor Trust
FLAN, the most common protection mod (implementing CPAPI) has its own permission node system. CC should simply add a permission node and check to allow players to permit people to interact with monitors in their claim. An issue was made on their repo, but the maintainer said that this is something that would to be implemented on the CC side, as permissions are now data based. Note that this issue is for the turtle issue, but is applicable to mostly everything. CPAPI fixes majority, but not monitor trust.

@Hellscaped Hellscaped added the enhancement An extension of a feature or a new feature. label Jan 11, 2025
@dimaguy
Copy link

dimaguy commented Jan 11, 2025

I'd be open to research and implement the CPAPI change if there's interest

@SquidDev
Copy link
Member

SquidDev commented Jan 12, 2025

I confess, I still don't quite understand the point of the Common Protection API. (On re-read, this comes across as passive-aggressive! I don't mean that, it's a genuine question! I can't tell if it provides actual value, or if it's just Patbox's "avoid FAPI at all costs").

On (Neo)Forge, permission mods have typically worked by listening to the various block events. Similar events also exist on Fabric (which CC:T does fire, and listen to the cancellation of!), and Flan does handle them, so I'd most things to just work!

The only case (which the issue mention) is turtle movement, which don't fire Fabric events. However, in those cases, we do use vanilla's built-in ServerLevel.isUnderSpawnProtection, which, as mentioned in that issue, Flan should probably be mixing in to anyway!

@dimaguy
Copy link

dimaguy commented Jan 12, 2025

The point of Common Protection API is to integrate with a finer common set of permissions provided by the various claim mods when its not just breaking/placing blocks but also different kinds of interactions with the world. Given Fabric's more simplistic approach on event loops, its only natural that a separate API had to pop up to cover those cases. For the server I'm staff in, we had to create a shim between Flan and CC (and plethora) for these purposes by mixing into the vanilla spawn protection checks to make this work

@Patbox
Copy link
Member

Patbox commented Jan 16, 2025

Common Protection API exists because mods sometimes needed interactions that that weren't really bound to events (or shouldn't trigger events, as it could cause mods mis-behaving because of listening to these events to do things). Before many mods did have direct compat checks instead, which could get annoying with multiple protection mods on the market.

Mixing into ServerLevel.isUnderSpawnProtection and applying protection logic in it won't work in many cases, as it misses a lot of context preventing more precise control over interactions.

Also my "avoid FAPI at all costs" is limited to my libraries if they don't benefit from using it (it also allows me to test things out on snapshots if Mojang does some big breaking change, which is nice). Pretty much all non-library mods require fabric api to work (excluding some simpler ones that just happened to not need it).

@SquidDev
Copy link
Member

I guess some background here — waaaaay back in 2016-2022, a friend ran a SpongeForge server with CC(:T), and it was a nightmare. I spent weeks tracking down bugs in SpongeForge/GriefDefender, trying to get them fixed in upstream, and/or finding janky workarounds for them. Now, this is not to say 1.10-era Sponge is anything like Fabric in 2025 (I know it's much better), but really has hammered home that this stuff needs to work out the box.

I definitely agree that there is scope for a protection API! There is absolutely times where you need those finer-grained checks (similarly, I don't expect FAPI to bundle a permissions system). But if every non-trivial content mod has to depend (soft or hard) on a third-party library, that's a bit naff.

This is why I bring up ServerLevel.isUnderSpawnProtection (and FAPI events). It's not a comprehensive or perfect check (heck, vanilla barely calls it), but it's a useful lowest-common-denominator that is easy for any mod to call, and any mod to hook into.

Given Fabric's more simplistic approach on event loops

I'm assuming you mean the event priority system, in that it's a DAG, rather than Forge's HIGHEST-LOWEST system? Has this been raised with the FAPI team? This feels fixable, if they're willing.

@Patbox
Copy link
Member

Patbox commented Jan 17, 2025

Fabric already has priority system, but it's fully dynamic instead of being bound to few selected levels (mods can add their own relative priorities).

I still wouldn't call ServerLevel.isUnderSpawnProtection lowest common denomanor through, as it prevents both breaking and interacting, which is not what many mods want (example case where it would be an issue are trade shops in claims, which need to be interactable to not breakable).

@dimaguy
Copy link

dimaguy commented Jan 17, 2025

Perhaps we could have a middle ground for this: if there's CPAPI available, CC:T uses it, if not, it uses the vanilla mechanisms. Theres no real one size fits all that works best for everyone due to each's pros and cons.
I don't think theres anything inherently wrong in dynamically improving behaviors when more resources are available instead of relying on what amounts to individual hacks and hooks into each other's mods when theres universal interfaces available

@Hellscaped
Copy link
Author

Perhaps we could have a middle ground for this: if there's CPAPI available, CC:T uses it, if not, it uses the vanilla mechanisms. Theres no real one size fits all that works best for everyone due to each's pros and cons. I don't think theres anything inherently wrong in dynamically improving behaviors when more resources are available instead of relying on what amounts to individual hacks and hooks into each other's mods when theres universal interfaces available

I mean theres nothing stopping you from doing something like "return !cpapiCheck(yadayada) || ServerLevel.isUnderSpawnProtection(yapyapyap)" to call both and if either return true refuse to do anything

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An extension of a feature or a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants