GUIShop Should Rework How it Handles Invalid Input
A248 opened this issue ยท 2 comments
GUIShop works for the most part if the user enters correct items. However, adhering to all the requirements of a valid shop is not always a simple task, and for users, it can be easy to mess up.
GUIShop does not have a coordinated approach for handling invalid input. It often proceeds as if the input was valid, and when the feature in question fails, the error message varies depending on what exactly went wrong.
This results in a heavy support burden as well as lower-quality user experience.
It was about a year ago that I knew almost all of the exceptions thrown by GUIShop and could explain the solution when I saw a user encounter one such issue. As the code changes, the exact details of these exceptions change, and I no longer have this ability.
It would be better if some of the code would be reworked to handle invalid input better. At the least, including what went wrong in the exception message is better. I'd suggest maintaining general fail-fast behavior (as opposed to silently hiding problems) but clearly indicating what went wrong.
This has been the approach I've been taking since the 9.x series was released. I have been giving items a visible queue, both in lore, and BARRIER's showing on error, or invalid items. Console logging has massively been enchanced to handle almost every error provided, yet people still manage to slide a few through that will lead to a stack trace. The error logging has been enhanced with almost every minor version released after 9.x to both help the user, and our support when assisting someone who's run into an issue.
Here is an example of the error logging. This is using a 1.16.5 based config, with some errors (like missing shops, and newer items that don't exist in 1.8.8) in a 1.8.8 server.
https://i.gyazo.com/bfadb99ff212d617c8c20fe504d7aca6.mp4
You can see item's have a title showing the error in-game, but also print out a console error. The only issue i've been having is when custom-nbt is provided, you can see some console spam that does not come from GUIShop OR MiniNBT. It's actually Spigot spamming these messages into console and there is no way from me to prevent them, however I still print into console that a custom-nbt error has occurred.
Let me know what you think.
Perfect, that's good. To be honest I've been a little out of the game recently and I'll need to take another look at the codebase. I just thought I would open this issue with regards to general strategy, as I remember this was the thing that stood out to me when I was an active contributor about a year ago.
The error logging you made looks good. It is a nice solution to maintaining a configuration that reasonably works with multiple versions out of the box. It would be way too much work to provide different default configurations per version - I wouldn't even contemplate that.
Regarding the custom-nbt issue, you or I could investigate this and find out if there's a way to easily detect invalid custom NBT. We could attempt to perform the same checks that Spigot does.