Fix template compatibility checks to honor custom AbilityRandomizer (Ghost, Half-Celestial, Half-Fiend) - #69
Open
karl-speer-GGL wants to merge 19 commits into
Open
Fix template compatibility checks to honor custom AbilityRandomizer (Ghost, Half-Celestial, Half-Fiend)#69karl-speer-GGL wants to merge 19 commits into
karl-speer-GGL wants to merge 19 commits into
Conversation
Nuget package updates Going through the new AbilityRandomizer inputs for template applicators Adding new test cases as needed QOL refactors
karl-speer-GGL
changed the title
Gonna try a big template applicator validation logic shift to set theory, outlined in comments in the GhostApplicator. Also adding some self-validation to the AbilityRandomizer (helping ensure our max roll is positive, which is required for abilities) - need to actually CALL the method, but I'll get to that.
Once I get the table tests passing, I can run the profiler against the Ghost applicator and see if we improve over the old ability adjustment selection
Might wanna move the "GetCompatiblePrototypes" (from name) and nix it altogether. If someone wants the names, they cal lthe GetCompatibleCreatures method. If they want those in a prototype, then they 1. Get the names 2. Build the prototypes themselves with the factory 3. Pass that into the applicator and say ApplyToPrototypes Then the applicators wouldn't need to reference the factory at all, and it just becomes "do I want to check compatibility against prototypes" (do it live) or "do I want to check compatibility against creature names" (hit table cache). Extra step for the caller, but drastically simplifies the templateapplicators...
Now to fix over 900 compilation errors! But the non-duplicated verification logic living in the creature verifier has made the template applicators much cleaner and leaner, and centralizes the chaining logic nicely as well.
Got all the unit tests updated, pattern looks pretty good. Fiendish creature next.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes template validity checks so they correctly account for a caller-provided
AbilityRandomizerwhen evaluating minimum ability requirements.The issue affected templates with minimum ability gates:
• Ghost (minimum Charisma)
• Half-Celestial (minimum Intelligence)
• Half-Fiend (minimum Intelligence)
Root Cause
Compatibility checks for random generation could evaluate template eligibility using default ability-roll assumptions instead of the active
AbilityRandomizerconfiguration (custom roll or set rolls). This caused incorrect valid/invalid outcomes and intermittent stress-test failures.What Changed
• Updated compatibility filtering in:
•
GhostApplicator.GetCompatibleCreatures(...)•
HalfCelestialApplicator.GetCompatibleCreatures(...)•
HalfFiendApplicator.GetCompatibleCreatures(...)• Minimum-ability validation now uses
abilityRandomizer.Validate(...)with the active randomizer configuration and relevant ability adjustments.Tests
Expanded unit coverage to validate compatibility behavior across randomizer modes:
• default randomizer behavior
• custom
Roll• custom
SetRolls• high-roll scenarios that should allow otherwise borderline creatures
Updated test suites include:
•
GhostApplicatorTests•
HalfCelestialApplicatorTests•
HalfFiendApplicatorTestsAdditional
• Updated
README.mdbuild status badge branch reference from master to main.