Fix sacrifice recipe qsort comparator - #5127
Draft
NeptixFTW wants to merge 1 commit into
Draft
Conversation
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
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
Fixes #4703 by replacing the sacrifice-recipe
qsortcomparator with one that satisfies the C comparator contract and keeps unused zero victim slots after valid creature models.Root cause
qsortrequires its comparator to return a negative value when the first element sorts before the second, zero for equal elements, and a positive value when it sorts after the second. The previous comparator returned the boolean expressiona < b, so it returned only0or1: unequal values could be reported as equal, and ascending values could be reported in the wrong direction.The resulting behavior is runtime-dependent because different C library
qsortimplementations compare and partition elements differently. The Amiga runtime exposes the shippedCHEAPER_IMPSfailure and logsFound unsupported CHEAPER_IMPS sacrifice. The Windows runtime used for testing masks that particular failure: unpatched master happened to retain the shipped one-IMP recipe as[IMP, 0, ...], so the warning was not emitted and the price effect still worked, despite the comparator being invalid.Validation
zero/imp=1,imp/zero=0,lower/higher=1,higher/lower=0, and the test failed with exit code-1.1,-1,-1,1, and0for equality), parsed theCHEAPER_IMPSrecipe as IMP model 23 followed by zero slots, increasedcheaper_diggersfrom 0 to 1, and reduced the create-imp price from 600 to 450. The test passed with exit code0.The temporary functional-test harness and development game copy are not included in this change.