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 bug fixes #155

Closed PJBrs closed 7 months ago

PJBrs commented 8 months ago

These are various bug fixes. I include a rather big range of spelling errors as well, insofar as they pertain to the doc parts of classes, which can be seen as bug fixes as well :-)

canismarko commented 8 months ago

There's some conflicts due to another PR I just merged. I'll resolve them manually soon.

PJBrs commented 8 months ago

Hi, I was able to rebase these commits myself, I can push them tomorrow.

Incidentally, the new image patches don't seem to render quite so nicely with the MSavage latex style, they're slightly off-center, and they used to leave space for any information in the appearance_text variable, but now the image overlaps with that text. I tried to make a conditional based on use_tex_template, but that didn't seem to work (yet)... I realise that combining an image and an appearance text was never intended, by the way :)

Also, I think dungeonsheets now depends on python-3.10. I'm on python-3.9. and I get:

Traceback (most recent call last):
  File "/usr/bin/makesheets", line 33, in <module>
    sys.exit(load_entry_point('dungeonsheets==0.19.0', 'console_scripts', 'makesheets')())
  File "/usr/bin/makesheets", line 25, in importlib_load_entry_point
    return next(matches).load()
  File "/usr/lib64/python3.9/importlib/metadata.py", line 86, in load
    module = import_module(match.group('module'))
  File "/usr/lib64/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 972, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  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 20, in <module>
    from dungeonsheets.character import Character
  File "/usr/lib64/python3.9/site-packages/dungeonsheets/character.py", line 78, in <module>
    class Character(Creature):
  File "/usr/lib64/python3.9/site-packages/dungeonsheets/character.py", line 112, in Character
    portrait: Path | None = None  # Path to image file
TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'

I need to add from __future__ import annotations to character.py to make it go away.

If necessary I can make this into a bug report?

canismarko commented 8 months ago

I need to add from future import annotations to character.py to make it go away.

Yeah, dropping support for older python versions was part of the other PR that I just merged in, but I want to restore support for at least 3.9. I added your suggestion to master, does it work now?

Incidentally, the new image patches don't seem to render quite so nicely with the MSavage latex style

Mind opening an issue? Then I can tag the other contributor and see how best to fix it?

Thank you!

canismarko commented 8 months ago

CI now passes with python 3.9.

PJBrs commented 8 months ago

I rebased my patches yesterday, here's the updated pull request. And dungeon-sheets indeed works again with python 3.9, thanks!

PJBrs commented 8 months ago

Added one more patch that addresses a capitalisation inconsistency. It's very non-invasive and fits well with the other patches in this PR.

bw-mutley commented 8 months ago

hi @PJBrs , could you please be so kind as to explain me how to better help you with your fixes? I'm not experienced with Github so I don't know how to manage the 4 branches in your fork of this dungeon-sheets. Are all of them supposed to be merged in my local version? Or should I create a separate version for each?

PJBrs commented 8 months ago

@bw-mutley Please see my answer in the other pull request.

PJBrs commented 8 months ago

@canismarko Is there anything that you need me to do about this pull request here? I'd like to get it out of the way in order to move on to more interesting patches. I do have a couple more simple bug fixes in my local tree, but I don't think it helps to push more patches here.

So is there any specific patch that worries you? I personally thought these were all rather straightforward and simple, but maybe I'm missing something :-) In any case, I've been running with these for more than a year now, and I don't think they break or change anything.

canismarko commented 7 months ago

Looks good, thanks.