New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin XLRStats: fix 'xlrinit' command #388

Merged
merged 3 commits into from Jun 9, 2018

Conversation

Projects
None yet
3 participants
@jonesman
Contributor

jonesman commented Jun 5, 2018

Lets the 'xlrinit' command wipe only XLRStats tables (name starting with 'xlr_') as intended, and not touch the other B3 tables stored in the plugin's xxx_table variables.
I have tested it on a server just in case, it does continue counting at 0 after the reset.

Plugin XLRStats: fix 'xlrinit' command
Lets the 'xlrinit' command wipe only XLRStats tables (name starting with 'xlr_') as intended, and not touch the other B3 tables stored in the plugin's xxx_table variables
@82ndab-Bravo17

This comment has been minimized.

Contributor

82ndab-Bravo17 commented Jun 5, 2018

Good job, that works well as long as the default table name haven't been changed, but may cause tables to not be initialized if they have.

Better to leave out the 2 tables that we don't want to touch - clients and penalties?

@danielepantaleone

This comment has been minimized.

Member

danielepantaleone commented Jun 6, 2018

XLRstats table names are saved in the XLRstats plugin as attributes. The original code loops over all those attributes and truncate only such tables. Dis anyone tried to run automated tests to see if the problem shows up?

@82ndab-Bravo17

This comment has been minimized.

Contributor

82ndab-Bravo17 commented Jun 6, 2018

It picks up all the tables that xlrstats uses from this list, which includes the client and penalties tables:
# names for various stats tables
playerstats_table = 'xlr_playerstats'
weaponstats_table = 'xlr_weaponstats'
weaponusage_table = 'xlr_weaponusage'
bodyparts_table = 'xlr_bodyparts'
playerbody_table = 'xlr_playerbody'
opponents_table = 'xlr_opponents'
mapstats_table = 'xlr_mapstats'
playermaps_table = 'xlr_playermaps'
actionstats_table = 'xlr_actionstats'
playeractions_table = 'xlr_playeractions'
clients_table = 'clients'
penalties_table = 'penalties'
# default tablenames for the history subplugin
history_monthly_table = 'xlr_history_monthly'
history_weekly_table = 'xlr_history_weekly'
# default table name for the ctime subplugin
ctime_table = 'ctime'
# default tablenames for the Battlestats subplugin
# battlestats_table = 'xlr_battlestats'
# playerbattles_table = 'xlr_playerbattles'

@jonesman

This comment has been minimized.

Contributor

jonesman commented Jun 6, 2018

Hi, thanks for looking into this!

@82ndab-Bravo17 I think with something like this (wiping stuff from the database) it can't hurt to be explicit, better something does not get wiped by accident then if it does ?

@danielepantaleone I just looked at the 'test_xlrstats.py' and didn't find anything related to the 'xlrinit' command or to database access. I can confirm by looking at B3's log and a phpMyAdmin listing, that now only tables starting with 'xlr_' are truncated.

@82ndab-Bravo17

This comment has been minimized.

Contributor

82ndab-Bravo17 commented Jun 7, 2018

Normally I would agree with that, but since we can see from the code what gets wiped currently we can also see what we don't want to be wiped and allow those with custom table names to use xlrinit as well.

@danielepantaleone

This comment has been minimized.

Member

danielepantaleone commented Jun 7, 2018

Then I would just exclude those tables from the for loop in xlrinit 😊

Exclude tables from 'xlrinit' using variable
Now the table names that should be excluded from 'xlrinit' can be put into an array
@jonesman

This comment has been minimized.

Contributor

jonesman commented Jun 8, 2018

OK, i was unsure what the 'ctime' table was used for. So above is a commit that lets you exclude tables by putting their names into an array.
Please feel free to close this if you didn't like my wording or the placement of the variable.

@@ -1823,7 +1826,7 @@ def cmd_xlrinit(self, data, client, cmd=None):
xlr_tables = []
for attr in dir(self):
attr_val = getattr(self, attr)
if attr.endswith('_table') and attr_val.startswith('xlr_'):
if attr.endswith('_table') and attr_val not in self.exclude_from_xlrinit:

This comment has been minimized.

@danielepantaleone

danielepantaleone Jun 8, 2018

Member

Since it's being used only here I don't feel the need of storing the tables in an attribute (and use sets instead of lists 😉). I would just do:

if attr.endswith('_table') and attr_val not in {'clients', 'penalties'}:
Update __init__.py
Exclude tables 'clients' and 'penalties' in xlrinit
@jonesman

This comment has been minimized.

Contributor

jonesman commented Jun 8, 2018

OK, changed accordingly

@danielepantaleone danielepantaleone merged commit cd17a70 into BigBrotherBot:master Jun 9, 2018

@danielepantaleone

This comment has been minimized.

Member

danielepantaleone commented Jun 9, 2018

Thanks 👍

@jonesman

This comment has been minimized.

Contributor

jonesman commented Jun 9, 2018

Thanks everyone for taking the time! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment