sumpfork / dominiontabs

Divider generator for the card game Dominion and its expansions.
http://domdiv.sandflea.org
Other
123 stars 125 forks source link

How to Deal with Second Edition Cards #73

Closed nickv2002 closed 7 years ago

nickv2002 commented 8 years ago

How do you want to deal with the second edition which are already shipping to stores?

Some Options I thought of:

  1. New "Second Edition" sets for Dominion and Intrigue
  2. New "Second Edition" Group of the new cards
nickv2002 commented 8 years ago

Additionally, it sounds like we're getting new rules to be updated and new wording on a few cards (adding "may") that might actually change the rules for them slightly.

sumpfork commented 8 years ago

I think '2nd ed.' versions of Dominion and Intrigue will probably work ok. These can also have the wording changes. There'll be a good amount of data duplication, obviously - I guess we could get fancy and have sets inherit from others with overriding, adding and deleting, but I'm not sure that's worth the effort and would make it harder for people to contribute that aren't familiar with how those mechanisms work.

We already have an example of (sub-)set duplication between the 'Dominion' and 'Base' sets.

I'm not sure we'll be able to track individual slight card text/rule updates in new printings of other sets very well, and I suspect it probably doesn't matter that much - people do have the actual rules and cards in case there are questions.

nickv2002 commented 8 years ago

@stumpfork What about for the folks that just purchase the upgrade packs with the 7 new cards (14 total between the two 2nd edition sets)? Can we also put in an Upgrade set for those?

sumpfork commented 8 years ago

To just print the new dividers instead of printing a whole set again, you mean? Hmm, yeah, I guess that'd be nice - in fact, I might need that :).

Well, is it worth having some basic inheritance in that case? Like a set_definitions.json file that lists something like

[
...
{
    "set": "Dominion 2nd. Edition",
    "inherits_from": "Dominion",
    "cards_to_delete": ["cardname1", "cardname2"...]
},
...
]

with the default being that cards from parent sets are included if not deleted, and overridden if specified again in the actual cards.json file for the inheriting set?

nickv2002 commented 8 years ago

I'm fine with inheritance. But there's not a lot of crossover so I'm also fine with the dumb option of just having duplicated data for cards that are in multiple sets.

(I figure many people with existing prints of the dividers will be buying the upgrade packs with the 14 new cards, myself included.)

nickv2002 commented 8 years ago

Missed this last week, but here's all the info on the new cards as well as the removed ones for Dominion (Rulebook PDF) and Intrigue (Rulebook PDF).

Not sure if we have a good version of the second edition symbol up yet, but we can work on incorporating these into the generator.

nickv2002 commented 8 years ago

Here's an updated group of icons for every set including the new Second Editions.

wvoigt commented 8 years ago

I know this probably will not be popular, but all the card data duplication is starting to make it hard to keep everything up to date. Just about every bit of card data is duplicated in each language. And since there is no unique key/identifier on each card that spans languages, it is hard to match cards across the languages.

I know it would be a big change, but I think the following would make things easier to maintain in the long run:

This has several advantages: If there is any structural changes/fixes, it goes to all languages immediately. Lets say a new card is added, then all languages at least get the divider, even if it temporarily has the primary language. It will also make it more obvious that a translation is needed. As it is now, it is just not there. And until someone translates it, and adds its data, it does not even show up.

nickv2002 commented 8 years ago

@wvoigt I think you're generally right and your changes would make things easier to maintain. However, we have to balance all that with the work it would require to make the changes vs the cost of continuing as is. If we knew with certainty that there would be no more expansions, only new promos, would this be worth doing? What about 1 expansion? 2, 3?

So sure, it does make sense to refactor at some point but the calculus of when to do that and who will do the work is difficult.

sumpfork commented 8 years ago

I do agree in principle, and note that not only future sets but also translations would greatly benefit from the increased sharing and thus error checking and consistency that the English version has received. Plus, it would be easier to error check what is missing in translations.

I also think we're not that far from doing this. Card names are mostly already a unique identifier, with only revisions requiring added tagging. Sets are already unique. That said, I do agree that tags for both should be separate from printed name strings. Separating out the common info just requires making a common JSON file with only that info and the card tag, and leaving the card name/set name/description/extra in the language-specific files. One question is whats should happen to mapping.json. We still need a translation of set names to readable names, but right now that file also contains the promo card title translations. I think it should be just sets.json and contain a mapping from set tag to set name in the specific language - promo card names should be specified in the cards.json file and their tags should be used to specify images.

So, a common card_spec.json entry should be

 {
  "card_tag": <card_tag>,
  "cardset_tag": <cardset_tag>,
  "cost": "4"
  "potcost": 0,
  "types":[
   "Action"
  ]
 }

and cards.json should just have

 {
  "card_tag": <card_tag>,
  "description": "+1 Card\n+1 Action\nYou may play an Avanto from your hand.\nWhile this is in play, when you play a Silver, you may trash a card from your hand.",
  "extra": "Sauna is a promotional Action card. It's a cantrip that allows you to trash a card when you play a Silver. It is a split pile card, with five copies of Sauna sitting on top of five copies of Avanto.",
  "name": "Sauna"
 }

in the appropriate language.

nickv2002 commented 8 years ago

@sumpfork Would your proposed card_spec.json and cards.json files have any special logic or features to handle split-piles etc that have different cards under one divider? Or would it just work the same way it does now with a separate split and combined entries?

ghostsquad commented 8 years ago

👍

sumpfork commented 8 years ago

I hadn't really thought about reworking split piles in this context, but we certainly can consider it if there's a good proposal.

nickv2002 commented 8 years ago

@mr-ice was kind enough to type up the 2nd edition cards: https://github.com/sumpfork/dominiontabs/pull/83/commits Still need to add the 2nd edition Intrigue Cards too.

nickv2002 commented 8 years ago

As for split piles: let's just put every individual card in cards.json just like you listed including both cards from a split pile. Then, for split pile cards we can have a special tag in the card_spec.json like so

{
  "card_tag": <card_tag>,
  "cardset_tag": <cardset_tag>,
  "splitpile_tag": <splitpile_tag>,
  "cost": "4"
  "potcost": 0,
  "types":[
   "Action"
  ]
 }

Any cards that went together in one pile would get the same splitpile_tag, then, when generating, if the user selected the combined option, the generator would look for anything with the same splitpile_tag and combine it into one entry programmatically by selecting the lowest cost and union-ing the types. This way we can get rid of extraneous entries for the combined piles. It should work for stuff like the Cornucopia Prizes too. Does this sound reasonable?

mr-ice commented 8 years ago

Thanks for the mention -- I wasn't following this thread until @nickv2002 and @sumpfork pointed it out.

I don't yet have my Intrigue update pack. I got the base update and started typing to get dividers printed.

As to splits, I certainly think the splitpile_tag could work. I've got a concern that I noted going from dividers to wrappers, however. Some of the wrappers were printing 1 card width (or zero, I can't be sure) (for example I noticed this on 'Amulet'). This was fixed by adding an explicit count to the json data, but I haven't determined why it was not correct.

The mechanism behind splitpile_tag should account for accumulating the number of cards so that wrappers style divider depths will be calculated properly.

nickv2002 commented 8 years ago

FWIW: @mr-ice links in https://github.com/sumpfork/dominiontabs/issues/73#issuecomment-251478070 show the 2nd Edition Intrigue new/removed cards.

mr-ice commented 8 years ago

Thanks, that actually helps a lot, copy paste is better than typing.

I ran into something curious while putting these in -- newlines in description get honored, but in extra they disappear. I'm able to put in <br> but not sure if that's the right thing to do.

Anyway, I think I've amended the pull request so you can see them all, but it's messy because there was trailing whitespace elsewhere that I removed when I removed the bit added by json.dump().

wvoigt commented 8 years ago

@mr-ice As for "Amulet", it was an error. Issue #81 already addresses it. It is covered under pull request #82. If you find any others please let me know. I printed out my vertical wrappers a couple of weeks ago, found the errors, and make the updates.

@nickv2002 Splits today are covered in the card_groups.json file. As a result, when grouping for wrappers, the total card count will be the sum of all of the cards grouped together. I think it makes sense to keep this separate from the cards.json. Individual card information in one place, groupings in one place.

What I would like to see different in card_groups.json is a unique identifier ("card_tag") for the group, and then change the "new_name" to be "name" and to then also include a "description" and "extra". That way the grouping (splits in this case) can have their own text for the whole group. As it is today, it uses the text from the first card. Also, if we ever simplify the translations, these 3 fields could easily be included in the translation files (referenced by the "card_tag")

And in the spirit of keeping like things together, I propose we should have a sets.json. So this would have the unique identifier for the set, its name, it's image file, etc, and then a listing for each card ("card_tag") associated with the set. This would mean the cards.json would no longer need the sets identifier, and we would only need one copy of gold, silver, and any other cards doubled up. We could even have 2 different versions of cards (with different identifiers). In this way, 2nd edition sets are just separate sets, that point to many of the same cards and a few different cards. We correct one card, and it is corrected in all sets it appears in. So the structure is:

Once the json is chagned, then the code changes are mostly in read_write_card_data():

  1. load cards.json (as a starting database, not as actual cards as we do today)
  2. go through the sets that are requested, adding the cards for each set from the cards.json
  3. process all the card groupings
  4. process the set.json, groups.json, and cards.json for the language requested. (or maybe we combine these into one file, since everything has a unique identifier now.)

At the end, we have the listing of cards we pass on to the rest of the program. From there on it is the same as it is today.

nickv2002 commented 8 years ago

@wvoigt I mostly agree with what you're saying but couldn't we just allow cardset_tag to be a list similar to the types tag and then eliminate the need for a second set.json file? So if Gold was part of several sets we could just list them there, or we could create 2 version of a different card (i.e. one that got a rules wording change for 2nd edition) and have them belong to different sets as long as the card_tag was different.

wvoigt commented 8 years ago

@nickv2002 There would still be some set information in other places (setImages, setTextIcons, etc). But I agree changing the cardset_tag to be a list would also work.

nickv2002 commented 8 years ago

@sumpfork if you're okay with the changes that @wvoigt and I suggest, I can handle creating the cards.json file this week sometime (I'm pretty good with scripting text manipulation etc).

@wvoigt if you can provide me an example of how you want groups.json to format I can make that too (I have a general idea of how it would work but want to be sure I don't miss anything).

You both are more familiar with the code so if you could take charge of the changes described here that would be great.

sumpfork commented 8 years ago

On a cursory review I think I'm good with all of this - it's certainly moving in the right direction and we can revise later as we start playing with it.

nickv2002 commented 8 years ago

I created a little Gist showing a draft version of how I think the new files should look.

Will this revised format of information be workable with the main code? What other information should be added to the JSON output?

wvoigt commented 8 years ago

@nickv2002 This looks good. I like it. Just a few comments:

@wvoigt if you can provide me an example of how you want groups.json to format I can make that too (I have a general idea of how it would work but want to be sure I don't miss anything).

I would say for the common DB, keep it similar to today's card_groups.json, but just change card names to be their card_tag. Then "new_name" becomes "card_tag". Then the question is do we have a groups.json for the language that has "name", "description", "extra", or do we just add them to the new cards_en_us.json. My vote would be to just add them to the cards_en_us.json. So all card/groups are in the same language file and are in the same format.

We will need a small sets.json for the set language information. Example:

"intrigue2ndEdition": {
        "set_name": "Intrigue 2nd Edition",
        "text_icon": "I"
}
nickv2002 commented 8 years ago

@wvoigt Thanks! 👍

  • I thought I read somewhere that Intrigue 2nd Edition was not going to have the base cards. I may be wrong. But it is worth double checking.

Good call, you are correct. I will fix this.

  • Can we have sets for 1st Edition + the new 2nd edition cards? Normally not important, but if folks use wrappers, then the count would be adjusted for the randomizer cards.

Yes, that's a good idea. Actually we need a set that's just the new 2ndEd stuff which you can print with the 1stEd set.

  • I know this is personal preference and data is data, so I'm not pushing hard on this one. But in cards_en_us.json could the order be "name", "description", "extra".

I agree and tried to print it that way. However printing the dictionary of translation data proved to be tricky because the line-breaks and characters outside of UTF-8 (I spent an hour trying various thing) so instead I just threw it into json.dump(en_Translation_cards, open(outTransFile, 'wb')). and let that handle everything but lost control of the print order. If it's really important, I can see about fixing it in the final version, maybe just with a sed swap or something like that. Do you know if I can just put line breaks into the JSON file instead of \n? That would make it a bit easier.

I'll also work on the sets and groups JSON info.

nickv2002 commented 8 years ago

Check out my updated Gist for files revised based on @wvoigt suggestions above.

Questions for @wvoigt ?

  1. Doesn't the splitpile_tag fulfill the common DB you were talking about above? It points to Traveler, Event, Landmark, Split-Pile, Hermit Madmen groups... (though the first item of the Traveler Group might need tweaking).
  2. I wasn't sure what the text_icon field was specifically for, so I just auto-populated it with the first letter of a set's name.
wvoigt commented 8 years ago

@nickv2002 The splitpile_tag will make it a little harder to do, and is not quite as easy to "see" what is in each grouping. But I can see the value of having everything card related in one file. And what's a little more code, eh?

But can we call it group_tag instead? To me these are cards that are grouped together. I don't think splitpile describes it well. And not all of these are categorized as Dominion splitpiles.

A few other comments:

The text_icon is the text version of the set image. This is the current English version:

    'dominion': 'D',
    'intrigue': 'I',
    'seaside': 'S',
    'prosperity': 'P',
    'alchemy': 'A',
    'cornucopia': 'C',
    'cornucopia extras': 'C',
    'hinterlands': 'H',
    'dark ages': 'DA',
    'dark ages extras': 'DA',
    'guilds': 'G',
    'adventures': 'Ad',
    'adventures extras': 'Ad',
    'empires': 'E',
    'empires extras': 'E'

Also, while we are adding in everything to cards_db.json, how about adding an "image" : "image_file_name", for each of the promo cards:

    'walled village' = 'walled_village_set.png',
    'stash' = 'stash_set.png',
    'governor' = 'governor_set.png',
    'black market' = 'black_market_set.png',
    'envoy' = 'envoy_set.png',
    'prince' = 'prince_set.png',
    'summon' = 'summon_set.png'

for for example:

{
    "card_tag": "Walled Village",
    "cardset_tags": [
        "promo",
    ],
    "cost": "4",
    "image": "walled_village_set.png",
    "types": [
        "Action",
    ],
}

Nice work! This is really starting to take shape.

nickv2002 commented 8 years ago

@wvoigt Thanks for the feedback. See updates on the Gist.

I fixed up the text_icon tag to match what you listed above.

I added image tags to the relevant promo cards. If there's no image tag the code should just use the default one for the set it's printed with, which might vary depending on if the card is grouped with a 1st or 2nd edition printout. Also, FWIW, the flag promo image from Summon will be the default promo image from now on (including being used for Sauna/Avanto) so I just renamed that image to promo_set.png.

I also changed to group_tag as you suggested. Good change.

  • card_tag and splitpile_tag/group_tag need to be unique. I see "Page" used in both. The group tag should probably be "Page -> Champion". Similarly the "Peasant" group should be "Peasant -> Teacher". I didn't check for others, but we do need to make sure each tag is unique. All translations are tied to that tag.

Fixed to Page -> Champion and Peasant -> Teacher

  • I think it would be best to not use slashes / in the tags. I would substitute a dash. I guess one question is if we should have any other "rules" for tags to follow. For example, case, other characters to avoid, etc.

Fixed to be -. No specific opinion on rules, though I generally like long/descriptive camelCase strings (with no spaces or underscores) for variable names.

  • the cards_en_us.json file needs entries for each of the splitpile_tag/group_tag entries.

Not sure what specifically you meant here? They should all be there now. Is there any you see missing?

You're correct that not having a separate groups file will be a bit more coding, but it shouldn't be too bad, just iterate through the entire Cards_DB after reading it in and build a reversed dictionary keyed by each set or group. I can write that code loop if it helps. Seems like we've essentially building a Many-to-Many relationship between sets and cards.

Thanks!

wvoigt commented 8 years ago

@nickv2002 Good work. Yes, that was my thoughts on the image tag. Use the set's image unless a card has a specified one.

_the cards_en_us.json file needs entries for each of the splitpile_tag/grouptag entries. Not sure what specifically you meant here? They should all be there now. Is there any you see missing?

For example, there should now be entries in cards_en_us.json for:

Oh, just found another one. Tournament is used as a card_tag and group_tag. The group_tag should probably be something like Tournament and Prizes if it includes the "Tournament" card, or just Prizes if not.

I'm fine with using a single Cards_DB file. As you said, the code is not too bad. I haven't had time to actually code everything up to test. But I've worked out all the pieces on paper. I just need some keyboard time. Hopefully this weekend.

nickv2002 commented 8 years ago

@wvoigt Thanks for all the feedback. I had a busy day but I'll fix the Tournament to group with the Prizes (Tournaments and Prizes) this weekend. 👍

wvoigt commented 8 years ago

@nickv2002 I just noticed that debtcost is not making it over into the new DB on the cards that cost debt.

nickv2002 commented 8 years ago

@wvoigt thanks for the corrections. I just uploaded a new version with a group_tag for Tournament and Prizes and the debtcost tag.

sumpfork commented 7 years ago

Starting to look great. Do you want me to merge (or do you want to merge) #83 as a deployable though inferior interim solution for those with the new base cards, or rather wait for the reworked proper solution? Sorry I'm contributing little - 6 months old at home, formed my own company, that kind of thing...

nickv2002 commented 7 years ago

@wvoigt how's the integration coming? Do you need help with this this week? Trying to establish an ETA to answer @sumpfork's question…

I suppose there's no harm in merging it #83 now (so I just did that), it's more a question of whether we deploy it to the website for the public.

wvoigt commented 7 years ago

@nickv2002 Sorry, worked on it this weekend, but forgot to post. I'm finished with the initial implementation. Not saying it is perfect (I'm sure there are bugs), but I think it is far enough along for others to tryout. Here is a link on Gist to the files.

How should we go about combining all of this? Do you want to create a branch that we then both work on? Then when we are done, the branch gets submitted?

We also need to start thinking about how we create the text translation files for the other currently supported languages (de, fr, it).

Implementation Notes:

The cards_db.json is currently not a valid JSON file. A handy tool is at http://jsonlint.com. The json load errored on the extra commas at the end of lists. Similarly the sets text file has a trailing comma for the last entry.

If --special_card_groups is used and the resulting group_tag does not have any language translation (even in 'en_us'), it will print an error on the card similar to: _ERROR: Missing language entry for grouptab 'Page -> Champion'. This should really only be for us, to help find entries that need to be made in the 'en_us' translation file. After that, then the English text will be there to remind others about their own translation. This allows for a simple search for "ERROR" in the PDF file.

I added an option called --edition that can be "1", "2", "latest", or "all". Defaults to "all". This allows for a quick filtering for those that don't want everything (i.e., "all").

I created a sets_db.json to hold additional set information including the "image" file, default "set_name" and "text_icon" values, and the "edition" information for the above option. The default "set_name" is "*<cardset_tag>*". Again to help point out if we are missing something in the default translation file. This had the side effect of removing a lot of data from cards.py.

I assumed the following file structure (I'm open to suggested changes):

card_db/
    cards_db.json
    sets_db.json
    <language>/
        cards_text.json
        sets_text.json

This had the advantage of not reusing any existing file names. And I think they are descriptive.

I have not changed the dray.py file yet. I was limiting my updates for now. But if the Mixed case for the page footer bothers anyone, the change is on line 797: Change: setTitle = c.cardset.title() to: setTitle = c.cardset

Question With Events, Landmarks, and Prizes now included in --special_card_groups, I've been trying to figure out what should become of --exclude_events, --exclude_landmarks, and --exclude_prizes. One option is to just remove the options. Another would be to change their meaning. For example, --exclude_events could be changed to group all the events across all expansions. Similarly for Landmarks (if they show up in other expansions). The question is: where to put these? They span the whole set of expansions now. Maybe another "Extras" section at the very back?

The --exclude_prizes could morph into an option that groups the prizes together, but not combine with "Tournament". Or, we go the opposite, we change the grouping to be "Prizes" instead of "Tournament and Prizes", and then --exclude_prizes groups them with "Tournament".

In any case, that part of the code is questionable at this point.

Let me know if you find anything.

nickv2002 commented 7 years ago

@wvoigt Thanks! all that sounds great. I'd say to go ahead and make a new branch.

The cards_db.json is currently not a valid JSON file.

Yeah, I was just manually removing those training commas rather than write some smart code to skip the last ones and forgot to do it the last time I updated. Updated to remove the commas now.

We also need to start thinking about how we create the text translation files for the other currently supported languages (de, fr, it).

I can adapt my scripts to create language files based on the translations we've currently got, but let's get the English version working first.

I like your --edition filter. Great idea.

For --exclude_* let's just focus on 2 basic modes: all the cards (nothing combined and grouped) as the default, and a max grouping mode where everything that is combine-able (prizes, travelers, events, landmarks, split piles, etc) is put together, even if it's across sets. I think that should satisfy most people (and if you want separate Events for each set they're in, just create 2 PDFs, one for each set).

For, all your other questions I think your instincts are fine.

mr-ice commented 7 years ago

A handy tool is at http://jsonlint.com.

I can also recommend the node json tool at https://www.npmjs.com/package/json

alternately, use the json only through the python json module, so you're just dealing with data structures in one language.

wvoigt commented 7 years ago

alternately, use the json only through the python json module, so you're just dealing with data structures in one language.

@mr-ice , I don't understand the comment. What do you mean by one language? The data is loaded through the json module.

nickv2002 commented 7 years ago

@mr-ice I get what you're saying, but the library wasn't giving me control on the order that the arguments were written out for each dictionary item so I just wrote plain text.

nickv2002 commented 7 years ago

Sorry, hit Close & Comment instead of just Comment

wvoigt commented 7 years ago

@nickv2002,

For --exclude_* let's just focus on 2 basic modes: all the cards (nothing combined and grouped) as the default, and a max grouping mode where everything that is combine-able (prizes, travelers, events, landmarks, split piles, etc) is put together, even if it's across sets. I think that should satisfy most people (and if you want separate Events for each set they're in, just create 2 PDFs, one for each set).

I think I'm partial to 3 groupings:

To do the Max grouping there would need to be some dummy expansion set to hold the grouped cards. Not sure what it should be called or what the set image should be.

wvoigt commented 7 years ago

@nickv2002 ,

I'd say to go ahead and make a new branch.

I guess I was looking for some guidance. Do I make the branch under sumpfork/dominiontabs (can I even do that?), under you, or under me? Does anyone of them have an advantage for us in making our individual changes?

I can adapt my scripts to create language files based on the translations we've currently got, but let's get the English version working first.

I agree on the English version. I think we are very close there. Mostly missing group entries. What I fear is that the mapping may be tricky unless you know the language. There will need to be a mapping for each English "card_tag" to each card "name" for the language. Just worried there will be a lot of mapping by hand since the way the files were structured, everything including the "name" was in the target language.

mr-ice commented 7 years ago

What do you mean by one language?

Meaning you can use the json python library to read and write your json then you manipulate the data structure in python -- you can do this interactively or through scriptlets. Never writing anything in json by hand. The drawback seems to be what @nickv2002 mentioned:

library wasn't giving me control on the order that the arguments were written out for each dictionary item so I just wrote plain text.

which is a perfectly fair comment, the library only does "hash" order and "sorted" order.

It seems like sorted order was the way I found it when I contributed my tiny bit. Is it reasonable to pick one of those (maybe 'sorted' since it's at least predictable)?

example: Gist

mr-ice commented 7 years ago

Alternately, you can use an OrderedDict like this

from collections import OrderedDict
data = json.load(file('cards.json'), object_pairs_hook=OrderedDict)

Which will preserve the order you find. nothing different required on json.dump(). New keys have to be appended.

New Gist

nickv2002 commented 7 years ago

@mr-ice yeah an OrderedDict is probably correct way to do it, but I just dug in and wrote the files myself for expediency since it's a 1-time transition. Thanks for the tip though.

nickv2002 commented 7 years ago

@wvoigt We're both Collaborators now so we should have all the privileges to create/merge branches. I don't know if there a most-agreed-upon-correct way to do branches and merges, so I think it makes sense for you to create a new branch here with your work and I can help contribute to that. Once we're happy we can merge back to the master branch.

As for the non-English language stuff, I just looked at the files and it's not hard to figure out what text is what for most of the cards based on description/cost/set. Maybe I can write a script to create the JSON file with all the possible English names for each entry so it's easy to update by just deleting the wrong text. Then I can go through and make a quick first pass on the obvious stuff. Finally we can solicit help from some folks on BGG for what remains. It's a good point to update everything too, now that we've got the new system, since many of the translations are out of date.

And if you want to go with the 3 grouping strategy that's fine too. Was just trying to simplify since you could get the middle option effectively by creating multiple PDFs. Events are the only cross Expansion group now (existing in Adventures, Empire, and a Promo).

wvoigt commented 7 years ago

@nickv2002 I created a branch called "Card-DB-refactoring" to hold the changes. Please let me know if I should have done something differently. At the moment, all test fail since all the parts are not there yet.

So I'm looking at the new generated dividers, and in particular the expansion dividers. And while we are focused on "1st Edition" vs "2nd Edition" currently, ultimately when I print my dividers I don't think I really want to see "Dominion 1st Edition" and "Intrigue 2nd Edition". I think I would rather see just "Dominion" and "Intrigue". I know what version they are. Now I know others would probably want to see the edition information. So the question is how to provide both.

I propose that the sets_db.json file be changed slightly to include an optional"short_name" field that contains a shorter name. For example

    "dominion1stEdition": {
        "image": "base_set.png",
        "set_name": "*dominion1stEdition*",
        "short_name": "Dominion",
        "text_icon": "",
        "edition": [
            "1"
        ]
    },

Then with an option we can either assign just the set_name as before, or the short_name if it exists. I'm open as to which would be the default. "short_name" could also be included in the sets_text.json file for each expansion that might need a shorter name in a different language.

    "dominion1stEdition": {
        "set_name": "Dominion 1st Edition",
        "short_name": "Dominion",
        "text_icon": "D1"
    },

And again, this is only for the text on the expansion divider. The set name on the footer would still show the longer name.

In other updates:

I added an option called --upgrade_with_expansion which will put the upgraded cards into the corresponding earlier expansion. So all 1st edition cards as well as the upgrade cards appear in the 1st edition set. That way the new cards are listed on the expansion dividers and any tab/ordering fit the expansion as a whole. And because cards were deleted in the 2nd editions, this combined set of cards is different from the 2nd edition.

I fixed --exclude_events to be the max version that we discussed. The resulting group was put into an "Extras" set. I did the same for --exclude_landmarks, but I know this really doesn't cross expansions at this time. It was more for keeping the existing option. At the moment --exclude_prizes does not do anything.

nickv2002 commented 7 years ago

@wvoigt Great work. I think you did the branch right, I just created and merged a pull request on that branch to add updated set icons & the new format JSON DB files. https://github.com/sumpfork/dominiontabs/pull/87

New icons are from this BGG file. Notably, the first and second editions of both Dominion and Intrigue have slightly different icons, so I remapped them using the new sets_db.json.

I think defaulting to the short_name field is fine. I don't use the Expansion Dividers, so I don't even see set name on those once they're cut out. I added short_name to the sets_en_us.json file in another pull request.

--upgrade_with_expansion is a good option. Just make sure the new cards still get the newer set icon that's linked from their set in sets_db.json

Questions:

  1. In sets_db.json do you actually want to fill in the "text_icon": "", line? They're all blank now so why have them?
  2. In sets_db.json why all the * on both sides of the set_name tags? (Except on Promos: "set_name": "*promo",) Are they supposed to be everywhere? Again, why have them if they're everywhere?
sumpfork commented 7 years ago

Great work folks, let me know when you're close enough for me to start testing and reviewing.