QuiltMC / rfcs

Repository for requests for comments for proposing changes to the Quilt Project.
Other
61 stars 33 forks source link

RFC 0017: Standard Feature Toggle. #17

Closed LambdAurora closed 2 months ago

LambdAurora commented 3 years ago

This RFC aims to defined a standardized way to add feature toggles to mods and synchronize between servers and clients.

Rendered markdown

i509VCB commented 3 years ago

I am going to think about this for a bit - I have some ideas here.

Haven-King commented 3 years ago

I intend on having a config API with proper config value syncing. In a world where that exists, it doesn't make much sense to me to implement this as well, when there's so much functional overlap.

LambdAurora commented 3 years ago

I intend on having a config API with proper config value syncing. In a world where that exists, it doesn't make much sense to me to implement this as well, when there's so much functional overlap.

@Haven-King I Believe those two things try to achieve different goals.

A config API would be very good at synchronizing configurations between the client and the server (both directions I guess), but it's not really designed to allow features or ranges (if this RFC is modified to allow more complex data-structure than just an enum), while this RFC aims directly at allowing features and maybe ranges (I currently have no idea how this could be represented on the network).

The other issue is a config API implies that the mod is also present on the server, while this aims to be as generic as possible so you could have a common server mod managing permissions stuff and multiple client mods using this.

Haven-King commented 3 years ago

How is a feature or range different from a config value? As for the mod being required on the server, it implies no such thing. Config values are synced with an ID and value much like your features would be, and grants all the flexibility that goes along with that.

zeroeightysix commented 3 years ago

@LambdAurora

A config API would be very good at synchronizing configurations between the client and the server (both directions I guess), but it's not really designed to allow features or ranges

Can you clarify? I'm sure a capable configuration API would have all the necessary features in place to introduce things like ranges and 'feature toggle' metadata.

The other issue is a config API implies that the mod is also present on the server

I agree with @Haven-King on that this is not at all the case. A reasonable compromise would be to require the config API to be present on the server, which I don't believe to be a problem.

I do want to plug fiber here, a configuration system Pyrofab and I primarily worked on. It was meant to cover most usecases the fabric ecosystem needed, and grew quite capable of those. Unfortunately it hasn't been very active for the past year, but I'd love to blow new life into it if it turns out suitable as a solution to this RFC (or as a standard configuration system for quilt).

leo60228 commented 3 years ago

This seems like a very niche feature. I feel like this fits more in individual mods than in QSL.

Haven-King commented 3 years ago

@zeroeightysix I have been building out Conrad as a prototype to a standard config library for Quilt. Pyrofab has looked into it and approves of my general approach, so we'll likely be building off of that.

sylv256 commented 2 years ago

This appears to be meant for use in quilt only. However, I propose we allow other mod loaders to use the same packet standard. It's not really a "standard" if it's only going to be used in one thing. Now, I see why you would make this; you can technically implement this in alternative APIs, but my point is that if we want compatibility between fabric/forge servers and quilt clients (or vice versa), we'll want to do something like POSIX.

i509VCB commented 2 years ago

This appears to be meant for use in quilt only. However, I propose we allow other mod loaders to use the same packet standard. It's not really a "standard" if it's only going to be used in one thing. Now, I see why you would make this; you can technically implement this in alternative APIs, but my point is that if we want compatibility between fabric/forge servers and quilt clients (or vice versa), we'll want to do something like POSIX.

I've publicly said I would like this to be something that is platform agnostic, but I have not had time to work on that concept yet.

hibiii commented 2 years ago

I was thinking of vanilla servers and more mainstream servers and I was reminded that AIO PvP mods existed and they certainly have a similar feature... I could only find docs for LabyMod and they use JSON out of all things. I think the packet format is good, a good balance of ease of implementation and compactness, given how this is meant to be between servers and clients and use PacketByteBuf.

I personally believe what helps people adopt anything is accessibility. If this comes to pass, the only way that it would gain any traction in non-Quilt communities is plugins or mods for non-Quilt loaders that add the functionality, regardless of names. The packet namespace being quilt just means Quilt came up with this standard.

How is a feature or range different from a config value? As for the mod being required on the server, it implies no such thing. Config values are synced with an ID and value much like your features would be, and grants all the flexibility that goes along with that. @Haven-King

I think the main difference between this (the way I see feature override) and a config sync API is that a config sync is to config mods that are guaranteed to be there, whereas feature override would be for mods that might not even be there. In other words, feature override would be akin to enforcing servers' rules by code.

As an illustration, you could just say "no movement mods" as an admin and it wouldn't apply to like 90% of people but me, with a well behaved parkour mod, who wouldn't need to restart my client just to disable it, it would just disable itself because it wouldn't be looking for superparkourmodxd:enabled but rather quit:player_movement_mods.

chexo3 commented 2 years ago

In other words, feature override would be akin to enforcing servers' rules by code.

Config sync =/= feature override, and you should not create the tools to “enforce rules via code” with feature blocking that requires client cooperation anyway.

Don’t give server admins a false sense of security.

hibiii commented 2 years ago

It's not meant to replace or even enhance server moderation, cheaters will still cheat regardless, I know that much. This will just make it so people who don't want to cheat don't accidentally cheat. In fact, if you allow me to draw parallels, this is like HackControl for the MC Classic protocol extension (hack in this context being modded niceties), it controls if you can have nice things or not (like not flying in a parkour level), but not how nice nice things are (flight speed).

gdude2002 commented 2 years ago

I think it's a good middle ground for a lot of things - yes, it's not anticheat, and realistically Quilt's job isn't to prevent cheating.

However, there are a lot of things it would be excellent for - since the implementation is left up to the mods themselves, there's a lot of possibilities for a feature like this:

That's just a few things off the top of my head. Try not to think of this with a narrow mindset, because the possibilities really are quite extensive.

chexo3 commented 2 years ago

I don’t think it should be thought of as a feature toggle though. Really it should be part of config syncing. Mods can support changing config values during gameplay if they want, but they don’t necessarily need to.

It shouldn’t be a general type of thing. The server can already give clients cheaty things like flying and ESP, it doesn’t need a mod client side to do that. In modern Java edition a sort of feature toggle like this would end up just being used to give a false sense of security, and has privacy implications depending on how much data is shared with the server.

Also, tools for this already exist: plugin channels

QSL should have tools for mods to set up plugin channels, but I don’t think we should try to create a “standard” around them or have a plugin channel for feature toggles available by default. Let mods handle syncing or not syncing with the server. If you really want a “feature toggle” API, create it outside of Quilt itself and let mods use it, IMO.

gdude2002 commented 2 years ago

I can't think of any advantages your approach would have - standards are a good thing, and this is absolutely not configuration and not something that would be trivial to shoehorn into a more general config system.

There are some fairly large plans for a feature like this already.

LambdAurora commented 2 years ago

Also, tools for this already exist: plugin channels

QSL should have tools for mods to set up plugin channels, but I don’t think we should try to create a “standard” around them or have a plugin channel for feature toggles available by default. Let mods handle syncing or not syncing with the server. If you really want a “feature toggle” API, create it outside of Quilt itself and let mods use it, IMO.

This RFC is using plugin channels, and the prior art (LambdaControls) also uses them. The goal here is to give a way simpler way to deal with feature toggles stuff, the main reasoning is to abstract it to avoid having 500 standards of mods doing it, making it a pain for server admins to support.

The goal here is to offer a much more generic solution that is simpler to interact with.

And about the false sense of security, I don't see how this does it? It's not replacing anticheats, it's just giving new tools to both modders and server admins.

falkreon commented 2 years ago

Regarding features and security, let's step back and think about what the respective sides should be permitted to do and what they can be expected to do.

The Client

Can present features that it is loaded with, which it is unable to disable without a disconnect. These may change basic assumptions about networking and sync behavior, and missing one of these can cause the server to unintentionally crash a client. So this presentation of features by the client is for the client's benefit. I would call this INSTALLED features.

We also have the opportunity to offer a set of features which can enhance the relationship between the client and server in some way. Think, "WorldEdit GUI". Plugin channels can probe this data manually by sending a S->C message along the feature's channel and awaiting a response. No response means that the feature can't be used. But I'm thinking about teaching people how to do that custom packet handshake versus teaching people to register a feature, and it's pretty clear which one is more approachable. I'd call these features OFFERED

The Server

The server can report a list of features which are mandatory to have a smooth and feature-complete experience. Let's call this REQUIRED features. This set must be a strict subset of the client's INSTALLED + OFFERED feature set, otherwise this API should alow the server to give players the most helpful disconnect message it can.

I think it might be helpful, upon learning the client's OFFERED features, to report an ACCEPTED feature list, which completes an optional feature handshake.

"Deny" makes no sense from a server standpoint. It's probably a net security loss for the server to advertise blacklisted features, as the slightly-more-than-casual cheater will change one letter of the feature name and slide right in, or the feature will simply not advertise itself. If you want to say, "I'm incompatible with this" that's another thing, but that's not the sense I'm getting from this RFC and discussion.

TL,DR;

It makes sense to me for the client to positively enumerate INSTALLED and OFFERED features, and it makes sense for the server to positively enumerate REQUIRED and ACCEPTED features, and the names of these are important. It does not make much sense to me for either side to negatively assert anything.

gdude2002 commented 2 years ago

This sounds fine at a basic level, but feature negotiation should not be limited to the connect phase - it should be possible for a server to say "no more feature X" or "now we're allowing feature X" at any point. This is because it provides a convenient way for mods to support runtime toggling of features without having to make use of custom logic - for example, GOML claims where you can't use your minimap.

anonymous123-code commented 2 years ago

From my understanding, INSTALLED and REQUIRED are compability checks similar to problems regsync tries to solve. To me it seems like those are unchangeable while joined on a server and, similar to the server declaring incompabilities, out of scope for this RFC.

Now, with the OFFERED and the ACCEPTED features left, this is quite similar in behavior to the current RFC: The client sends a list of features it can offer, and the server responds with the features out of that list it accepts. The one difference, except from the different implementation, is the fact that features automatically get their own plugin channels. However, assuming that we want multiple mods providing the same features, this might encourage mod incompabilities as these will share channels - rendering providing plugin channels useless.

Additionally, I don't see why a deny functionality should be a security loss - malicious mods can simply not make use of this API.

The one security concern I have is that this functionality might provide information on the server's anticheat - which cannot really be avoided.

anonymous123-code commented 2 years ago

The one concern I have with both concepts is a privacy concern: The client still sends data to the server, probably revealing the mods using this system.

To avoid this, the server simply could send overrides for all features where the defaults differentiate from the server's standards. This should come with some sort of wildcard to catch e.g. all radar features, but then re-enabling one specific sub-feature.

I also wrote about such a system in the archived Thread #ModID list alternatives in #qsl-general on the Quilt Toolchain Discord grafik grafik

falkreon commented 2 years ago

The Discord poster has struck an important note here: A lot of the concepts in this RFC aren't entirely separate from the concern of permissions. That's not a good thing!

There are some places where permissions and networking concerns dovetail: when a server communicates permissions to a client (again, as permitted extensions) you can prevent desyncs. You cannot require a client to disable a clientside feature; you can only ask it nicely. But this is still a permission concern, not a capability concern.

We absolutely do need a way to negotiate capabilities, but don't get them confused.

sylv256 commented 1 year ago

This appears to be meant for use in quilt only. However, I propose we allow other mod loaders to use the same packet standard. It's not really a "standard" if it's only going to be used in one thing. Now, I see why you would make this; you can technically implement this in alternative APIs, but my point is that if we want compatibility between fabric/forge servers and quilt clients (or vice versa), we'll want to do something like POSIX.

Is this still planned? Seeing as how Quilt no longer interacts with Forge or Fabric, I'm not sure how this would be achieved.