Handle invalid config entries gracefully

[almic]

Server Implementation

Paper

Server Version

1.17.1

Describe the bug

Setting a negative value for max-tiles in the chunk processor causes a massive amount of console spam and seems to delete all tile entities in loaded chunks.

To Reproduce

1. Place some chests in a chunk
2. Enable chunk processor and set max-tiles to -1
3. Restart server and join
4. Chests are gone and massive console spam

Expected behaviour

At the very least, not spam the console and not delete blocks.

Screenshots / Videos

No response

Error log (if applicable)

https://paste.gg/p/anonymous/fe62defc051e457e95f59f5c29e9bef1

Plot Debugpaste

https://athion.net/ISPaster/paste/view/e45b3c123ebb475c80f7f18063589926

PlotSquared Version

6.2.3-SNAPSHOT

Checklist

• [X] I have included a Plot debugpaste.
• [X] I am using the newest build from https://www.spigotmc.org/resources/77506/ and the issue still persists.

Anything else?

Scared the crap out of me and I terminated the server as soon as I saw it so that none of the chunks would save in this state. This also happened on version 6.1.5, which I updated to the latest version (6.2.3) and tested again for this report. Results were the same. The provided error log is just the single error, as far as I can tell this is the only one but it spammed thousands of lines per second. I anticipated this would disable the check for tile entities in the chunk processor. I am using a different plugin (Insights) for block entity limits, which I disabled, so it is not interfering with PS and didn't cause this bug. If negative values simply can't be supported, it would be at least be nice if doing so anyway wouldn't delete blocks and just disable the processor. I really think that it should be supported as an "uncapped" limit though, as I want to use the max-entities limit but have an uncapped max-tiles limit. I am unsure if any value below 1 would have the same result, I just know that -1 does.

[NotMyFault]

Why do you set it a negative value? The chunk processor limits max tile entities, as its description says. 0 is the smallest amount you can set, you can't have -1 tiles in a chunk.

[Citymonstret]

Why do you set it a negative value? The chunk processor limits max tile entities, as its description says. 0 is the smallest amount you can set, you can't have -1 tiles in a chunk.

To be fair, it's fairly standard to allow negative values as a "disabled" value. It's probably a worthwhile change to make for limits, treating everything below 0 as "infinite" (aka no limit).

Thus, setting the value to 0 should disallow tile entities entirely, whereas a limit of -1 would be the same as 2147483647

[NotMyFault]

To be fair, it's fairly standard to allow negative values as a "disabled" value. It's probably a worthwhile change to make for limits, treating everything below 0 as "infinite" (aka no limit).

There are a bunch of options that fail if an input lower than 0 is provided, our config stuff was never really made for other inputs.

[almic]

I explained why I put a negative value already, Citymonstret reiterated that. It is, at the very least, pretty standard to make it so that invalid config values won't totally trash the server. Just preventing the plugin from starting with a negative value would be miles better than current behavior.

@NotMyFault it would at least be nice if the documentation said the value ranges supported. From what I could find there was no indication on the supported values for this. People (me) are stupid, they will input unexpected and incorrect values. A startup message saying "-1 is not a valid value" should be the standard here, not undefined and bugged behavior.

[SirYwell]

I agree, invalid values should be rejected instead of leading to undesired behavior.

[NotMyFault]

It looks like my former response came off wrong. I'm not against validating values, I was solely mentioning that we have more than this config option in place that needs to be validated properly and consistently.

For example, looking at the cache or rate limiting stuff, using a negative value here will throw a generic Guava exception that by nowhere near leads anyone to the config option of PlotSquared who didn't take a look at the codebase before. Negative values for the tab completion caching handler are not caught at all and throw the generic "an error occurred" message (from the looking alike) and setting a negative value for the lighting mode will throw a recursive NPE, because it's not handled either.