No Magic Numbers for Preset Location Exclusions #3801
Conversation
…r feeding `RandomizerCheck` values directly in presets instead of a string with magic numbers.
| @@ -866,7 +872,8 @@ const std::vector<PresetEntry> spockRacePresetEntries = { | |||
| PRESET_ENTRY_S32("gRandomizeDampeHint", 1), | |||
| PRESET_ENTRY_S32("gRandomizeDoorOfTime", RO_DOOROFTIME_OPEN), | |||
| PRESET_ENTRY_S32("gRandomizeEnableBombchuDrops", 1), | |||
| PRESET_ENTRY_STRING("gRandomizeExcludedLocations", "78,143,144,229,"), | |||
| PRESET_ENTRY_CPP_STRING("gRandomizeExcludedLocations", FormatLocations( | |||
There was a problem hiding this comment.
Why not just use FormatLocations inside applyPreset? Having a type like ENUM_LIST_TO_STRING where the value is a vector of ints, and then you can just string build in applyPreset directly, rather than in the header.
There was a problem hiding this comment.
Doing it here means the std::string is stored here in the preset entries vector, so the .c_str() call will return a pointer that will stay valid. If we call format string in applyPreset, the pointer returned from the .c_str() in that function will point to a local std::string that gets freed at the end of the function, potentially resulting in a use after free. Unless the string gets copied as part of CVarSetString perhaps? Which means it would then be fine to do that.
garrettjoecox
merged commit 61cf2bd
into
HarbourMasters:develop-macready
Previously, a change to the
RandomizerCheckenum caused all location exclusions in the presets to be off by 1 because they were feeding literal strings. This addsFormatLocationsandPRESET_ENTRY_TYPE_CPP_STRINGto allow for feedingRandomizerCheckvalues directly in presets instead of a string with magic numbers to prevent this in the future.Build Artifacts