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
163 stars 67 forks source link

Companions and Weight info. #127

Closed bw-mutley closed 2 years ago

bw-mutley commented 2 years ago

Summary of changes:

major changes: 1) added ranger_beast property to Ranger class in ranger.py. It can be initialized by a tuple of the form: (beast, proficiency), where beast is supposed to be a monster.Monster instance and proficiency is an int. The initializer parses the beast.__doc__ in order to adapt it as indicated by the Ranger's Companion feature.

2) added ranger_beastproperty to Character class. It is initialized in analogous way aswild_shapesfor Druids but using the ranger_beast property of Ranger Class.

3) added a companions property to Character class. It can be initialized through a list of beast names or instances of Monster class. A function companion_resolver was prepended class Character definition to convert str to Monster if needed. (mimicked from the druids wild_shape setter);

4) adapted make_sheets.py to include a list of companions;

5) Added Carry weight and capacity info, parsed from the list of weapons, armor and equipment;

6) Added auxiliary functions and re definitions to stats.py and dice.py.

minor changes: 1) changed make_sheets.py and character class in order to allow a portrait to be included both as a bool or as a filename.

2) created companions form (mimicked from monsters form);

3) fixed small issues with MSavage template;

canismarko commented 2 years ago

Thanks for combining this into one PR. Makes it much easier to review.

I have a few requests and questions before I merge this in.

  1. I don't think there are any tests or documentation included. Please included tests where appropriate and at least some documentation for using the companions feature.
  2. Can companion_resolver in character.py be integrated with the content resolver in content.py?
  3. I like the improved regexs for dice.py. Can those be somehow integrated with the other functions in dice.py, maybe modifying read_dice_str() to use the new version? I don't really want to maintain two sets of regexs that do the same thing.
  4. Also on the topic of your dice_roll_mean function, it looks like "1d20" would not match the regex since there are two digits after the d and \d wouldn't match one digit (same with 10d6 and 1d6+15). Could you make sure there is a test for something like dice_roll_mean("1d20") and the other examples I provided?
  5. In make_sheets.py it looks like create_gm_spellbook would return TEX even if HTML was being generated, which would make the HTML invalid. Can you confirm and fix this? The HTML output already includes a spellbook for the GM content, so maybe tap into that mechanism?
  6. In character.py, could you modify lines 792 and 795 so that they don't rely on implicit truth values? Minor quibble I know, but it will make it clear what's actually being tested. E.g. instead of and self.magic_items, and len(self.magic_items) > 0.

Cheers!

bw-mutley commented 2 years ago
1. I don't think there are any tests or documentation included. Please included tests where appropriate and at least some documentation for using the companions feature.

Sorry, I will include it.

2. Can `companion_resolver` in `character.py` be integrated with the content resolver in `content.py`?

Yes, I can do it. I was in doubt where to put first, but now you saying it is much clear.

3. I like the improved regexs for dice.py. Can those be somehow integrated with the other functions in dice.py, maybe modifying `read_dice_str()` to use the new version? I don't really want to maintain two sets of regexs that do the same thing.

I will do that too.

4. Also on the topic of your `dice_roll_mean` function, it looks like "1d20" would not match the regex since there are two digits after the d and `\d` wouldn't match one digit (same with 10d6 and 1d6+15). Could you make sure there is a test for something like `dice_roll_mean("1d20")` and the other examples I provided?

Yes, another one to fix,

5. In `make_sheets.py` it looks like `create_gm_spellbook` would return TEX even if HTML was being generated, which would make the HTML invalid. Can you confirm and fix this? The HTML output already includes a spellbook for the GM content, so maybe tap into that mechanism?

I wil check it. IIRC, I used a format function to apply the correct form.

6. In `character.py`, could you modify lines 792 and 795 so that they don't rely on implicit truth values? Minor quibble I know, but it will make it clear what's actually being tested. E.g. instead of `and self.magic_items`, `and len(self.magic_items) > 0`.

Ok, seens better indeed. I will be offline this weekend due to a trip, but I fix and modify those when I am back next week. I guess you can close this one and I open a new one when I'm ready?

Cheers!

Thanks for the lessons :)

bw-mutley commented 2 years ago

Hi Mark, I was coming back to this updates. About this:

  1. Can companion_resolver in character.py be integrated with the content resolver in content.py?

At first read I thought you wanted me to put companion_resolver in content.py, but now I am in doubt: do you want the _resolve_mechanic method to be able to resolve a companion too? Also, is there a better place to put the companion_resolver? Initially, I just mimicked what I could understand from other methods, e.g. the wear_armor under class character. Please explain me a bit more, as I am not so experienced programmer.

==EDIT== I think I understand now. You are asking to use self._resolve_mechanic instead of creating a new function. I think I am ready to go now, just have to run some tests here.

canismarko commented 2 years ago

Hi, BW.

What I'm thinking is that in def companions(...) setter, instead of using companion_resolver(), you just call self._resolve_mechanic() with the appropriate arguments (e.g. SuperClass=monsters.Monster). This is what wear_armor does. _resolve_mechanic does something very similar to companion_resolver, but is more robust. If there's some feature that you need that's in compaion_resolver() but not in _resolve_mechanic, let me know and I can add it (with tests).

Cheers, Mark

On Fri, Mar 18, 2022 at 3:42 PM bw-mutley @.***> wrote:

Hi Mark, I was coming back to this updates. About this:

  1. Can companion_resolver in character.py be integrated with the content resolver in content.py?

At first read I thought you wanted me to put companion_resolver in content.py, but now I am in doubt: do you want the _resolve_mechanic method to be able to resolve a companion too? Also, is there a better place to put the companion_resolver? Initially, I just mimicked what I could understand from other methods, e.g. the wear_armor under class character. Please explain me a bit more, as I am not so experienced programmer.

— Reply to this email directly, view it on GitHub https://github.com/canismarko/dungeon-sheets/pull/127#issuecomment-1072782026, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASBDGGDDVQ4BRATE4HQJ5LVATTCTANCNFSM5QOSKQMQ . You are receiving this because you commented.Message ID: @.***>

--

No trees were killed in the sending of this message. However a large number of electrons were terribly inconvenienced.