Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Multiple Fixes for the ModPrefabCache #469

Merged
merged 5 commits into from
Sep 26, 2023
Merged

Multiple Fixes for the ModPrefabCache #469

merged 5 commits into from
Sep 26, 2023

Conversation

MrPurple6411
Copy link
Member

@MrPurple6411 MrPurple6411 commented Sep 25, 2023 •
edited

Changes made in this pull request

  • Added a ModPrefabRequest Cache to call upon so that it is not making multiple requests anymore.
  • Null Check ProcessPrefabAsync
  • Add prefab name to error log when missing PrefabIdentifier

Breaking changes

  • Should be none.

@MrPurple6411
Multiple Fixes for the ModPrefabCache
60e7104 
1. Set cached prefabs as inactive.
2. Set check before caching prefab as its all async and may have been registered moments before.
3. Null Check ProcessPrefabAsync
4. Add prefab name to error log when missing PrefabIdentifier
@MrPurple6411 MrPurple6411 linked an issue Sep 25, 2023 that may be closed by this pull request
Issue with duplicate cache items #394
Closed
@MrPurple6411 MrPurple6411 self-assigned this Sep 25, 2023
@LeeTwentyThree
Copy link
Member

This seems fine for now, should be ready to merge, but it would probably be best to make a system that uses existing prefab requests for “work in progress” prefabs rather than running multiple requests at the same time.

Metious
Metious reviewed Sep 26, 2023
View reviewed changes
Copy link
Member

@Metious Metious left a comment

Choose a reason for hiding this comment

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

Added a few comments

Nautilus/Assets/ModPrefabCache.cs Outdated Show resolved Hide resolved
Nautilus/Handlers/PrefabHandler.cs Outdated Show resolved Hide resolved
@MrPurple6411
Fix duplicate Cache at request level instead of just trying to destro…
80f9c45 
…y the duplicates.

Created a Dictionary<string, ModPrefabRequest> called Requests in tht ModPrefabCache.
Made GetModPrefabAsync return the cached request instead of making a new one.
Made ModPrefabRequest add itself to the ModPrefabCache.Requests when created.

Retargeted InstantiateAsync and SpawnAsyncReplacement to use GetModPrefabAsync isntead of bypassing the request system.
Metious
Metious approved these changes Sep 26, 2023
View reviewed changes
Copy link
Member

@Metious Metious left a comment

Choose a reason for hiding this comment

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

lgtm

@MrPurple6411 MrPurple6411 merged commit 4330e5e into SubnauticaModding:master Sep 26, 2023
1 check passed
@MrPurple6411 MrPurple6411 deleted the ModPrefabCacheFixes branch September 26, 2023 21:04
@github-actions github-actions bot locked and limited conversation to collaborators Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Metious Metious Metious approved these changes

@LeeTwentyThree LeeTwentyThree Awaiting requested review from LeeTwentyThree

Assignees

@MrPurple6411 MrPurple6411

Labels
Type: Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Issue with duplicate cache items
3 participants
@MrPurple6411 @LeeTwentyThree @Metious