CivClassic / JukeAlert

A plugin that creates a new block in Minecraft that records player entries and actions. Anti-grief tool. Built for Paper 1.16.5
BSD 3-Clause "New" or "Revised" License
0 stars 18 forks source link

Rework /jalist into text #10

Open Maxopoly opened 4 years ago

Maxopoly commented 4 years ago

Current GUI one is bad and unreadable.

Needs a session based approach to reduce lagg. Also get rid of the async snitch loading and its 100 consistency problems, just keep an in memory mapping of group id --> snitches.

KaloHG commented 3 years ago

Bump! (it's been 7 months. Snitches are starting to cull/culled.)

wingzero54 commented 3 years ago

/ja list gui was vastly improved by Orinnari with #32 and #33 , may be worth re-visiting whether we also want to have a text /jalist because the gui is quite nice at this point.

Inframission commented 3 years ago

Thinking this is still important to the extent that people want to view their snitches in more ways than voxelmap highlights. Without a text version of jalist there are few ways for bots or mods to get snitch data, and thus for players to make snitch management tools

awoo-civ commented 3 years ago

/ja list gui was vastly improved by Orinnari with #32 and #33 , may be worth re-visiting whether we also want to have a text /jalist because the gui is quite nice at this point.

The issue with the GUI doesn't come down to a couple of minor issues. A GUI as we can have it in Minecraft is fundamentally unsuitable for /jalist. Let's consider the usages of /jalist:

1. Checking for snitches that need to be refreshed (looking at cull times).

The GUI is unable to display any text without hovering over an item so you have to hover over each item in the GUI in order to see its value. This makes for a horrible user experience.

With text you can simply see any of the values you desire at a glance.

Moreover, there's little to no advantage to a GUI here. GUIs make sense for highly interactive plugins like NameLayer and Citadel. /jalist just lists information and text is generally better suited for that.

That said, there is actually a way to make this far better in the GUI. Use the blocks in the GUI to convey cull state instead of the less useful entry/logging snitch distinction. Cull times can be simply represented by blocks that can have different colors, like clay. For example, red clay – 1 day to cull, orange clay – 1 week, etc. That way you to some extent retain the very important glanceability of cull times that text provides.

Ideally the display mode would be configurable, so you could pick what's shown – clay for cull times, notes/jukes for entry/logging snitches. Cull times should be the default though, as they're what /jalist is most often used for.

2. Scraping the entire list for mod/bot use.

Text is much better here, obviously.

Now, I'm well aware of the administration's negative stance on client-side modifications, but let's be realistic here. Maintaining snitches is enough of a pain even with all the visualisation tools being available to the player. Unless tedious manual work of navigating to snitches is intentional, not at least providing an optional text interface is simply counterproductive.

Diet-Cola commented 3 years ago

We've definitely gone "GUI mad", even in my opinion the more recent /ep showall has no use being a GUI but its what is was implemented as. We definitely do need to actually start cutting down on its usages as well since the most recent server timings has shown that /jalist can even create a lag spike

Protonull commented 3 years ago

…since the most recent server timings has shown that /jalist can even create a lag spike

Wasn't that /ja and not /jalist?

Diet-Cola commented 3 years ago

Anything using CMC's GUI stuff right now is combining together to create lag as seen here: https://spark.lucko.me/TUJJUzj08H

Protonull commented 3 years ago

Heh, okay so that answers my question to PaperMC the other day :P it would make sense that lag would be concentrated in anything CMC since it's handling the requests of handfuls of plugins, but I digress. That particular problem is caused by player heads in GUIs. Off the top of my head, /ep showall and /ja display player heads to depict players: /ep showall depicting the prisoner, and /ja depicting the player entering, logging in, logging out, etc.

awoo-civ commented 3 years ago

We've definitely gone "GUI mad", even in my opinion the more recent /ep showall has no use being a GUI but its what is was implemented as.

It's a great example:

image

I don't know players by their heads so while they look pretty, they serve no purpose and convey 0 useful information. I need to hover over each single head to be able to see anything and I have no way of visually establishing say, how many players were pearled in the last week.

Diet-Cola commented 3 years ago

image I did play around with jalist as text but I haven't spend more than an hour working on it, we 100% need to add a pagable text element api to CivModCore like we have with the GUIs, theres a lot of stuff we could do with something like that. for example I don't really thing /nl does its job very well, I would rather /nl lists your current groups in text, you click on the group you would like to edit and then it would launch in to that groups GUI

KaloHG commented 3 years ago

I can't contribute much to this sadly, as I'm not an amazing programmer. But as a concerned player :>

Please do make sure to make it so legacy mods such as SnitchMaster could read this, I don't believe this is hard to do, you just do the same styled formatting as what it was originally in 1.12.2

Thank you!

Also appreciate awoo and diet for coming in clutch :)

Protonull commented 3 years ago

Something that should be kept in mind when making any kind of command, is whether or not they'll be, for want of a better description, synchronous or asynchronous. From what I can tell, when you send messages to the command-sender during the actual command execution (so not as part of a scheduled task), those sent messages are considered a command response, which is why, if you use Kira and send the command /whoami, you'll get your name back, but if you did /lovetap (the BanStick command), you'd probably get less information than you think (assuming you have permissions) because some of the information is only sent later within a scheduled task, which is done after the command execution has technically concluded. This may have implications with bots etc. The current command schedules the collection of the snitches within an asynchronous task to offset the load of collecting what could very well be a large amount of data.