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

bloodhunter: Fix patron for Order of the Profane Soul #167

Open PJBrs opened 1 month ago

PJBrs commented 1 month ago

The Bloodhunter Feature Selector for OtherwordlyPatron lists the options for the patron choices capitalised. However, the code in FeatureSelector expects them to be lowercased. Make the options lowercase.

An alternative, more robust fix might have been to change some stuff over here: https://github.com/canismarko/dungeon-sheets/blob/5e07e3861700021be75ad08f036c67a110ed53e7/dungeonsheets/features/features.py#L92

So, the user's selection in feature_choices is lowered, but it is not checked against a lowered t.options in FeatureSelector. Then again, I think it only should check against the keys of t.options in FeatureSelector... Anyway, that's all academic for now, this patch just fixes https://github.com/canismarko/dungeon-sheets/issues/134

Usually, I'd have added https://github.com/canismarko/dungeon-sheets/pull/147/commits/c86857530d24ecc52689bec3c0415525bdbc2238 as well, which is a good way to test this fix, but this time I decided to err on the side of caution :-)

PJBrs commented 1 month ago

Just to add the other solution:

diff --git a/dungeonsheets/features/features.py b/dungeonsheets/features/features.py
index 399dded..74b5010 100644
--- a/dungeonsheets/features/features.py
+++ b/dungeonsheets/features/features.py
@@ -89,10 +89,12 @@ class FeatureSelector(Feature):
         new_feat.source = t.source
         new_feat.needs_implementation = True
         for selection in feature_choices:
-            if selection.lower() in t.options:
-                feat_class = t.options[selection.lower()]
-                if owner.has_feature(feat_class):
-                    continue
-                new_feat = feat_class(owner=owner)
-                new_feat.source = t.source
+            for k in t.options.keys():
+                if selection.lower() == k.lower():
+                    feat_class = t.options[k]
+                    if owner.has_feature(feat_class):
+                        continue
+                    new_feat = feat_class(owner=owner)
+                    new_feat.source = t.source
+                    break
         return new_feat

This way, it doesn't matter whether the options in a FeatureSelector are capitalised or lowercase. But don't let that refrain you from merging this PR!

PJBrs commented 2 weeks ago

Switched to the alternative fix!