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

new shit #6

Merged
merged 25 commits into from
May 11, 2021
Merged

new shit #6

merged 25 commits into from
May 11, 2021

Conversation

emptyrivers
Copy link
Owner

I think this counts as a major version bump

@emptyrivers
Copy link
Owner Author

@Meorawr since you've had some experience playing with this before, would you mind giving this a spin? I didn't find any unexpected breakages but I'm famously bad at regression testing.

@emptyrivers
Copy link
Owner Author

Major changes are:

  • adopted LibSerialize, and included an automatic migration from the homegrown encoding to something that LibSerialize understands
  • Archivist now supports management of multiple archives that can exist independently of each other in saved variables:
-- assume AddonA, AddonB are both savedvariables that archivist has interacted with before
local Archivist = select(2, ...).Archivist
local first = Archivist(AddonA, {-[[table of store types to register in first archive]]})
local second = Archivist(AddonB)

first:Load("RawData", "jerry's data")

second:RegisterStoreType({-[[snip]]})
-- etc etc, first & second operate totally independently of each other as far as consuming code is concerned

Archivist.lua Outdated Show resolved Hide resolved
Archivist.lua Outdated Show resolved Hide resolved
Archivist.lua Outdated Show resolved Hide resolved
Archivist.lua Outdated Show resolved Hide resolved
}, {
__index = proto
})
if type(sv.internalVersion) ~= "number" or sv.internalVersion < self.internalVersion then
Copy link
Contributor

Choose a reason for hiding this comment

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

I've got a slight concern here that doing migrations upon initialization of the instance may cause script timeout errors for people with gargantuan amounts of data, considering that this first migration will be touching every single store to decompress, deserialize, re-serialize, and re-compress things.

Even if it isn't a significant issue now, any future migrations which do similar things would potentially have a stacking effect in the case where multiple migrations need to be performed.

It might be better to apply this migration on a store-by-store basis at the point when an individual store is opened by checking the internal versioning at a store level to see which deserializer should be used to initially read it.

Copy link
Owner Author

Choose a reason for hiding this comment

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

This is a good point. I think I'll work on this some more in a follow-on change, once rossnichols/LibSerialize#4 & SafeteeWoW/LibDeflate#6 are complete.

& interpret "unversioned" SV as version 0
also tweak an error message to be more informative
@emptyrivers
Copy link
Owner Author

Thanks @Meorawr & @InfusOnWoW, I appreciate the extra eyes ❤️

@emptyrivers emptyrivers merged commit c4f21a0 into master May 11, 2021
@emptyrivers emptyrivers deleted the new_shit branch May 11, 2021 19:42
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