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

Fix force_family_event edge cases #155

Merged
merged 1 commit into from Dec 28, 2023

Conversation

SChinchi
Copy link
Contributor

  1. Multiple command calls shouldn't stack the hooks.
  2. If a stage has no family dccs, e.g., limbo, the dccs-hook returns an empty selection which causes an error for Evaluate(). It should instead return the original selection.
  3. The dccs- and nondccs-hooks can become desynced. R2API.Director ensures that all stages use the new api, but if someone sets the dccs pool to null, the nondccs hook will be consumed on the next stage, but the dccs on the stage after that. Both hooks/patches should be applied and unapplied from the same method to avoid this.
  4. The nondccs-hook is insufficient to ensure family events with the old api. On top of ensuring the family event chance is 100%, it also requires the family categories to lift their stage restrictions.

The only loophole left is a dccs pool existing but not yielding any family events, e.g., limbo with the default R2API pool and a modder assigning the family events with the old api, or someone removing the family events from, for example, golemplains. In that case the command won't fall back to finding a family from the old api. I think this is fine because as long as a pool exists, it should only use that. And if someone intentionally removes the family events and wants to test the result, they will be very surprised if an event still occurs due to a fallback mechanism.

@xiaoxiao921 xiaoxiao921 merged commit a3fc610 into harbingerofme:master Dec 28, 2023
@SChinchi SChinchi deleted the fix_family_event branch December 28, 2023 23:26
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.

None yet

2 participants