Skip to content

Conversation

@mbbush
Copy link
Collaborator

@mbbush mbbush commented May 21, 2025

This is not ready to merge yet, but I wanted to show what I'm working on and get some feedback.

There's a lot going on here.

  • I originally hacked together a script that would read the card text from the dominion strategy wiki list of cards, and convert it into the format expected by this project (e.g changing {{Cost|3}} to 3 Coins).
  • After evaluating its results when rendering all the cards, I opted to instead keep the wiki formatting, and write new code for handling the inline images. The main reasons for this were:
    • The wiki formatting doesn't cause extra replacements to happen unintentionally. For example, the card text of Alchemist says "At the start of Clean-up this turn, if you have a Potion in play, you may put this onto your deck.". Currently when rendering that, this project substitutes the Potion symbol, even though it's not referring to the Potion resource, but the card named Potion.
    • The wiki formatting has more options for indicating size
    • The wiki formatting allows better control over line breaks, although actually making that work with reportlab is a challenge.
  • I wanted to keep the existing text for now, so I could render both side by side (with --front rules --back wiki-text) to compare them.
  • I added an option to render the actual card images (which I scraped from the wiki), so I could compare the way this code rendered the text to the way the actual cards do. It was very close; this is what led me to adjust the horizontal margin and font size to get the wrapping on certain cards like Mine to match what the card does when the orientation is vertical. I left that code out of this PR for now (except for adding it to text_coices), but I could include it if @sumpfork or @nickv2002 would find it helpful.
  • While I was re-implementing inline images, I learned more about various options in reportlab, and added more parameters relating to vertical scaling to get images to line up with the surrounding text. I also added the leading and autoLeading parameters to make some vertical room around the images as needed.
  • I found the mixture of percentages and absolute points in the current size code confusing. I settled on consistently using a factor to multiply by the font size.
  • I used a more object-oriented paradigm with type hints in inline_images.py because I found it easier to understand/use.

A few things that still aren't done:

  • Handling cards that are grouped, like the Knights, or the Augurs, or Plunder/Encampment. The wiki has a page for each individual card, and I think it makes sense to have an entry in the card db for each individual card, along with one for the group, as we currently have in most (but not all) situations (we're missing Knights, Castles and Prophecies, among others). The question that remains is what text to put on the group divider. I can think of three options:
    • Generate it dynamically from the text of each card in the group. This could work well as a follow-up to the refactor work I did in Ungroup cards if the group only has one card in it #567, but it seems like it would probably result in a lot of duplication, especially for the rules text.
    • Retain hand-written and manually-maintained text for the group cards.
    • Write some sort of script that merges the individual card text into the group card's text.
  • Scraping card text in additional languages. This will require loading each individual card's page and parsing the wiki text. Downloading all the pages is straightforward; I've already got a script that does that with a reasonable rate limit. I'm not sure how challenging it will be to scrape the wiki text; it will depend on how consistently formatted they are.
  • I don't think this is the form that I want to merge this in. I'd rather actually update the description in the card db rather than relying on a random external file, this was just easier to manage git conflicts for now.
  • I'd love to also pull down the FAQ section from the wiki for the "Rules", but that's going to be another whole big project. Definitely not in the same PR.

Note regarding the <nobr> tag: the reportlab docs say this will prevent line breaks, but it doesn't actually work. I patched the library code to fix it and plan on submitting a patch to them. In the meantime, the tag is simply parsed and harmlessly ignored using the published versions of the reportlab library. Take a look at how Sailor is rendered with vertical orientation if you want an example.

@sumpfork
Copy link
Owner

I haven't looked in detail, but I'm very happy to replace the current language used to encode things like inline images - it grew organically out of a single initial substitution and has never been revisited. Been bugging me for a while.

Comment on lines +186 to +190
for c in cards:
if not hasattr(c, "wiki_text"):
print(f"{c.name} has no wiki_text")
elif not c.wiki_text:
print(f"{c.name} has falsey wiki_text: [{c.wiki_text}]")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is debugging code that will go away.

if divider_text == "wiki-text":
s.fontSize = 11 # Dominion cards seem to be printed in 11 point font
s.leading = 13 # This is the space between adjacent lines
s.autoLeading = "max"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should leave a comment explaining this one too

return

s = getSampleStyleSheet()["BodyText"]
s.fontName = self.fontStyle["Rules"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was surprised that this wasn't using the Minion font. Any objections to using self.fontStyle["Regular"] instead? It's a little bit of extra work to switch back to Times for the <dash> rendering, since in Minion the dashes have whitespace between them, and to register the font family so bold and italics work, but I've got code locally that does all of that.

Comment on lines +45 to +48
self.blank_image = "highres/Coin.png"
self.question_image = "highres/CoinQ.png"
self.x_image = "highres/Coinx.png"
self.n_image = "highres/Coin{n}.png"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are higher resolution images I downloaded from the wiki. I'm not quite sure where to land on the tradeoff between resolution and generated file size. This should get clarified before merging, either not referring to files that aren't committed, or committing them.

Comment on lines +41 to +43
self.question_image = "coin_small_question.png"
self.x_image = "coin_small_x.png"
self.n_image = "coin_small_{n}.png"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scaling up the small images doesn't look very good when printed. I love the way the cost icons look, with number rendered as text overlaid on the blank image. If anyone's willing to do so, it would be awesome to have a set of coin and debt images with the numbers rendered in that way, to include as PNGs. Maybe I'll open an issue asking for help with that, as I've already got a lot on my plate.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


IMAGES = InlineImages(
coin={
"m": CoinInlineImage(1.2, 1, valign="-28%", resolution="low"),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This valign value, together with the autoLeading, were the key pieces in getting the number on the coin image to line up with the surrounding text.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 1.2:1 aspect ratio is because the images include whitespace on the right hand side. I don't think we need that; it would be better for them to just be square.

Comment on lines +37 to +40
def convert_wiki_markup(text):
"""This whole function is obsolete"""
if not text:
return ""
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left this here for now because it was harmless, but this is purely left-over from my original approach of keeping the current formatting code for inline images.

@mbbush mbbush mentioned this pull request May 21, 2025
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.

2 participants