Solybum / PSOBBMod-Addons

GNU General Public License v3.0
29 stars 29 forks source link

Basic usability update #8

Closed GilPalma closed 7 years ago

GilPalma commented 7 years ago

The point of this update was to make my life easier. I added a few things that I wanted and in the process changed a couple of things that seemed off in the code, namely how variables were being declared globally and interfering with each other. This resulted in problems when having configuration files for each addon, Lua would only find the first one.

GilPalma commented 7 years ago

Screenshot of the new Character Reader (without any other component enabled) solybum_28-04-2017

Solybum commented 7 years ago

There are some nice changes here. I'll see if I can address everything

Now regarding the PR itself. You added a change that reverts this fix, maybe you forgot to pull the latest changes before doing the PR. Also, I kinda don't like how you put everything in a single PR, the new config files for the monsters and player reader (which is not meant to be used by people yet), they could be in a separate PR.

So, I guess you could make 3 separate PRs

GilPalma commented 7 years ago

1 - From my testing I noticed no problems with having all local variables. There may be some that make sense to switch back to global but so far I thought this would be best. 2 - Some options are really just fluff but things I'd like as a user. 3 - Regardless of which features will be implemented, right now having a dropbox with 3 choices was just doubling the amount of clicks. In terms of usability, this just seemed like the best choice. However, if this clashes with your plan, I won't object. I will pay attention to future changes and see if there's anything I can add. 4 - The other config files weren't really needed but for facilitating future updates, I feel like this was an important step. If more features are added (like filters, sort methods, etc) these config files may just come in handy. 5 - As for the pull request, you seemed to have merged anyway. This time I only really set my mind to push to github after making all the changes on the addon's files so everything was clumped together. In the future, if I ever change anything else, I'll make sure to have cleaner PRs. 6 - Not sure how I reverted that fix. Have to pay closer attention next time. Going to have to check that out. Sorry about that.