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: finished team widget color picker implementation #435

Closed Dark-Dragon closed 1 year ago

Dark-Dragon commented 1 year ago

Currently there is some unfinished code in the admin_team widget which is supposed to implement a color picker for team creation. In its current state it's actually hindering the user from creating a team due to overlap with the old but functional gui elements. This pull request finishes the remaining work to get it fully working and removes the older, no longer needed gui elements for team creation.

Known issues:

jlillis commented 1 year ago

Looks good and I'm willing to merge as is, with just a two notes:

1 - Perhaps we should just close the colorpicker if the user clicks out of it so the color preview isn't rendered over other GUIs? This is how the freeroam colorpicker behaves. 2 - Since the setTeamColor function only takes rgb values I don't think we need to worry about saturation.

Dark-Dragon commented 1 year ago

The picking form already behaves that way, although, it does so on mouse state 'up' where as gui elements are brought to front on mouse state 'down', so it would still be an issue while the click is held.

It's the area inside the team widget you click to open the picking form that could end up being rendered on top of other gui looking ugly, but so far I couldn't come up with a better solution:

Idea: Only render it while its parent window is the top most Complication: I haven't found a gui function or property that seems like it lets you check. Additionally while the picker form is shown, it wouldn't even be the top most, so we'd need to get the actual Z order from CEGUI, which I don't think can be retrieved. There's a 'ZOrderChangeEnabled'-property which I suspect could be used to keep it on the top, but it's not exactly the greatest solution.

Idea: Color a gui element instead Complication: Only the text of grid list items or gui labels can be colored. We could color a square character like ⬛, but it doesn't look very good and the preview would be very small. Font size can't be adjusted, so the only way to make it larger would be to create a new larger gui font and that overall just doesn't seem all that great to me.

As for the color saturation/value: I've had a look at the freeroam color picker, and it comes with value and alpha sliders. For this case just a value slider would be enough, it seems. I'm not super well-versed in color theory. White/black darker and brighter colors are all well within what the user should ideally be able to pick. Overall still a minor complaint.

Dark-Dragon commented 1 year ago

While it's not the perfect solution, I've changed the code so the team widget is 'AlwaysOnTop' while the color picker preview is shown. This will prevent it from rendering on top of other gui elements. Because the preview is only visible while the user is creating a new team it shouldn't be much of a bother for anyone under normal circumstances.

If I or anyone else can come up with a better solution, we can implement it then.

jlillis commented 1 year ago

Found an issue: because the widget is set to AlwaysOnTop, message boxes appear behind it. This means that if I try to create a team without providing a name I end up with a warning I can't dismiss:

image

Dark-Dragon commented 1 year ago

I came up with a different work-around using onClientGUIFocus that should work better overall, however in order to prevent the issue with the message/input box pop-ups specifically I needed to add guiFocus() in their code respectively. Only down side seems to be that after dismissing the message box the color preview will only re-appear after clicking the team widget again. We can make it re-appear automatically if we were to use guiFocus() on it after, but then we could potentially run into the issue again where it could render in front of something unless we also use guiBringToFront(). It's easy to add, but I'm not sure yet if I like it personally. If you think it's a good idea I'll add it. So to be clear that would be one guiFocus(aTeam.Form) and potentially one guiBringToFront(aTeam.Form) after each message box within the aTeam.onClick function.

jlillis commented 1 year ago

Tested this again recently and it works well enough. Sorry for the delay.