Separate PDF constraints from hard-coded constraints
Closed this issue · 5 comments
This issue may need a better title, as it's likely to lead to a more general discussion on the use case of a CLI > PDF generator vs. use as a backend step in another pipeline.
This one is a sister issue to #20, as it's inspired by the same blog post.
In that blog post, the implementation is shown to fit all 118 element names in a 32x32 grid as well as the names of the planets in a 7x7 grid. Thinking through some of our earlier discussions, perhaps the backend generator constraints can be decoupled from the CLI/PDF constraints. Mostly in config.py, some specific suggestions to discuss:
- Change
min_puzzle_sizefrom 10 tolen(longest_word). - Split
max_puzzle_sizeintomax_puzzle_size = 32(or more) andmax_PDF_puzzle_size = 25 - Increase
max_puzzle_wordsto some function related to the area of the puzzle. The current limits give 30 words divided into 625 cells, for a density of 0.048. The element names example gives about 118 words in 1024 cells, for a density of 0.115. At the element density, a 25x25 puzzle could support up to 72 words [though it may be unacceptably slow with the number of retries].
In general, have two sets of limits: one for sensible output in a CLI>PDF workflow and another for sensible performance/sanity reasons when this module is run "headless". I'm not sure whether the CLI argparse validation should be hard-coded to sane PDF specifications or if the PDF sanity-checking is done if the output is specified as PDF but it is more lenient with oversize puzzles when outputting to stdout or CSV.
If the validation is separated, perhaps add a check during PDF generation to warn (or error out) if it's greater than 25x25 due to overflowing the page.
The max puzzle size was originally much larger, I only limited it because that was best for the pdf output. Any larger and the puzzle text becomes so small it's unreadable. So having two sets of limits would not be hard to implement, it would just be weird to explain the limit to a user when they have a 50x50 puzzle and wants to save it to a pdf.
As for the minimum size, the only issue with setting it to the length of the smallest word is what happens when someone gives 10 words all 3 characters long. You'd have a tiny puzzle with hardly any words at all. I picked 10 because it was a good size to handle a decent amount of words and makes finding words a little challenging for level 1 puzzles (was thinking about early-age school kids). It also fits well on a PDF sheet. At a size of 10, the current generator easily fits all 7 planets. So we would either need to have a hard minimum (like 10 or 7) or require the user to specify the size (like the script you reference does). I usually like to provide sensible defaults so the user doesn't have to supply so man y arguments just to get any output.
The generator could be changed to fit all seven planets in a 7x7 puzzle but yes it would slow generation down as it would have to generate lots of puzzles to find a variation where all words fit. Currently, it can easily fit 5 of the 7 planets in a 7x7 puzzle.
The script from the blog post actually failed to do the planets in a 7x7 grid in 10 attempts the first 7 times I ran it.
python other.py planets.txt 7 7
...
I failed to place all the words after 10 attempts.The script from the blog post worked better on the list of elements, placing it on 5, and 7 tries when I tested it. The current generator was able to place 114 of the 118 elements in a puzzle of the same size.
All of that to say, I agree that more density would probably be a good thing. I can work on implementing the required changes this week to the config and handling the edge cases with cli, csv, and pdf output.
I forgot I also only limit the puzzle size when using the setter. If you set the size at time of initialization you can make it whatever you want. I originally did this so I could test it but forgot about it. I could simply remove the max size check on the setter and just let argparse handle that on the cli.
Good call about the minimum size. 7 seems like a reasonable hard-coded minimum (and I have no problems with leaving it at its current value of 10).
I think I should have time next week to give a more in-depth look at the implementation & changes.
I completely forgot I built the PDF builder to be fluid so a puzzle sized to 50 works fine (see attached). The puzzle grid and all fonts are calculated based on puzzle size and word count. I'm about to commit an update with puzzle size min @ 5 and max @50. I also updated the max puzzle words to 100.
I'm not sure who wants to do a 50x50 puzzle with 100 words but hey it's there. I ran a test of 35 PDF all sized to 50x50 with 100 words PDF calculations for the wordlist and the key didn't break so I think it's safe.