multitheftauto / mtasa-resources

This project maintains a list of up-to-date resources that come with Multi Theft Auto.
https://multitheftauto.com
MIT License
151 stars 149 forks source link

admin2: refactor ban functionality (resolves #151) #437

Closed jlillis closed 7 months ago

jlillis commented 1 year ago

To satisfy #151

This PR redesigns the add ban widget. It can be accessed by selecting a player and clicking "Ban" in the main tab, or clicking "Add ban" from the player tab. When accessed from the main tab it will autofill the target player's IP and serial. You can chose whether to add either the IP or serial (or both) to the ban.

image

Some remaining tasks:

While there are some remaining tasks I consider this ready for testing.

I haven't been able to test this with multiple players so I don't know if everything is working properly, especially the sync of added/deleted bans in the bans tab.

Dark-Dragon commented 1 year ago

Here are a couple of things I ran into while testing (all except the very last are manual bans from the ban tab, not the players tab):

jlillis commented 1 year ago
* I keep being told my serial is invalid when trying to ban my own serial

Fixed.

* After receiving an error for not providing an IP/Serial or an invalid one the the confirmation message box still popped up

Fixed.

* Sometimes I had to reselect a ban duration when submitting multiple bans in a row (can't quite tell how to reproduce just yet)

Added a line to reset the ban duration to default when the widget is shown, which should fix this.

* Confirmation message box pops up despite not checking IP or Serial

Fixed.

* Maybe the confirmation message box type should be question rather than warning?

I think it's more of a question rather than a warning but if anyone else feels otherwise it's easy to change.

* After confirming a ban there's a lack of feedback. I'm not sure if the gridlist is supposed to update immediately, but if it is, then it didn't appear to work during my testing. I believe normally there should be a chat output too, shouldn't there?

Partially fixed. I had to dig around the serverside code some more to figure out how the ban list sync was working. New bans should appear in the ban list as soon as they are added. It's not outputting to server log/chatbox currently, added that to my todo list.

* The 1 second ban I presume is just used for testing

Correct.

* Maybe the confirmation could say the ban duration again?

There isn't a ton of room in those messagebox widgets, but I'll play with them and see what works.

* Generally the widgets in admin2 appear to leave less empty padding space around the sides than this one, might be worth considering to adjust for consistency?

Just reduced the horizontal padding by 50%.

* Hide sensitive player data should probably mask the IP/Serial if checked when banning through players tab.

Added to todo list.

jlillis commented 10 months ago

I've finally got around to crossing off the rest of my todo list and am calling this done and ready for more testing. I "fixed" the unimplemented ban/unban commands by just removing them - if there are enough complaints I'll go ahead and get them added back in.