Skip to content

Fix: Prevent patching custom models#3367

Merged
briaguya0 merged 4 commits intoHarbourMasters:develop-macreadyfrom
Archez:prevent-patching-custom
Nov 14, 2023
Merged

Fix: Prevent patching custom models#3367
briaguya0 merged 4 commits intoHarbourMasters:develop-macreadyfrom
Archez:prevent-patching-custom

Conversation

@Archez
Copy link
Contributor

@Archez Archez commented Nov 8, 2023

Our graphics patching for DList are very explicitly tied to the authentic DList from the game. Attempting to patch custom models under the same path name will either lead to undefined behavior or straight up crash entirely. This could leave people stuck in a crash loop until they clear their ship.json or mods folder.

Turns out, the resource class has an IsCustom field! This PR leverages the IsCustom field from the resource before patching and if it's custom, return without patching.

There is no visual feedback in the cosmetic editor currently. Wasn't sure if we should do something about that. Some cosmetic affects patch multiple DLists, or also include in-code color changing, so we can't just disable them entirely if one of the DList is custom but the other isn't. From a "prevent crashes" point of view, I think this should resolve like 99% of the issues.

If we ever feel like we want to support patching custom assets at a later time, we would need to revisit this check, but for now I really don't see a need to support patching custom assets.

Closes #2883

Build Artifacts

@Archez
Copy link
Contributor Author

Archez commented Nov 13, 2023

Updated to also not patch chest textures when a custom chest model is provided. This is necessary since our chest texture doesn't use ResourceMgr_PatchGfxByName and instead does an in memory patch within the chest actor.

I left a TODO for us to explore a better way to handle CTMC for model packs (e.g. have packs provide dedicated DLs per chest type and use those directly instead of patching).

if (hasCreatedRandoChestTextures) return;
if (hasCreatedRandoChestTextures || hasCustomChestDLs) return;

// Don't patch textures for custom chest models, as they do not import textures the exact same way as vanilla chests
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this handle the case of the custom models being toggle-able by alt? I don't think so if I'm understanding this correctly, or maybe it's a non-issue for another reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah good catch, probably not since its a one time look up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok rearranged some things so it checks for the model being custom on every update instead of a one-time check.
Not a fan of having to check every update, but its fine for now.

@briaguya0 briaguya0 merged commit e66eb87 into HarbourMasters:develop-macready Nov 14, 2023
@Archez Archez deleted the prevent-patching-custom branch November 20, 2023 07:29
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.

3 participants