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.

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

Hotreload save #1635

Merged
merged 10 commits into from
Oct 3, 2022
Merged

Hotreload save #1635

merged 10 commits into from
Oct 3, 2022

Conversation

kianzarrin
Copy link
Collaborator

@kianzarrin kianzarrin commented Aug 18, 2022

  • hot reloading saved data can fail because binary serializer tries to get data types from the wrong TMPE assembly.
    This PR applies a patch that forces all data types to use latest TMPE assembly.
  • Also if LOM is active asset data is hotreloaded.

First commit: introduce Preload Patch phase
Second commit: hot reload

EDIT: summary from discord talk:

  • the patch here is only applied when hotreloading
  • the patch only makes a difference when data was saved with a version of TMPE that resides in App Domain.
  • I don't see any drawback to the patch except for the time it takes to process string (which I suspect is insignificant).
  • LOM is required for asset data to persist because LOM stores asset custom data when loading. after load asset custom data is lost outside of LOM.

@DaEgi01 DaEgi01 marked this pull request as ready for review August 20, 2022 11:04
@krzychu124
Copy link
Member

This feature shouldn't be available on public/release version

@kianzarrin
Copy link
Collaborator Author

This feature shouldn't be available on public/release version

@krzychu124 its only available when hot-reloading. otherwise the code is not executed.

@krzychu124 krzychu124 added this to the 11.7.1 milestone Sep 14, 2022
@krzychu124
Copy link
Member

This feature shouldn't be available on public/release version

@krzychu124 its only available when hot-reloading. otherwise the code is not executed.

I understand, but I don't think normal users should be able to use hot-reloading feature. We have features like ExtCitizen data holding ParkingAI state which will desync and cause flying citizens all over the map. It's very minor issue if you develop or test things but very annoying when you actually play normally.

One more thing, I see that asset data reload will work only if LOM is available, right?

@kianzarrin
Copy link
Collaborator Author

kianzarrin commented Sep 15, 2022

I don't think normal users should be able to use hot-reloading

OK I make it DEBUG only. CS should probably turn off file observers too. but that can be done with LOM patch as well.

I see that asset data reload will work only if LOM is available, right?

Yes. LOM holds the data that needs to be transferred.

@krzychu124 krzychu124 self-requested a review September 21, 2022 22:24
@krzychu124
Copy link
Member

Q: What will happen if I hot-reload different mod after hot-reloading TM:PE (patch is still active)?

Copy link
Contributor

@chameleon-tbn chameleon-tbn left a comment

Choose a reason for hiding this comment

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

Lgtm

@krzychu124 krzychu124 mentioned this pull request Oct 2, 2022
1 task
@kianzarrin kianzarrin merged commit aca88f5 into master Oct 3, 2022
@kianzarrin kianzarrin deleted the hotreload-save branch October 3, 2022 23:35
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