Achievement type safety #39777
Merged
Achievement type safety #39777
Conversation
This file contains 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
jbytheway
force-pushed
the
achievement_type_safety
branch
from
7d2b64d
to
1a9672c
Compare
For event_statistic, add a type() method. For event_transformation, add a fields() method. Consolidate the type of all these field lookup objects as std::unordered_map<std::string, cata_variant_type> (previously some places had used std::map).
We can expect this to happen when loading old saves, so we must expect it.
Detect various forms of error at verification time.
I didn't think this comment was sufficient to explain the mysterious "throw" here.
The optional deserialize overload wasn't propagating errors to its caller properly, leadning to some JSON issues not being reported.
jbytheway
force-pushed
the
achievement_type_safety
branch
from
1a9672c
to
579edb1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
SUMMARY: Infrastructure "Achievements have better up-front checking to detect definition errors"
Purpose of change
Make it easier to debug mistakes made in json when defining event stats, transformations, scores, and achievements.
Also, doing this checking up-front means we can more safely ignore issues that arise when processing events, deeming them to be due to changes in event definitions. See the discussion in #39717 for more details.
Describe the solution
Every
event_statisticnow exposes atype, and everyevent_transformationexposes afieldswhich defines the fields it has, together with their types.For this to be possible,
event_field_transformationhas also gained a type signature which can be used to derive the associated types.Use all these types to add a bunch more checks at "Verifying" time when loading game data.
Some other changes that happened which working on this:
units.hcata::optional, the code was previously dropping errors. Now it propagates them correctly.maps tounordered_maps.Describe alternatives you've considered
There's more up-front checking that could be done, but this is a good start.
Testing
Tweaking the currently existing definitions and observing that I get useful errors.