Skip to content

Fix sacrifice recipe qsort comparator - #5127

Draft
NeptixFTW wants to merge 1 commit into
dkfans:masterfrom
NeptixFTW:fix-4703-sacrifice-sort-comparator
Draft

Fix sacrifice recipe qsort comparator#5127
NeptixFTW wants to merge 1 commit into
dkfans:masterfrom
NeptixFTW:fix-4703-sacrifice-sort-comparator

Conversation

@NeptixFTW

Copy link
Copy Markdown

Summary

Fixes #4703 by replacing the sacrifice-recipe qsort comparator with one that satisfies the C comparator contract and keeps unused zero victim slots after valid creature models.

Root cause

qsort requires 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 expression a < b, so it returned only 0 or 1: 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 qsort implementations compare and partition elements differently. The Amiga runtime exposes the shipped CHEAPER_IMPS failure and logs Found 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

  • Reproduced the invalid comparator on exact unpatched master with a temporary KeeperFX functional test. It returned zero/imp=1, imp/zero=0, lower/higher=1, higher/lower=0, and the test failed with exit code -1.
  • Ran the same functional test with this fix. It returned valid comparator signs (1, -1, -1, 1, and 0 for equality), parsed the CHEAPER_IMPS recipe as IMP model 23 followed by zero slots, increased cheaper_diggers from 0 to 1, and reduced the create-imp price from 600 to 450. The test passed with exit code 0.
  • Built the fixed executable with the repository-supported i686 MinGW CMake configuration.
  • Launched the fixed executable successfully, reached the main menu, and loaded original Dungeon Keeper campaign level 1.

The temporary functional-test harness and development game copy are not included in this change.

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.

config_rules.c qsort issue (bad comparator)

1 participant