franciscojunior / Vampidroid

Vampidroid is a mobile management solution for your Vampire: The Eternal Struggle™ card game cards. With it you will be able to search card text, manage your deck and many other things.
GNU General Public License v2.0
1 stars 1 forks source link

Updated database to the latest Sabbat sets. Changed the ids to match … #65

Closed GiottoVerducci closed 5 years ago

GiottoVerducci commented 5 years ago

…the universal ids of the cards and removed the autogenerated ids.

franciscojunior commented 5 years ago

Great work, @GiottoVerducci ! Thank you very much! Database import is a very important task when new cards arrive and sometimes, because of the utf-8 handling and double quotes, can be very tricky too. I remember I had to make some imports a lot of times to get it right. :)

Got the update up and running! I had to make some changes though:

  1. Add some lines to DatabaseHelper.java:
// This is the migration to update card listings to Sabbat Sets expansion
static Migration MIGRATION_7_8 = new ReplaceCryptLibraryCardsMigration(7, 8);

and

.addMigrations(MIGRATION_7_8)

Those changes are needed because Room library checks when the database version has changed and requires a migration procedure. In this case, the procedure is just to replace the cards. In the future we may have some other more complex changes.

  1. Add some null checks in the entity constructors to get empty strings: this.name = name == null ? "" : name; this.text = text == null ? "" : text;

and so on.

This was caused by a little difference in the imported database: where there are null values now, I used to have empty strings. But with this little change, everything goes back to normal. I also can change the code base to take in consideration that there could be null values instead of empty strings.

Would you mind add the migration lines to your pull request so I can merge it? If you can't do that right now, I can merge your work and add the lines separately.

About the entity null values, we can change the migration database to contain empty strings instead of null values or we can change the code to take those null values in consideration instead of empty strings.

About the database import, would you mind to add the procedure you used to create the database? There are some import scripts in the project (on sql folder) which may need some adjustments.

Thank you again for your support and help!

GiottoVerducci commented 5 years ago

1/ I've made the change for the migration on the Pull Request. I left migration from 6 to 7, I suppose it's meant to be incremental.

2/ The easiest way to handle values as before would be to change the import SQL query to replace null by empty strings:

database.execSQL("insert into CryptCard(uid, Name, Type, Clan, Advanced, Group, Capacity, Disciplines, Text, SetRarity, Artist) select Id, Name, Type, coalesce(Clan,''), coalesce(Adv,''), Group, Capacity, Disciplines, CardText, Set, Artist from updatedb.crypt"); database.execSQL("insert into LibraryCard(uid, Name, Type, Clan, Disciplines, PoolCost, BloodCost, ConvictionCost, Text, SetRarity, Artist) select Id, Name, Type, coalesce(Clan,''), coalesce(Discipline,''), coalesce(PoolCost,''), coalesce(BloodCost,''), coalesce(ConvictionCost,''), CardText, Set, Artist from updatedb.library");

I did that change in my Pull Request.

3/ As for the database import procedure, I didn't use any SQL script. I did it simply using DB Browser for SQL Lite https://sqlitebrowser.org/dl/

I added the how-to the README.md file

4/ I've updated the README.md to mention the Dark Pact. It's the way WW allows users to use their IP. You should add the Dark Pact logo in the app (eg., about menu), or when the app is launched on a splash screen.

Le mar. 5 mars 2019 à 21:03, Francisco Figueiredo Jr. < notifications@github.com> a écrit :

Great work, @GiottoVerducci https://github.com/GiottoVerducci ! Thank you very much! Database import is a very important task when new cards arrive and sometimes, because of the utf-8 handling and double quotes, can be very tricky too. I remember I had to make some imports a lot of times to get it right. :)

Got the update up and running! I had to make some changes though:

  1. Add some lines to DatabaseHelper.java:

// This is the migration to update card listings to Sabbat Sets expansion static Migration MIGRATION_7_8 = new ReplaceCryptLibraryCardsMigration(7, 8);

and

.addMigrations(MIGRATION_7_8)

Those changes are needed because Room library checks when the database version has changed and requires a migration procedure. In this case, the procedure is just to replace the cards. In the future we may have some other more complex changes.

  1. Add some null checks in the entity constructors to get empty strings: this.name = name == null ? "" : name; this.text = text == null ? "" : text;

and so on.

This was caused by a little difference in the imported database: where there are null values now, I used to have empty strings. But with this little change, everything goes back to normal. I also can change the code base to take in consideration that there could be null values instead of empty strings.

Would you mind add the migration lines to your pull request so I can merge it? If you can't do that right now, I can merge your work and add the lines separately.

About the entity null values, we can change the migration database to contain empty strings instead of null values or we can change the code to take those null values in consideration instead of empty strings.

About the database import, would you mind to add the procedure you used to create the database? There are some import scripts in the project (on sql folder) which may need some adjustments.

Thank you again for your support and help!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/franciscojunior/Vampidroid/pull/65#issuecomment-469837066, or mute the thread https://github.com/notifications/unsubscribe-auth/AENkuMwB1A_wUj1eMWIG94f0OzuUMHhPks5vTs2UgaJpZM4bceSy .

franciscojunior commented 5 years ago

Thank you very much for the changes, @GiottoVerducci ! I think it is almost ready for merge. I just made a small comment about the CCP Hf on the copyright change.