mysurvive / pf2e-thaum-vuln

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

Move effect creation into base Implement class #142

Closed xyzzy42 closed 3 months ago

xyzzy42 commented 3 months ago

The free function intensifyImplement() can just use the actor's attributes.implement objects that are already there. It doesn't need to make a new one.

Don't default to acting on game.user.character first, as the player can control more than one character. If they want to control a character other than main one, they'll never be able to select it.

Move some of the checks for IV ability, like held implement, into the main intensifyImplement() function. The individual implement methods weren't consistent on what they would check.

It's not necessary to specify the actor as the origin in an effect when the effect is applied to that same actor. That's the default. It's only needed when the effect is from someone else.

Change amulet IV effect predicate to use token mark from EV, this way the rules don't need to be edited.

Have base class IV method use a static class property with the Uuid of an effect to put on the actor. The derived classes will define it, if they have one. If one isn't defined, then it gives the unsupported implement error.

amulet, lantern, regalia, and weapon don't need intensifyImplement() methods anymore.

The base method can also take optional effect data, tome uses this, since it needs to modify the effect.

Could also check for an existing IV effect here and remove old ones or refuse to run (already done this turn).

mysurvive commented 3 months ago

Sorry for the multiple commits. I'm not sure what happened there.

The intensifyImplement function wasn't working for GMs (no actor was being passed, and they don't have a character, so it was erroring). I added a check in case a GM didn't have a token selected and added the actor to the macro argument list. It might behoove us to also have a check in place in case the GM accidentally has... some other actor other than a thaumaturge selected when they click it so it gracefully exits.

There are a couple more errors that I will look at after I get some sleep. Most notably with unsupported IV actions not having their name appear properly in the dialog.

image

mysurvive commented 3 months ago

Variable in a class can't be initialized and have a getter but no setter. Removing the initialization of the variable in the constructor allows us to just use the getter to get the name value.

I guess we could alternatively also make a setter for name, but I'm not sure which is actually more performant.

xyzzy42 commented 3 months ago

Looks like you merged in different branch from my repo, which was older, into this one, and that undid/combined some changes that shouldn't have gone together. I originally pushed db032, but then changed the commits 4 minutes later with 71d0d3b. But then db032 was merged into the branch.

xyzzy42 commented 3 months ago

Variable in a class can't be initialized and have a getter but no setter. Removing the initialization of the variable in the constructor allows us to just use the getter to get the name value.

I guess we could alternatively also make a setter for name, but I'm not sure which is actually more performant.

So there was in 71d0d3b just a getter for name and no local variable. But in db032 there was just a local variable. Merging the two branches added both at once.

mysurvive commented 3 months ago

I'm sorry, I'm really, really sick and haven't been sleeping at all. I guess I pulled the wrong branch locally and that, combined with my naivete on github created the perfect disaster. I think everything is cleaned up, granted with a sullied commit history.