Skip to content
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

[V3] Data Converter #1293

Merged
merged 14 commits into from
Apr 3, 2018
Merged

[V3] Data Converter #1293

merged 14 commits into from
Apr 3, 2018

Conversation

mikeshardmind
Copy link
Contributor

Type

  • Bugfix
  • Enhancement
  • New feature

Description of the changes

Adds a data converter for cog creator use (and for core cogs too I guess)

@mikeshardmind
Copy link
Contributor Author

The underlying logic should be sound, but this needs a bit more testing before use.

@mikeshardmind
Copy link
Contributor Author

Additional note: This only handles global setting values currently, additional methods need to to be added for handling guild, member, user, channel, and role settings. (More to come on this PR)

@palmtree5
Copy link
Member

I have one question on this: how would the user specify the path to the data file? It's not like we have a consistent location for the actual location of Red in V2 which would mean that predefining the location of the data file for a cog is probably not a good idea

@mikeshardmind
Copy link
Contributor Author

I wasn't planning on this being entirely automated, it would still require a user defined path. (possibly with either a prompt or a file drop)

This just lays the groundwork for some mass config changes from a file. with minimal direct interaction with Config. Still a WIP.

Ideally, at some later point when more conversion tools are ready, you could point a V3 instance at an old V2 install location and have it import every setting which it knows how to (which would still require individual cog creators to specify a conversion spec, and the location their files were stored relative to the base path of V2)

If you think this doesn't belong in base red, I can package this as a coglib instead when the downloader and cog manager are ready to support that.

@palmtree5
Copy link
Member

Yeah, I was clarifying there would be user interaction at some point in the process. I think the basic framework of this belongs in core though

@mikeshardmind
Copy link
Contributor Author

I'm putting any further work on this one on hold until resolution of #1294 since I will need to work directly with some of config's internals for this.

@tekulvw
Copy link
Member

tekulvw commented Feb 22, 2018

#1335 should help significantly with this.

@mikeshardmind
Copy link
Contributor Author

get_raw and set_raw should make incorporating proper conversions for things at the correct scopes easier. I haven't forgotten about this PR, but I haven't worked on it since my prior comment either.

@mikeshardmind
Copy link
Contributor Author

This has been significantly modified to use new config methods, properly scope data, and if not properly scoping the data, put the data in a custom data group.

With the modifications made, this requires fully retesting which I have not subjected it to yet.
I do not have time to do this today, so this will not be ready for b9 unless someone else has the time to test this.

@mikeshardmind
Copy link
Contributor Author

mikeshardmind commented Mar 5, 2018

Do not merge this even if it looks ready at any point until I update otherwise. This can be merged whenever now.

@mikeshardmind
Copy link
Contributor Author

This has been tested now. The following conversions are tested and working:

  1. Bank Accounts
  2. Custom Commands
  3. Economy Settings
  4. Filter
  5. Past Names
  6. Past Nicknames

I'd appreciate a second set of eyes on this though.

Modlog's conversion is not done, but it also is not available through a menu. If I didn't miss any issues with this, this can be merged as-is and have the modlog conversion added as a later PR as unlike the ones here, the modlog conversion will be non-destructive (All of the above imports override existing values (but should retain values without a setting in v2 data))

This also is not specific to any backend as it works directly with config. rather than with the drivers.

@tekulvw tekulvw merged commit d65f885 into Cog-Creators:V3/develop Apr 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants