mysurvive / pf2e-thaum-vuln

Improvement for Thaumaturge Exploit Vulnerability
MIT License
10 stars 11 forks source link

Changes for Lantern and Intensify effects. #139

Closed xyzzy42 closed 3 months ago

xyzzy42 commented 3 months ago

Standardize on the slug "intensify-vulnerability-{slug}" for the effects of an intensified implement. This will give the roll option "self:effect:intensify-vulnerability-{slug}" to be checked in rule elements. The existing effects were not consistent on this.

Standardize on the name "Effect: Intensify Vulnerability (Implement)" for the effects. This is most like how other effects that come in different versions are named.

Automate some more of the Lantern effects:

Use an AELike that calculates the radius of the light, saves it to an actor flag. Use just one Aura and one TokenLight, with the calculated radius, for the three different sizes.

The Aura RE will push new effects to allies and enemies in the aura. Allies will get "in-token-light-ally" while enemies get "in-token-light-enemy".

This way "target:effect:in-token-light-enemy" can be used to determine if a Lantern bonus should apply to the roll based on the target being in the light.

The ally effect gives them the +1 bonus to RK and Perception. The enemy effect has a -2 to Stealth and Deception.

The IV effect has two AdjustModifier RE to change the value for the thaum's bonus from +1 to +2, but only for the target marked by EV. It also relabels the bonuses to "… (Intensified)".

The selector for the RK bonus is changed from "all" to "skill-check". This way it doesn't show for attack rolls. I believe all RK rolls will always be skill checks.

The bonsues are labelled "In Lantern Light". This fits better, and no effects exist for the dim light, so bright is unnecessary and what the roll is for is already known.

The perception bonus is no longer set to hide if disabled. There's no way to automate detecting if non-creature targets are in the light.

Give all effects a sourceId that matches their id. The pf2e system does this with it's compendiums.

There are few things which don't work yet:

Allies don't get a +2 bonus when IV is used and the roll is against the EV target. Getting the effect to know about IV, and which creature is the EV target, is harder.

The -2 penalty for enemies when IV is used will still be off by default. Same problem getting the effect to know if IV is on.

There's no way to automate the invisible/concealed stuff.

If there is more than one thaumaturge, it's possible for an ally in one lantern's light to get a bonus against a target in another lantern's light. There's no way to tell the effects apart.

mysurvive commented 3 months ago

When an unsupported implement was managed, the logic broke in varying ways - typically removing the roll option from one of the implements and providing incorrect rank for the implement (makes sense). My last commit fixes that.

I am still reviewing this, sorry it's taking so long.

xyzzy42 commented 3 months ago

Because slug wasn't set for the unsupported implements? And then a bunch of stuff would break with that blank.

mysurvive commented 3 months ago

Correct.

Also, is there a reason you would prefer a numeric rank over a string rank? I don't mind either way but I'm thinking forward to homebrew implements and user experience. Strings might be more user friendly than numbers, and even moreso if it's similar to self:amulet:adept I can be convinced otherwise though. I'm actually not even sure the self portion makes sense, because there aren't target or origin roll options for implements, right? Or are there?

Edit - I am actually OK with a numeric rank. I'm finding more use cases for a numeric ranking system. I am still curious about the self portion of the roll option though

xyzzy42 commented 3 months ago

The string based ones are still there. I wanted the numeric value so I could use, "value":"@actor.attributes.implements.lantern.rank*10+10", in the AELike for the lantern radius, instead of having to write three different rules for it. It would also be possible to use it with brackets, if the value isn't simple math but it's still just the same rule with a different +x for each implement rank.

But I also think there are REs were using using rank:X will make the predicate simpler, like with gte, lte. etc.

As for self, that's how the system does those options, and they become target options too. E.g., I've got a catfolk thaum, he has self:trait:catfolk from the system when he attacks. When he gets attacked and is the target, then the system changes it to target:trait:catfolk. There is a bit of code in getSelfRollOptions() that replaces the self prefix with target or origin depending on the actor's function in the roll.

So there could be some rule that has "the target of your EV gets a -1 status penalty to attack you, if you are adept or better in amulet" and the RE could use {"gte": ["target:implement:amulet:rank", 2]} in the predicate of an effect that has been placed on the attacker by the EV.

mysurvive commented 3 months ago

I see, I am fine with this then, though the rule elements that are added to the feats should go away, as using the derived roll options actually solve a lot of problems that I was running into adding the roll options programmatically. I will work through getting all of the {rank}:{implement} roll options migrated to self:implement:{implement}:rank:{rank} roll options. I'm still reviewing the rest of the code to make sure nothing else breaks.

xyzzy42 commented 3 months ago

Yes, I think getting rid of the roll options added to the feats will make things simpler.

It might make sense to add derived roll options with names like self:implement:{implement}:adept too. I think depending on what the predicate needs to do, one or the other option would be the most useful to use.

mysurvive commented 3 months ago

Keeping the {implement}:{rank} schema for the string roll options for 2 reasons:

  1. There are already a lot of predicates that use it
  2. I don't see a good reason to pollute other actors' roll options lists with two roll options that effectively do the same thing. I think it's fine to have both on the origin though.