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

Capstone #41

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

rhagelstrom
Copy link
Contributor

Needs more testing but:

The other stuff was big change but this is the really big change.

The inline applied effect comes with a number of issues.

  • Can't set a duration which was at one point a feature request
  • Can't use the cyclers
  • Always applies to the bearer of the AURA
  • Makes the effect really long because it's really two effects
  • Since the effect is so long its a lot harder to understand for a lot of people
  • Needed conditionals for things like IF: FACTION(!self). More confusion for I think a lot of people

Obvious can't just get rid of the inline effect otherwise there would be pitchforks and torches, but if one isn't there we can fall back to an alternative method.

So the fallback is the method that BCE/G does it and you use as a descriptor as the label of the effect you want the aura to apply. The label is the first clause of the effect. It pulls these effects from the custom effects list.

Things to note about that.
When it does that, it's looking at the aOther bucket in rAuraDetails. aOther is the default bucket for when it doesn't fall into any other bucket. This is a big reason we need to keep everything nice and tidy.

FG is god awful dumb with searching lists of things so FG vanilla this is a sequential search. If we have a lot of things in aOther and a big custom effects list, then we might see performance issues.

Default for this search is if BCE/G is loaded then it just uses BCE/G binary search of the custom effects list which is lighting fast and efficient, so then a lot of things in aOther isn't really that big of a deal. But if you aren't running that or are doing PFRPG2, then you don't have that and are back to the sequential search.

Alternatively we could put BCE binary search code into aura and then this potential gotcha I think goes away.

If you do want to go this route of pulling from the custom effects list, I'd recommend then deprecating the inline effects because of the reasons I stated at the start and also it would allow better read of the code and less user confusion on how to use aura.

@rhagelstrom rhagelstrom requested review from mattekure and bmos June 10, 2024 00:47
@rhagelstrom rhagelstrom self-assigned this Jun 10, 2024
@rhagelstrom rhagelstrom marked this pull request as draft June 10, 2024 01:57
@mattekure
Copy link

One potential issue I see is that by making the effect defined only in the custom effects window, you eliminate the ability for players to create their own effects. Only the GM can create the custom effects and they are not shared with players unless they are defined in a module that the player loads. I know in the games I participate in, it would be disruptive to always have to ask the GM to add a new effect or tweak one mid game.

@rhagelstrom
Copy link
Contributor Author

One potential issue I see is that by making the effect defined only in the custom effects window, you eliminate the ability for players to create their own effects. Only the GM can create the custom effects and they are not shared with players unless they are defined in a module that the player loads. I know in the games I participate in, it would be disruptive to always have to ask the GM to add a new effect or tweak one mid game.

That is fair criticism. I’d make the case that in game, players or for even that matter GMs don’t create auras on the fly. Even creating effects on the fly is either a normal game disruption or is something that doesn’t normally happen. Anyhow does that case outweigh the potential benefits?

@rhagelstrom
Copy link
Contributor Author

One potential issue I see is that by making the effect defined only in the custom effects window, you eliminate the ability for players to create their own effects. Only the GM can create the custom effects and they are not shared with players unless they are defined in a module that the player loads. I know in the games I participate in, it would be disruptive to always have to ask the GM to add a new effect or tweak one mid game.

I think I better understand the concern. Aura is all server side now. So what it is looking for is done at the server and the gm effects should be available. Good test case

@MisterDDT
Copy link

This also creates a problem of the character sheet not having the info needed for the effect to work. We've done this in other cases but this would cause it to be like this always.
So if a player needed to use that effect on another table, even if they had AURA effects ext running, the effect would not work without the GM loading up the custom AURA effect in the custom effects list.

@rhagelstrom
Copy link
Contributor Author

This also creates a problem of the character sheet not having the info needed for the effect to work. We've done this in other cases but this would cause it to be like this always. So if a player needed to use that effect on another table, even if they had AURA effects ext running, the effect would not work without the GM loading up the custom AURA effect in the custom effects list.

I see the point on deficiencies but do those outweigh the benefits? And do the benefits outweigh the change to begin with? To addresses the traveling table, you'd have to have something built into the char. Which maybe looks like a custom effect for players which travel. That would be a standalone ext idea or just built into something. Maybe just keep this PR around until someone builds that?

…ce is dying or if not dying

Added descriptors and checks for applied effects for CreatureTypes, CreatureSize, Alignment
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.

3 participants