Skip to content

Defend against a crash in dynamically-created menus#16

Open
ceejbot wants to merge 1 commit intoExit-9B:mainfrom
ceejbot:ceej/empty-menu-arrays
Open

Defend against a crash in dynamically-created menus#16
ceejbot wants to merge 1 commit intoExit-9B:mainfrom
ceejbot:ceej/empty-menu-arrays

Conversation

@ceejbot
Copy link

@ceejbot ceejbot commented Oct 3, 2023

This bug showed up as a crash in MenuControl::GetShortText() in my mod.

If both the values and the short names arrays are empty, GetValue() returns empty string, which std::ranges::find() does not find in the empty short names array. The distance from array end to beginning is therefore 0, and the subsequent index into shortNames causes a crash.

Detect this case and return empty string.

I tripped over this because I have some dynamic menus that are sometimes empty. I've worked around the problem in my mod, but I thought you might want the fix.

If both the values and the short names arrays are empty,
`GetValue()` returns empty string, which  `std::ranges::find()`
does not find. The distance from array end to beginning is
therefore 0, and the subsequent index into the empty `shortNames`
causes a crash.

Detect this case and return empty string.

I tripped over this because I have some dynamic menus that are
sometimes empty. I've worked around the problem in my mod, but
I thought you might want the fix.
ceejbot added a commit to ceejbot/soulsy that referenced this pull request Oct 3, 2023
I believe I have proven it to my satisfaction. If it is not
a fix, I will do something drastic like eat an entire pint of
ice cream in one sitting. There is no ice cream in my house, so
this would obviously be traumatic.

The root bug is in how MCM-Helper handles empty dynamic menus,
and it's been around for a while. The workaround is to never
give MCM-Helper an empty array, but instead to show the string
"-- empty --" (or its translation) in the drop-down.

This is a PR with a fix for MCM-Helper:
Exit-9B/MCM-Helper#16
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.

1 participant