Jamiras / RATools

Script interpreter for writing achievements for retroachievements.org
MIT License
48 stars 11 forks source link

correct IntegerConstantExpression comparison in DictionaryEntry Compare #487

Closed Biendeo closed 4 months ago

Biendeo commented 4 months ago

There's an edge case involving dictionaries which keys use a mixture of positive and negative integers. For example:

SONG_ABOUT_A_GIRL_UNPLUGGED          = 0x6b450446
SONG_AMERICAN_WOMAN                  = 0x49e39403
SONG_BAND_ON_THE_RUN                 = 0x57c4ea83
SONG_BEAT_IT                         = 0x69e63d11
SONG_BEAUTIFUL_DISASTER              = 0x75a03c4c
SONG_CRAZY_TRAIN                     = 0x1ac0a35e
SONG_DAMMIT                          = 0x131e5f8b
SONG_DO_IT_AGAIN                     = 0xcaac63ee
SONG_LOVE_REMOVAL_MACHINE            = 0xf991002d
SONG_THE_MIDDLE                      = 0x417b1aa8
SONG_MISERY_BUSINESS                 = 0x2d1f51f8

SONG_INFO = {
    SONG_ABOUT_A_GIRL_UNPLUGGED: "About A Girl",
    SONG_AMERICAN_WOMAN        : "American Woman"
    // etc...
}

for song in SONG_INFO {
    song_name = SONG_INFO[song]["Title"]
}

(a full working example can be found at Guitar Hero World Tour.rascript.txt)

This causes an error:

394:27: error: No entry in dictionary for key: 3400295406U
    song_name = SONG_INFO[song]["Title"]
                          ^~~~

This error disappears when any of the key values are updated, elements are removed from the list, or elements are re-ordered in the list (it is not affected by the values of the dictionary changing though).

This stems from the IntegerConstantExpression comparison between DictionaryEntrys comparing the difference between the values in ReplaceVariables() (as the keys are formerly variables). However, this doesn't correctly sort the list because some values are x - y > int.MaxValue, so in this case, SONG_DO_IT_AGAIN is sorted to the wrong end of the list, which means future BinarySearch()s fail for this entry.

The fix is to replace this with the int.CompareTo(), which correctly handles this case.