Open WyndStryke opened 3 years ago
If you could fork the repo and commit your changes, then link the appropriate branch from here, that would be helpful.
Thanks!
I hope I've done this right, I forked it, downloaded GitHub Desktop, cloned it, merged my local changes, committed it to the fork, and then wrote the changes into the fork's master branch:
https://github.com/WyndStryke/arkadius-trade-tools
So hopefully you should be able to compare the fork with the main project and see what changes I made.
Wishlist items
Thanks for taking care of that. 👍
I haven't dug into things deeply yet, but it looks like you've covered all of the appropriate bases.
Your wish list covers the items I was going to bring up. Especially important, IMO, is the max value of the slider. Making that dynamic should be relatively simple, but we don't want to present options to the user which will just frustrate them.
A couple of notes:
I have been experimenting with a local version of the code which can store up to 3 months worth of data (default still 30 days). This adds this/last/prior month selectors. If this is of interest the source code is attached.
Not sure if you would want this functionality in the public version or not, since the main focus of ATT seems to be performance rather than adding loads of different features, and extending the stored data would have an obvious performance hit (simply adding the option should not have a negative effect, however).
It is a modified version of the official v1.11.2 to add support for up to 92 days (=3 months) worth of stored data instead of 30, and added new selectors for 'this month', 'last month', 'prior month', and 'all time'. This functionality is intended for guild leaders rather than normal guild members.
However, I would recommend that, for performance reasons, the extended period should only be used for a single guild (i.e., the GMs) rather than extending the data retention period for all guilds you are a member of.
Also note that the excellent new exports module may supply what GMs need, without the requirement to store more than 30 days of transaction data.
Caveats - Do not trust the new translations (I am only monolingual). Do not trust the new code (I am new to LUA). Similarly I am new to GitHub and don't know how to do branches / pull requests / etc. I have tested with about 60 days of data, but not across a year boundary or other edge conditions, and only on PC/NA with keyboard&mouse. I presume that os.time() function gives the calendar date on the client machine rather than the server, so 'this month' etc would refer to the local start time and end time..
I have attached the modified v1.11.2 code as a .zip
ArkadiusTradeTools_92_days.zip
Labels - enhancement, user request