javierriveracastro / betteroll-swade

A Better Rolls port for SWADE
GNU General Public License v3.0
15 stars 31 forks source link

Settings menu #689

Closed ddbrown30 closed 3 months ago

ddbrown30 commented 3 months ago

I added the settings menu. This comes along with a bit of refactoring to make things more consistent and reliable but I tried not to touch too much.

For the moment, I just split the options into two tabs, world and user. All the world settings are in the world tab and all the user settings are in the user tab. The intention here is to eventually break these down into more tabs for better organization but I'll leave that to you to decide on and implement.

The biggest change in functionality is the user settings. Anything that was a client setting is now a user setting. User settings are saved to the flags on the user. This ensures that the setting travels with the user to different devices and is not wiped by clearing the cache.

The menu probably needs some CSS love but I'll also leave that to you. It looks fine as is, but it could use some spacing and padding adjustments and it would be nice if the apply button was pinned to the bottom of the window.

I tested every single option by hand. I'll never claim that there are no bugs but I feel pretty confident that all the core functionality is working (or at least isn't any more broken than it was before).

javierriveracastro commented 3 months ago

This is more complicated, but there are some things that I don't like. Probably result of too many time using Python, but...

It also has some merge conflicts with your other branchs, so I will merge this from my IDE that is way easier than github, so I close this here (it is still merged).

ddbrown30 commented 3 months ago

This is more complicated, but there are some things that I don't like. Probably result of too many time using Python, but...

* I hate * imports. Sometimes you can need to edit the file in a text editor (not a fancy IDE) and you need to be able to know where things came from.

* No global scope variables (constants are OK). So that cascade count makes me nervous. I'll move it under game.swbr

It also has some merge conflicts with your other branchs, so I will merge this from my IDE that is way easier than github, so I close this here (it is still merged).

I can't speak to the imports thing. My JS knowledge is pretty newb level so I could definitely be doing stuff in a non-optimal way.

For the global variable, I 100% agree. I couldn't find a better place for it but I forgot about the game variable. That's a smart place to put it.