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

Add a method for prioritizing modify triggers. #165

Open
wants to merge 3 commits into
base: beta
Choose a base branch
from

Conversation

pyrrhicPachyderm
Copy link
Contributor

For some of the triggers we've added recently, it can matter which order they're called in, but currently that order is undefined, and indeed unstable. This is something that concerned me when I was writing them, but I didn't have a good solution at the time.

The specific case that's prompted me to write this fix is the scripting Emily's power card rework. She was asking this about writing a script that would make it so that, by default, you draft three cards instead of four. Exactly the sort of thing that modifyCardGain() is good for: it should be a simple "if you'd draw four, instead draw three". Except that there's a possibility of conflict with Mentor: specifically if Mentor receives Boon of Reimagining, then if matters which trigger runs first. Running Emily's, then Mentor's results in four cards, but running Mentor's then Emily's results in three. And this order isn't stable. The problem would be even worse if it was modifying up to five, instead of down to three: then it would matter which triggered first even on default drafts (one order would yield two cards, the other three).

Currently, I've added prioritisation only for the modify triggers (modifyGain, modifyCost, modifyCardGain, modifyElements and modifyThresholds). In theory, prioritisation could be relevant for other triggers, e.g. the time passes trigger. But I suspect that anything on those triggers that requires prioritisation will also require wait calls between successive runs, and that seems far too complicated to be getting into until there's an actual use case.

The numbers used are a rough aim at something that might be
appropriate, but as there are no priority conflicts extant in the
mod, and priority numbers need not be consecutive, and can even be
fractional or negative, they're really extremely arbitrary.
@pyrrhicPachyderm
Copy link
Contributor Author

It also occurs to me that, should this be added, Immense (and Spreading Hostility) could be rewritten to use this system (simply with very high priority) instead of the current way they're done. Don't know if they'd actually be better done that way, but it'd avoid the need for their onDestroy().

@@ -2062,7 +2073,7 @@ function MinorPowerUI(player, button)
startDealPowerCards({player = player, major = false, count = cards})
end
function modifyCardGain(params)
for _,obj in pairs(getObjectsWithTag("Modify Card Gain")) do
for _,obj in ipairs(getTriggers("Modify Card Gain", "modifyCardGainPriority")) do
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm assuming making this ipairs is having a different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I don't believe pairs() places any guarantee on order of iteration, while ipairs() does.

@@ -8,6 +8,7 @@ function Broadcast(params)
return "Blitz - Remember, Invaders get an additional set of Actions at the end of Setup"
end

modifyCostPriority = 10
Copy link
Contributor

Choose a reason for hiding this comment

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

we may want to add an extra 0 to these just to allow for more room in between things

Copy link
Contributor

Choose a reason for hiding this comment

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

i dont envision us needing that many, but kind of hard to go back later and change things here since it would break custom content

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Priorities don't have to be integers, or even positive, so until we hit floating point precision limits, there'll always be more room to slip things in. If it weren't for that, I'd be slightly uncomfortable relying on this system at all. I can still make them bigger if you want. I also had half a mind to mention that negatives/fractions are permitted in the documentation; I can toss that in as well if you'd like.

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.

2 participants