Skip to content

Comments

Fix options menu memory access violation on localization change#368

Draft
ProfLander wants to merge 2 commits intothemrdemonized:all-in-one-vs2022-wpofrom
Lander-Modding:localization-mav-fix
Draft

Fix options menu memory access violation on localization change#368
ProfLander wants to merge 2 commits intothemrdemonized:all-in-one-vs2022-wpofrom
Lander-Modding:localization-mav-fix

Conversation

@ProfLander
Copy link
Contributor

Adds a monkey-patch for ui_options.script to prevent it from accessing a deleted self.owner after localization is reset.

Closes #367.

@ProfLander ProfLander force-pushed the localization-mav-fix branch 2 times, most recently from 2f05d8d to 8d39c0f Compare September 11, 2025 06:53
Copy link

@ravenascendant ravenascendant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

patch timing is wrong. option table navigation is unnecessary if the functor is patched before the table is built.

@themrdemonized
Copy link
Owner

patch timing is wrong. option table navigation is unnecessary if the functor is patched before the table is built.

What you think if we put this change into ui_options.script itself and add this into the next exes version?

@ravenascendant
Copy link

patch timing is wrong. option table navigation is unnecessary if the functor is patched before the table is built.

What you think if we put this change into ui_options.script itself and add this into the next exes version?

Are any of the pre MCM mods that replaced UI options still a thing? warfare overhaul? dynamic mutants?

other than the risk of being over written, which is probably small (?) moving all your options changes to a script replacement does make sense.

@ProfLander ProfLander marked this pull request as draft September 11, 2025 18:08
@ProfLander
Copy link
Contributor Author

ProfLander commented Sep 11, 2025

Adopting ui_options into modded exes sounds good to me; I've drafted the PR while the question is open.

@ravenascendant How extensive are MCM's changes to ui_options? If xray-monolith is going to adopt the script as first-class gamedata, it would be preferable for that version to be the single source of truth, with downstream additions coming in via monkey patch.

We could break apart larger functions, or add new patch entrypoints within them if the current structure is too monolithic for that to be tractable as-is. Though I suppose that would raise the question of compatibility with vanilla 🤔

@ravenascendant
Copy link

Adopting ui_options into modded exes sounds good to me; I've drafted the PR while the question is open.

@ravenascendant How extensive are MCM's changes to ui_options? If xray-monolith is going to adopt the script as first-class gamedata, it would be preferable for that version to be the single source of truth, with downstream additions coming in via monkey patch.

We could break apart larger functions, or add new patch entrypoints within them if the current structure is too monolithic for that to be tractable as-is. Though I suppose that would raise the question of compatibility with vanilla 🤔

MCM doesn't use anything in ui_options.script

ui_mcm.script started as a lazy copy and rename of ui_options.script with a dynamic options table loader tacked on. I have since expanded upon it in many ways. "lazy" code duplication saves me from being affected by changes to ui_options.script
However bugs involving ui_options are likely to behave similarly with MCM because the structure and a majority of the code is still the same.

it uses some of the same textures and texture definitions. and some translation strings. eventually i will duplicate these into MCM since i know that Tronex made a new options UI for anomaly 1.6 and i don't want to be dependant on file that could be deleted or changed becaue of that.

if ui_options does become part of modded exe's i will submit a PR for the ui_options bugs that i have fixed in MCM. maybe open an issue for the ones i can't figure out how to fix. ;P

@ProfLander
Copy link
Contributor Author

ProfLander commented Sep 11, 2025

MCM doesn't use anything in ui_options.script

Gah, sorry; I got ui_main_menu.script mixed up with ui_options.script. Ran into an MO2 conflict with it in the early stages of debugging this PR, and the wrong one stuck in my head. No worries on that front then.

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.

Hidden Error: Memory access violation in options menu during localization reload

3 participants