Carpet

Carpet

Mods
2M Downloads

Suggestion: More robustness for client side rule value receiving and parsing

Fallen-Breath opened this issue ยท 0 comments

commented

About the issue

When a carpet client receive rules from the server via carpet protocol, it will simply use ParsedRule#set to set the value, without any exception handling

fabric-carpet/src/main/java/carpet/network/ClientNetworkHandler.java

Lines 60 to 65 in d6d3167

ParsedRule<?> rule = (manager != null) ? manager.getRule(ruleName) : null;
if (rule != null)
{
String value = ruleNBT.getString("Value");
rule.set(null, value);
}

If there is any exception thrown due to incompatible rule value during value setting, the exception will go all the way back to net.minecraft.network.ClientConnection and the client will be disconnected

Rule value parsing failure might happen when the client and the server have different versions of carpet (extension) mods. For example the mod changes the type of the rule, or adds a new enum value to an enum rule for new funtionality

Example

image

The screenshot above is what happens after the client received an unknown enum rule value. The exception should occur here

fabric-carpet/src/main/java/carpet/settings/ParsedRule.java

Lines 144 to 148 in d6d3167

else if (type.isEnum())
{
String ucValue = value.toUpperCase(Locale.ROOT);
return set(source, (T) (Object) Enum.valueOf((Class<? extends Enum>) type, ucValue), value);
}

Suggestion

In my opinion when received an unknown/illegal rule value from the server, instead of crashing hard and make the client disconnected, it would be better to ignore the incoming rule value and pretend nothing happened. Although this might creates some client-server desync for some rules that require both c/s sides to work together, that's much better than just give up and disconnect the client

A simple way to solve that could be adding a try-catch to surround the rule.set(null, value);