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

Enhance persistence plugin #464

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

Be1zebub
Copy link
Contributor

@Be1zebub Be1zebub commented Dec 21, 2024

Any reason why this plugin exists? Except logs & custom path.
I suggest deleting it, or at least updating it to match the native behavior and save more stuff - eg dt.

  1. rm bicycle code, just use duplicator lib (its saves dt & other helpful things) - make it better & native bahiavor
  2. use native methods Get/SetPersistent - for better compatibility with addons
  3. disable sbox persist hooks if plugin enabled - we make custom behaivor, default persist system should be disabled

Any reason why this plugin exists? Except logs.
I suggest deleting it, or at least updating it to match the naive behavior and save more stuff - eg dt.

1. rm bicycle code, just use duplicator lib (its saves dt & other helpful things) - make it better & native bahiavor
2. use native methods Get/SetPersistent - for better compatibility with addons
3. disable sbox persist hooks if plugin enabled - we make custom behaivor, default persist system should be disabled
rm unused code
@alexgrist
Copy link
Member

alexgrist commented Dec 21, 2024

The primary reason is the monumental lag that occurs from the regular saving of the entities as well as many other inefficiencies of the vanilla persistence. Using the duplicator library is the primary cause of the issue which is why we have our own implementation.

Originally we were using the sandbox persistence but unfortunately discovered the major pitfalls of it in our live testing, it will become very laggy over time as more and more entities are required to be saved.

@Be1zebub
Copy link
Contributor Author

Then maybe lets use just a few non laggy things like duplicator.CopyEntTable?
Im sure things like duplicator.Patse which do a lot of money work are cause lags.

@alexgrist
Copy link
Member

Ideally yes but you would need to test with a server that has lots of entities saved over a decent period of time to ensure those also aren't causes to the issue.

@Be1zebub
Copy link
Contributor Author

Be1zebub commented Dec 21, 2024

Okey maybe lets just add more stuff like dt to original implementation?
The reason why i do this - helix doesnt save alot stuff.

@alexgrist
Copy link
Member

Yeah that's probably a more reliable route.

@Be1zebub
Copy link
Contributor Author

Sounds good.
What do you think about these changes?

  1. use native methods Get/SetPersistent - for better compatibility with addons
  2. disable sbox persist hooks if plugin enabled - we make custom behaivor, default persist system should be disabled

@alexgrist
Copy link
Member

I think we avoided using the default persistence Get/Set due to other addons actually having issues because the implementations of persistence was different. Unsure about the other.

add more properties save to default implementation
add Entity:OnHelixPersistLoad/OnHelixPersistSave
@Be1zebub Be1zebub changed the title Fix persistence plugin Enhance persistence plugin Dec 21, 2024
add data arg to OnHelixPersistLoad
make custom data loading possible
@Be1zebub
Copy link
Contributor Author

PTAL

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.

2 participants