Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PHB24] Ranger & Paladin Classes + Subclasses #277

Open
wants to merge 17 commits into
base: phb2024
Choose a base branch
from

Conversation

Tel0k
Copy link

@Tel0k Tel0k commented Sep 16, 2024

No description provided.

@Tel0k
Copy link
Author

Tel0k commented Oct 24, 2024

Resolves #258

@kgirtxd kgirtxd linked an issue Oct 24, 2024 that may be closed by this pull request
added missing flavor text, feature replacements and other small fixes
@tregolani
Copy link

tregolani commented Oct 31, 2024

Seeing how I'll be updating my PR with some changes, should we update the "ASI" grants to be pointing towards feats instead (since ASIs are a type of feat now)?

<grant type="Class Feature" id="ID_WOTC_PHB24_CLASS_FEATURE_RANGER_ASI" level="4"/>

should become

<grant type="Class Feature" id="ID_WOTC_PHB24_CLASS_FEATURE_RANGER_FEAT" level="4"/>

@NickVendel
Copy link
Contributor

targeting feats would be type="Feat", and yeah, i think that's the way we gonna do it now.

@tregolani
Copy link

tregolani commented Oct 31, 2024

targeting feats would be type="Feat", and yeah, i think that's the way we gonna do it now.

These are the IDs for the class feature that grants feats. We'd have a <rules> tag inside this element that would then grant the Feats.

I'm mostly just trying to nail down "ID naming conventions" for my PR edit later this weekend.

@NickVendel
Copy link
Contributor

NickVendel commented Oct 31, 2024

but why? If all they do is grant Feats, why not just grant Feats right away?

@tregolani
Copy link

We need the "Class Feature" element so it shows up in the feature list of the class, just like Epic Boons.

@NickVendel
Copy link
Contributor

ah, yeah, then it should still be _CLASS_FEATURE_ just because it's that type.

@Tel0k
Copy link
Author

Tel0k commented Oct 31, 2024

tbh then they should be selections, not grants.. remember that the feature let you choose between ASI or another Feat for which you qualify.. same thing with Epic Boons..

@tregolani
Copy link

Right. We still need the "class feature" element so they show up in the list of class features. We then do a select in the rules of that class feature element to allow the user to pick their feat.

I mostly was asking to change the ID of the "class feature" element to _FEAT instead of _ASI, but in the books it's still called "Ability Score Improvement", so I think my point was moot. We should keep the class feature ID as _ASI.

@NickVendel
Copy link
Contributor

full name would probably be more appropriate, but i won't argue with an abbreviation either

@tregolani
Copy link

Well, what's the standard naming convention we want to go with, moving forward? ASI or ABILITYSCOREIMPROVEMENT?

@Tel0k Tel0k changed the title [PHB24] Ranger Class + Subclasses [PHB24] Ranger & Paladin Classes + Subclasses Nov 3, 2024
@Tel0k
Copy link
Author

Tel0k commented Nov 3, 2024

Resolves #257

Copy link

@tregolani tregolani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all files both in app and via code. Made suggestions where I thought appropriate. Most were styling changes or typo fixes.

core/players-handbook-2024/archetypes/paladin-ancients.xml Outdated Show resolved Hide resolved
core/players-handbook-2024/classes/class-paladin.xml Outdated Show resolved Hide resolved
Comment on lines +238 to +240
<div class="reference">
<div element="ID_WOTC_PHB24_FEAT_BLESSED_WARRIOR" />
</div>
Copy link

@tregolani tregolani Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this <div> reference does not show up in the class description.
image

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and the other divs are working properly though, as they show up fine in compendium entries

<p>You can channel divine energy directly from the Outer Planes, using it to fuel magical effects. You start with one such effect: Divine Sense, which is described below. Other Paladin features give additional Channel Divinity effect options. Each time you use this class’s Channel Divinity, you choose which effect from this class to create.</p>
<p class="indent">You can use this class’s Channel Divinity twice. You regain one of its expended uses when you finish a Short Rest, and you regain all expended uses when you finish a Long Rest. You gain an additional use when you reach Paladin level 11.</p>
<p class="indent">If a Channel Divinity effect requires a saving throw, the DC equals the spell save DC from this class’s Spellcasting feature.</p>
<div element="ID_WOTC_PHB24_CLASS_FEATURE_PALADIN_CHANNEL_DIVINITY_DIVINE_SENSE" />
Copy link

@tregolani tregolani Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as before. <div> elements do not show up in class descriptions.
image

core/players-handbook-2024/classes/class-paladin.xml Outdated Show resolved Hide resolved
Copy link

@tregolani tregolani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some small corrections based on my week of testing in the app. Mostly just small spelling corrections.

core/players-handbook-2024/classes/class-paladin.xml Outdated Show resolved Hide resolved
core/players-handbook-2024/classes/class-paladin.xml Outdated Show resolved Hide resolved
core/players-handbook-2024/classes/class-paladin.xml Outdated Show resolved Hide resolved
core/players-handbook-2024/classes/class-paladin.xml Outdated Show resolved Hide resolved
core/players-handbook-2024/classes/class-paladin.xml Outdated Show resolved Hide resolved
Comment on lines 71 to 72
<grant type="Spell" id="ID_PHB_SPELL_COMMUNE" spellcasting="Paladin" level="17" prepared="true" />
<grant type="Spell" id="ID_PHB_SPELL_FLAME_STRIKE" spellcasting="Paladin" level="17" prepared="true" /> -->

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I meant the 5th level spells granted at lvl 17 don't match the table. Grants are giving Commune and Flame Strike, but should be Legend Lore and Yolande's Regal Presence

core/players-handbook-2024/archetypes/paladin-glory.xml Outdated Show resolved Hide resolved
core/players-handbook-2024/archetypes/paladin-glory.xml Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ranger 2024
3 participants