canismarko / dungeon-sheets

A tool to create character sheets and GM session notes for Dungeons and Dragons fifth edition (D&D 5e).
https://dungeon-sheets.readthedocs.io/en/latest/
GNU General Public License v3.0
161 stars 66 forks source link

Assorted changes and a couple of fixes to the fancy sheets #147

Open PJBrs opened 1 year ago

PJBrs commented 1 year ago

Hi, I just found dungeon-sheets and I like it very much! I had been writing my character sheets by hand in latex using MSavage's style, but that isn't very automated.

I hope to contribute a little bit with a couple of ideas that are focused on the features pages. I think basically the patches speak for themselves, and for now I'd mainly like to get some reactions as to whether these patches are actually wanted, and perhaps what other / further directions to take with this.

I can also supply an example of the type of layout that I'm trying to achieve. I'm keeping a small wishlist of my own, with items like:

I should add that I can code only a little bit, but I tested the changes on all the example sheets.

Anyway, I really hope to hear whether this type of work is appreciated!

bw-mutley commented 1 year ago

Hi, I just found dungeon-sheets and I like it very much! I had been writing my character sheets by hand in latex using MSavage's style, but that isn't very automated.

Hi there, @PJBrs ! About this part, did you try using MSavage from Dungeonsheets? I've contributed to this part, let me know of any issues I can help.

Regards, Mauricio

PJBrs commented 1 year ago

Hi @bw-mutley!

Thanks! I think most of the more structural changes apply to the features pages, hardly any to MSavaga's style. I've tested both though.

If you have any suggestions though for continuing this PR, please let me know. In the meantime, I've added one more substantial change that renders the monster blocks more in keeping with the RPGTex D&D style. Problem is, my repo now is one long list of assorted changes, including spelling fixes, small additions, small enhancements and substantial formatting changes that affect how everything looks...

I'm force-updating the PR now, and then I'll see whether I can sort it better. In the meantime, any suggestion at all would be appreciated!

PJBrs commented 1 year ago

So, I sorted all my patches. The first five patches are trivial spelling and whitespace fixes. The next four add some equipment and offer very small corrections:

The "Fix" pdfbookmarks patch does not fully work, but as far as I can tell, it's less error-prone than the current behaviour.

The next five patches all represent small enhancements:

All the remaining patches offer rather substantial changes that I personally like, and I hope for some feedback on them.

These two patches sort spells by name and level and add headings for each spell level in the features pages:

These two patches sort feats by origin and level (character feats, class feats, background feats and racial feats) and add associated headers in the feature pages:

I tried to write these trying not to be invasive and ended up doing quite a lot of pattern matching in latex. Could have done them simpler.

The final two patches all concern relatively minor aesthetic changes that I liked:

Basically, I'm looking for comments on all of these, regarding:

bw-mutley commented 1 year ago

Problem is, my repo now is one long list of assorted changes, including spelling fixes, small additions, small enhancements and substantial formatting changes that affect how everything looks...

exactly how my local files are at the moment. I've made several additions and corrections too, but can't keep track of everything in commits like you did - seens like I have a lot to learn on how to keep with people here. To properly contribute again, I will - at some point - have to dowload the master branch again. I've seen many issues in my local files which were probably fixed.

canismarko commented 1 year ago

Hi, @PJBrs.

Apologies for the slow response. My day job got unexpectedly chaotic. Thank you for all the hard work and for organizing these commit in a useful way. I will take a closer look when things calm down a little bit and I get my feet back underneath me.

PJBrs commented 1 year ago

Hi @canismarko , take your time! By now it should be clear that I couldn't help myself anyway :-)

I think, given all patches I now added, the majority should be sufficiently palatable, but others also represent design choices that you might disagree with, and then there's the matter of whether my implementations are agreeable, aside from their effects.

I hope things will calm down soon!

PJBrs commented 1 year ago

@canismarko I noticed you triggered some tests. The latex ones fail because of the changes I made. The below patch adapts the tests to my other patches (and makes them pass). I'll look into the other failing tests another time.

--- a/tests/test_make_sheets.py
+++ b/tests/test_make_sheets.py
@@ -235,7 +235,7 @@ class TexCreatorTestCase(unittest.TestCase):
     def test_create_features_tex(self):
         char = self.new_character()
         tex = make_sheets.create_features_content(character=char, content_suffix="tex")
-        self.assertIn(r"\section*{Features}", tex)
+        self.assertIn(r"\section*{Class Features}", tex)
         self.assertIn(r"\subsection*{Martial Arts}", tex)

     def test_create_magic_items_tex(self):
@@ -248,7 +248,8 @@ class TexCreatorTestCase(unittest.TestCase):
         char = self.new_character()
         tex = make_sheets.create_spellbook_content(character=char, content_suffix="tex")
         self.assertIn(r"\section*{Spells}", tex)
-        self.assertIn(r"\section*{Invisibility}", tex)
+        self.assertIn(r"\subsection*{2nd-Level Spells}", tex)
+        self.assertIn(r"\subsubsection*{Invisibility}", tex)

     def test_create_infusions_tex(self):
         char = self.new_character()
PJBrs commented 1 year ago

I added the test I posted previously in the comments. Also, I reordered the new patches a bit to leave the DND-monster formatting at the end. I tested all monsters I could, made many formatting fixes as well, only remaining thing is to add bulleted lists.

And I wrote one more patch to enable DND-tables throughout, after I conjured up a regex that translated docutil's supertabular table header to a dndtable header. Next step is probably to see if I can improve any table I find.

PJBrs commented 1 year ago

Rewrote the monster formatting patch. I think it now is on par with, or better than regular latex.

I don't think I really have anything else to add anymore.

I didn't test all tables, but, in any case, the dndtables are less wide than the regular ones, so any text overflow will have been present anyway, and might be prevented now.

If necessary, I can upload some example character sheets... Would that help?

I did notice some potential enhancements, such as putting lair actions of dragons in a separate heading, just like aboleth, as well as formatting the breath weapons a bit more (\emph{breath weapon}). All that would be rather easy.

And one bug, perhaps, I can't seem to add certain monsters as companions, e.g.: UserWarning: Companion 'Will-o'-Wisp' not found. Please add it to monsters.py

EDIT

Let's let this sit here for a while, while I test more... Just force-updated because I clobbered one of my previous fixes. ;'-)

EDIT 2 - 13 feb

Fixed a small bug in the dndtable header regex.

PJBrs commented 1 year ago

@canismarko @bw-mutley I think I really have no other patches to add right now. I have put a character sheet online to demonstrate the effects of all the patches in this PR together, note the feat and spell headings, the dndtables with text wrapping in cells, and the monster box formatting (I've given this character, a druid, a sizeable amount of companions).

@canismarko How might I make this PR easier to review? I can, for instance, make separate PRs for different groups of patches, for instance as follows:

  1. Spelling fixes / bug fixes
  2. MSavage style sheet enhancements
  3. Other small enhancements
  4. Monster formatting
  5. DNDtables
  6. Spellbook headings per spell level
  7. Separated headings for character, class, racial, and background feats

@bw-mutley Perhaps you'd be interested to take a look at the various patches that deal specifically with the MSavage style sheet?

Finally, I tried to run the dungeonsheets tests, which complain about some missing html pages, but the latex tests complete fine:

$ pytest3 tests
============================================================================================== test session starts ==============================================================================================
platform linux -- Python 3.9.16, pytest-7.2.1, pluggy-1.0.0
rootdir: /home/pj/Src/dungeon-sheets/dungeon-sheets
plugins: cov-3.0.0
collected 125 items                                                                                                                                                                                             

tests/test_character.py ........................                                                                                                                                                          [ 19%]
tests/test_content_registry.py .....                                                                                                                                                                      [ 23%]
tests/test_dice.py ......                                                                                                                                                                                 [ 28%]
tests/test_encounter.py ..                                                                                                                                                                                [ 29%]
tests/test_epub.py ...                                                                                                                                                                                    [ 32%]
tests/test_equipment_reader.py .                                                                                                                                                                          [ 32%]
tests/test_features.py .......                                                                                                                                                                            [ 38%]
tests/test_html.py .s.........                                                                                                                                                                            [ 47%]
tests/test_latex.py .s.........                                                                                                                                                                           [ 56%]
tests/test_magic_items.py ..                                                                                                                                                                              [ 57%]
tests/test_make_sheets.py ...FFF.FFF...........                                                                                                                                                           [ 74%]
tests/test_monsters.py ....                                                                                                                                                                               [ 77%]
tests/test_multiclass.py ....                                                                                                                                                                             [ 80%]
tests/test_random_tables.py ..                                                                                                                                                                            [ 82%]
tests/test_readers.py .........                                                                                                                                                                           [ 89%]
tests/test_spells.py ....                                                                                                                                                                                 [ 92%]
tests/test_stats.py ......                                                                                                                                                                                [ 97%]
tests/test_weapon.py ...                                                                                                                                                                                  [100%]

[snip!]
============================================================================================ short test summary info ============================================================================================
FAILED tests/test_make_sheets.py::EpubOutputTestCase::test_character_file_created - jinja2.exceptions.TemplateNotFound: preamble.html
FAILED tests/test_make_sheets.py::EpubOutputTestCase::test_character_html_content - jinja2.exceptions.TemplateNotFound: preamble.html
FAILED tests/test_make_sheets.py::EpubOutputTestCase::test_gm_file_created - jinja2.exceptions.TemplateNotFound: preamble.html
FAILED tests/test_make_sheets.py::HtmlCreatorTestCase::test_create_extra_gm_content - jinja2.exceptions.TemplateNotFound: extra_gm_content.html
FAILED tests/test_make_sheets.py::HtmlCreatorTestCase::test_create_monsters_html - jinja2.exceptions.TemplateNotFound: monsters_template.html
FAILED tests/test_make_sheets.py::HtmlCreatorTestCase::test_create_party_summary_html - jinja2.exceptions.TemplateNotFound: party_summary_template.html
============================================================================= 6 failed, 117 passed, 2 skipped, 5 warnings in 38.05s =============================================================================
bw-mutley commented 1 year ago

Hi there, @PJBrs , sorry for the late reply.

Perhaps you'd be interested to take a look at the various patches that deal specifically with the MSavage style sheet?

Thanks for the contribution. I will fork your branch and try to do it this week.

PJBrs commented 1 year ago

@bw-mutley Great!

Let me know if you need the MSavage style sheet changes in a separate branch...

bw-mutley commented 1 year ago

@bw-mutley Great!

Let me know if you need the MSavage style sheet changes in a separate branch...

Hi there, PJ! Only now I could have some spare time to work on it. I've made a small comment about the sorcerer -> sorceror change, please take a look. About the MSavagesheet: I will need some more time since I've changed my PC and ended missing some tex files needed.

PJBrs commented 1 year ago

Hi @bw-mutley I thought sorceror was the correct spelling, but indeed, not only the PHB but also standard dictionaries prefer sorcerer. I wonder whether we should revert or reverse that spelling patch ;)

bw-mutley commented 1 year ago

Hi @bw-mutley I thought sorceror was the correct spelling, but indeed, not only the PHB but also standard dictionaries prefer sorcerer. I wonder whether we should revert or reverse that spelling patch ;)

I suppose it is up to @canismarko to decide. But my suggestion is to push a PR fixing all 'sorceror' ocurrences into 'sorcerer'.

PJBrs commented 1 year ago

I stashed that sorcerer spelling fix for now.

Unfortunately, I had some uncommitted local changes that have now been added to this PR, one to make more tables nice and beautiful, and a compile fix for Monster Slayer Ranger subclass... Sorry!

@canismarko Like I said, let me know if you want me to separate this PR into smaller, topically consistent PRs...

PJBrs commented 8 months ago

I noticed there has been some activity in this repo again, and I bumped into some small bits while making a new character, and I decided to reorganise this pull request. Hopefully, the different patches, and how they fit together, make more sense now.

Everything after Latex feature pages is potentially rather invasive. I hope at least some of these are ripe for cherry-picking.

canismarko commented 8 months ago

Sorry I haven't gotten to this in so long. It is a lot to look through all at once. Perhaps it would be better in a few different PRs? The spelling mistakes and stuff like that I can easily take. New content is great too.

I noticed there's a couple commits that change the order that features and spells. Maybe this should be a command line option? I can see why someone might want spells in level order, but I also can see why someone might want them in alpha order. I originally put them in alpha order because that's how I look them up when I want to know what they do.

Would it still be useful to you if it were --spell-order=level and --feature-order=natural or something like that?

PJBrs commented 8 months ago

I can make a couple of separate pull requests, starting with three relatively un-invasive ones. A first one could simply be the spelling fixes and the four obvious bug fixes:

I could make a second one with miscellaneous small additions:

And I could make a third one that only does some text formatting changes:

It would seem to me that these are the changes that ought to be relatively trivial to review :) I'd like to see that you make of these patches first, and then I can see how to submit the more invasive enhancements. To be sure, I can try to make that commandline option! And I'm not entirely happy with how I implemented ordering feats anyway.

So, how does that sound?

canismarko commented 8 months ago

That sounds wonderful, thank you for your patience. It will be great to finally get this all merged in.

PJBrs commented 8 months ago

Hi @canismarko - I've just created three separate pull requests. They all apply to master, so I don't think it matters at all in which order you pull them. They are 11 patches in total. Hope this helps!

PJBrs commented 8 months ago

Hi @canismarko , I rebased my entire local tree and force pushed it over here. Incidentally, I also noticed that there have been quite some updates in MSavage's dndtemplate itself, which required some additional work on our side (see https://github.com/canismarko/dungeon-sheets/pull/147/commits/e2ba58eb9bb5b25bb75c9bc311e942952e1bce5a).

I'd like to propose the following three steps:

  1. See what you make of my open pull request here - https://github.com/canismarko/dungeon-sheets/pull/155
  2. Open a bug report about image positioning in the MSavage latex template
  3. Open an additional pull request with the two patches that enhance how features are applied)
  4. Open an additional pull request for the now seven patches that touch upon the MSavage latex template

How does that sound?

@bw-mutley, if possible, I'd really like you to check those seven patches for the MSavage latex template. If you need it, I have a local ~/texmf tree that includes the dndtemplate and the rpgtex latex format; I could package that for you, if needs be.

bw-mutley commented 8 months ago

@bw-mutley, if possible, I'd really like you to check those seven patches for the MSavage latex template. If you need it, I have a local ~/texmf tree that includes the dndtemplate and the rpgtex latex format; I could package that for you, if needs be.

Hi there, @PJBrs ! Thanks for the awesome contribution. I've already read that and approve both the idea and implementation, just didn't test it yet. If you already tested, I am ok with that. If you want me to test, I will ask for you to wait until the weekend.

PJBrs commented 8 months ago

Hi @bw-mutley that's nice to hear! I wouldn't like to clobber this repo with pull requests, but I did prepare a separate branch with only MSavage latex template fixes. And it would be nice, I think, for you to test it a little bit. I read in my other pulll request that you had some trouble with git. I suggest some, if not all, of the following:

git clone https://github.com/canismarko/dungeon-sheets/
cd dungeon-sheets/
git remote add pjbrs https://github.com/PJBrs/dungeon-sheets
git fetch pjbrs
git checkout -B pjbrs/MSavage-latex-template

This will get you a clean repo with only my changes to MSavage's latex template. With testing, it might be easier for @canismarko to decide to pull these changes. I've made a character sheet that displays the working of the seven patches that specifically affect MSavage's latex template. Compare pictures to see differences.

Character without MSavage latex template patches: image

Character with MSavage latex template patches: image

Here's the source (remove .txt from the filename): llaewin.py.txt

bw-mutley commented 8 months ago

hi there, PJ! Thanks for your instructions, I've followed and proceeded with tests here. But I think I'm still missing something, because I wasn't able to reproduce the sheets you've posted. Most noticiable difference is the attributes fields, which is shown with the bonuses in the larger space in your example but not in mine. Checking further, I found the template in my local files are not the same as you have in your branch here:

https://github.com/PJBrs/dungeon-sheets/blob/MSavage-latex-template/dungeonsheets/forms/MSavage_template.tex

Could you help me to figure what is wrong in our process? BTW, reading the MSavage_template.tex above I would guess it wouldn't reproduce the ones you have posted.

PJBrs commented 8 months ago

Hi @bw-mutley, thanks for your tests!

I think two things. First, are you on the latest version of MSavage's dndtemplate? Some of my changes are specifically focused on dealing with recent changes on his side, and they also switched the locations of the attributes and the modifiers.

With regard to my instructions, I think I made a small mistake in the last command. Best to start over as follows:

git clone https://github.com/canismarko/dungeon-sheets/
cd dungeon-sheets/
git remote add pjbrs https://github.com/PJBrs/dungeon-sheets
git fetch pjbrs
git checkout pjbrs/MSavage-latex-template

Verify with

git branch

or

git log

The last commit you should see is MSavage latex template: set XP to milestone if -1

If all else fails, I can send you a tarball with that branch, let me know.

P.S., My patches also touch other files than MSavage_template.tex .

bw-mutley commented 8 months ago

P.S., My patches also touch other files than MSavage_template.tex .

Indeed, I should have suspected this to be the case after reading your PR in his repo! I will update with the results as soon as I can test it properly.

PJBrs commented 8 months ago

@bw-mutley Small heads up: I needed to add one more patch. I just noticed that MSavage's dndtemplate now treats proficiencies a bit differently, which caused the proficiency markers to disappear. My last patch makes them appear again.

You can get that patch with:

git fetch pjbrs
git checkout pjbrs/MSavage-latex-template

But while doing this...

  1. I found a better way that actually makes full use of half-proficiency and full-proficiency markers.
  2. I noticed that the remarkable athlete feat also, more or less, results in half proficiency, but then I noticed that it actually isn't added to initiative, whereas the feat text suggests that you can add half your proficiency bonus to any check that includes dex, str, or wis, which would include initiative as well, since that is a dex check.

@canismarko, should I open a bug report that remarkable athlete is only implemented for skill checks, but not for initiative?

So, I'm not pushing that better option now. Or maybe I should. Grmm... OK, decided to push it. Last patch is now MSavage latex template: Make full use of proficiency markers.

PJBrs commented 8 months ago

@bw-mutley ... and I found one final formatting issue in the MSavage latex sheet, so please do one more

git fetch pjbrs
git checkout pjbrs/MSavage-latex-template

Last patch now is MSavage latex template: Prevent spurious newlines.

I promise that I won't add anything else to my MSavage-latex-template branch. It got a little out of hand after I discovered how much MSavage's dndtemplate had progressed, and the extent to which dungeonsheets hadn't kept up with that.

In any case, that branch now is 10 patches ahead of canismarko/dungeon-sheets:master, and only touches code that is used in the MSavage latex template. As a check, I just built all examples against it, everything compiles, and also looks fabulous ;-)

Plan: @bw-mutley to test my MSavage-latex-template branch, and if he approves, I'm turning it into a pull request.

P.S., I found one remaining tiny issue, but it can wait after testing. It's too small to clobber things further.

And a picture of the final result! (This is what testing should produce now.) image

bw-mutley commented 8 months ago

Hi there, PJ! I will try to check this up carefully in the weekend.

===UPDATE=== Hi there, @PJBrs ! I've downloaded the latest MSavage repository, including your latest patch, and unfortunately, I couldn't make it work right now as is. Reason is, since my last instalation before this one, I've made some changes in the package and in my system as well, so I will have to build my own fix to make it run locally and I am affraid it will take longer than I thought.

I really want to come back and contribute to this project, but I won't be able to actually do so until next month and I don't want to hinder your contributions neither. So, I suggest you and @canismarko move on without my check, at least for now.

PJBrs commented 7 months ago

@bw-mutley OK, thanks for the heads up. In the meantime, I've also had another look at the dndtemplate itself, and @matsavage and I try to put together a half-caster spellsheet for the latex template. I'll ask him if he might be willing to test my MSavage latex template dungeonsheets branch. And otherwise, I'll turn it into a new pull request anyway. By now, I've tested my patches with every example in dungeonsheets, and they all render fine!

canismarko commented 7 months ago

@PJBrs Since you split this off into other PRs, is this still one you're interested in merging?

matsavage commented 7 months ago

@canismarko this may be due to me not knowing if the half caster sheet would be merged into my repo in time.

It’s now merged on my end so it may well be both, but I’ll leave for @PJBrs to clarify.

PJBrs commented 7 months ago

Hi @canismarko

No, this can definitely not be merged in the current form, but I hope you don't mind using this PR for discussion.

I was thinking about which steps to take next. I think it really helped to snip this PR in smaller pieces. Right now, I'd like to propose the following PRs next:

In the meantime, I'm still working on some improvements in my real changes to the feature pages:

  1. Ordering spells by level. As you suggested, I have now implemented a command-line option to order spells by level, and I think I should implement associated headings using jinja (my current patch uses latex, but this causes clutter.
  2. Ordering features by type. This looks very very nice, but I suspect that you'd like a commandline option for this one as well. Also, my patches don't quite work like I want them to.
  3. More fancy decorations, specifically turning dndtables on everywhere in the fancy feature pages (this patch is good) and a patch that uses every available bit of the RPGtex latex style to format monster blocks (this patch makes monsters look very very very nice).
  4. Some nice-to-have fixes here and there (e.g., fix pdfcreation with create-character; patch for portrait centering with MSavage template, beginning feature pages at page 4 instead of page 1)

So, I'll open the first three PR's somewhere this weekend, hopefully after @matsavage has had a look at them, and otherwise - I think I've tested them so often now, they just ought to be good!

And then the half-caster sheet! So I asked @matsavage his opinion about having a nice halfcaster sheet for his template, and we've whipped one up last week, ready for use by dungeonsheets. I haven't implemented that on my side, but it looks rather easy, since we just need to port the halfcaster-sheet code from the reguler fillable pdf to the MSavage code paths.

Does this work for you? I'll force-push this one here after I update my local branch (point is, I finally discovered whitespace control with jinja2 and how that affects latex output; it makes everything so much better, but now I also know that quite some of my patches were too hacky).

canismarko commented 7 months ago

@PJBrs No problem at all, just making sure there wasn't anything for me to review right now. Your plan sounds good. For #2, yes I think it makes sense to have it be a command line option, thanks.

PJBrs commented 7 months ago

I rebased all my branches and created three PRs, as discussed above!

PJBrs commented 7 months ago

I added two very very very small enhancements to my previous PRs and I finished a sufficiently working patch for ordering feats by type as well. I'll add the MSavage halfcaster sheet next.

@canismarko For now I'll first wait for your reviews of my other three PRs. After that, I can raise a couple more, with 1) more bug fixes, 2) ordering spells by level, and 3) more dnd formatting in the fancy sheets.

PJBrs commented 7 months ago

... and I implemented the halfcaster sheet for @matsavage 's template!

@canismarko , for the MSavage latex template I have the patch for portrait and symbol placement in this branch (fixes https://github.com/canismarko/dungeon-sheets/issues/157), and the halfcaster spellsheet implementation. I can either leave them here for a while, or I can add them to my MSavage latex template PR. Let me know what you prefer.

Also, I didn't mean to push @Valentin-Metz 's patch that fixes pdfbookmarks when adding images; I had it cherry-picked in my local branch for testing. I can remove it from this PR / branch, but then again, all the attributions are intact, so, I didn't see a problem myself.

PJBrs commented 7 months ago

I reordered and reorganised the patches in this PR. Together, they now constitute everything that I wanted to add. I added two patches for pdfbookmarks in the character, backstory and spellcasting sheets (both the MSavage template and the fillable pdfs). And I reworded some commit messages to make them easier to understand. All tests pass, all examples build, in latex, in epub. I don't think I can do these patches any better than this. Or at least, not without help :-)

After all this, I can think of only one more "aesthetic" change, and that is switching to lualatex for the MSavage latex template, which is way faster than xelatex, but does add a dependency on the luapstricks latex module.

PJBrs commented 6 months ago

So, I appear to have messed up the halfcaster patch. It only worked in combination with my order-spells-by-level patch, and even then by chance. I've added a bugfix and will open two more small PRs, one with a couple of fixes and one with a small number of minor enhancements.

In the meantime, it seems to me that one of the showstoppers of using MSavage's dnd template is the problem of the font path, is that right? So, I thought two things; on the one hand, it might be nice to have a fallback font in the dndtemplate.sty file. On the other hand, it shouldn't be too hard to locate the kalam font, with something like the following, but cleaner: ln -s 'dirname $(kpsewhich dndtemplate.sty)'/template ./

(Apostrophes should have been backticks, but I don't know what escape character to use here...)

PJBrs commented 6 months ago

The following seems to find the kalam font in the dndtemplate folder: TTFONTS=.:$(kpsewhich dndtemplate.sty | sed s/dndtemplate.sty//): makesheets

PJBrs commented 6 months ago

So I just succeeded in adding the MSavage dndtemplate.sty itself as a submodule to dungeonsheets, including the right path for the kalam font. It's in my wip branch over at my repo, I'll add it here later. I think that would fix https://github.com/canismarko/dungeon-sheets/issues/92

PJBrs commented 6 months ago

I added the MSavage latex template as a submodule. I also added it to the tests, and it completes fine. So, I think that, with this, everybody should be able to use the MSavage latex template, regardless of whether they have the kalam font installed or not.

PJBrs commented 5 months ago

Recently, that is, since I cleaned up my build script and removed the build directory before building dungeonsheets again, I get the following error running makesheets:

$ makesheets 
Traceback (most recent call last):
  File "/usr/bin/makesheets", line 5, in <module>
    from dungeonsheets.make_sheets import main
  File "/usr/lib64/python3.9/site-packages/dungeonsheets/__init__.py", line 13, in <module>
    from dungeonsheets import background, features, race, spells, weapons, mechanics
  File "/usr/lib64/python3.9/site-packages/dungeonsheets/mechanics.py", line 4, in <module>
    from dungeonsheets.content import Content, Creature
  File "/usr/lib64/python3.9/site-packages/dungeonsheets/content.py", line 17, in <module>
    __version__ = read("../VERSION").strip()
  File "/usr/lib64/python3.9/site-packages/dungeonsheets/content.py", line 14, in read
    return open((Path(__file__).parent / fname).resolve()).read()
FileNotFoundError: [Errno 2] No such file or directory: '/usr/lib64/python3.9/site-packages/VERSION'

I don't see that VERSION file getting packaged (nor did I think that it belonged above the dungeonsheets top directory).

This might be fallout from the switchover to pyproject.toml? In any case, it can be fixed with:

diff --git a/dungeonsheets/content.py b/dungeonsheets/content.py
index 6fc37ce..1a5e51e 100644
--- a/dungeonsheets/content.py
+++ b/dungeonsheets/content.py
@@ -2,6 +2,8 @@

 import warnings
+import importlib.metadata
+
 from abc import ABC
 from pathlib import Path

@@ -9,13 +11,7 @@ from dungeonsheets import exceptions
 from dungeonsheets.stats import Ability, ArmorClass, Initiative, Speed, Skill
 from dungeonsheets.content_registry import find_content

-
-def read(fname):
-    return open((Path(__file__).parent / fname).resolve()).read()
-
-
-__version__ = read("../VERSION").strip()
-
+__version__ = importlib.metadata.version(__package__)

 class Content(ABC):
     """A base class for all D&D 5e content types.

I can add this to a PR, or create a new one, if you want to. It seems to me that the VERSION file itself is a bit redundant, this way... Perhaps it can be removed?

===EDIT===

It looks like the old setup.py had this:

    package_data={
        "dungeonsheets": [
            "forms/*pdf",
            "forms/*.tex",
            "forms/*.txt",
            "modules/DND-5e-LaTeX-Template/*",
            "modules/DND-5e-LaTeX-Template/lib/*",
            "modules/DND-5e-LaTeX-Template/img/*",
            "../VERSION",
        ]
    },
matsavage commented 5 months ago

Since this is such a big change to the template, do we think it’s a good time to remove my name from it, since it’s certainly not just me who made the template at this point.

(It wasn’t me who named it this in the first place btw)

PJBrs commented 5 months ago

@matsavage What name would you propose? Something like fancy-latex-form.tex?

matsavage commented 5 months ago

@PJBrs I'm easy, how about latex_character_sheet_template.tex, to differentiate from the pdf templates?

PJBrs commented 5 months ago

@canismarko Right now, this PR consists of 23 commits, that cover four types of changes.

First, five patches aim to automatically assign racial spells through their associated feats, with some associated clean-ups and some feature fixes (this PR: https://github.com/canismarko/dungeon-sheets/pull/159).

Second, there still are a couple of relatively simple fixes here, fixing the version string for the current build system, a huge patch consisting only of spelling fixes, and a separate spelling fix for sorceror --> sorcererer (three patches).

Third, a total of eight patches all aim to make the fancy latex feature pages more beautiful. These commits all start with Latex feature pages:. They add:

  1. Spell sorting by level (and an associated commandline switch);
  2. Feat sorting by type (and an associated commandline switch);
  3. Spellcasting details added to the spell spell pages;
  4. Dndtables for all tables in the fancy sheets and more dndformatting of monsters (this PR: https://github.com/canismarko/dungeon-sheets/pull/165).; and
  5. Continued page numbering in the feature pages.

Fourth, and finally, I have commits that together bring the MSavage latex form (renamed to latex character sheet) at feature parity with the fillable pdf form, in the sense that it contains the same information, doesn't require additional user fiddling by turning it into a submodule, and then some more formatting things as well. The background of this is that @matsavage and I have gotten rid of some issues with transparency and we've added some code that scales font size in order to prevent text box overflow, similar to what the fillable pdf form does.

I'm especially happy with that last series. It would be very nice if it could see some testing. Obviously it works over here, but I didn't test with docker and I didn't test with windows.

matsavage commented 5 months ago

All looks good to me, I have confirmed this works with current versions of texlive

I have raised a PR to this branch to update the Dockerfile, as luatex and pstricks are required to be able to build the character sheets in with the fancy template.

I have updated the default makesheets arguments to use the fancy latex sheets, but there is probably some debate to be had about which should be the default, so feel free to revert that.

Note running the docker image with the following command

$ docker run -it -v $(pwd):/build ghcr.io/canismarko/dungeon-sheets:master --tex-template

will then use the template.

It may be worth updating the README.md to add this information, or maybe to have a section on all the possible output methods, with a sample image of each of them.

There should probably be some debate about which version of the sheet is built by the docker image, or if there should be

matsavage commented 5 months ago

It may also be worth adding a github actions step to run the docker image against a couple of the examples with a range of different settings to verify it works before pushing to docker hub.