luau / UniversalSynSaveInstance

Shortly USSI. A universal saveinstance revival. This can also be edited and used as a Roblox-Format-File writer.
https://luau.github.io/UniversalSynSaveInstance/
Other
84 stars 32 forks source link

Solutions for improperly formatted XML #28

Closed centerepic closed 3 months ago

centerepic commented 3 months ago

I know you're already aware of this, but do you have any ideas for what the causes for this might be? It seems like no matter how many compatibility options I toggle (except just completely disabling all special properties) places are corrupted all the time, even with safe mode. Is there any easy way to find the segment that's not formatted properly?

(Even after disabling special properties I pretty often get corrupted files, after testing a few times it doesn't seem to be an issue with saving as the file sizes match up perfectly.)

(This has been with testing with Celery, Solara doesn't really support this well since their file APIs are incredibly slow and yield forever.) Only guess I would have is appendfile in rapid series may cause race conditions or fail randomly, but if it's written properly this should virtually never happen.

phoriah commented 3 months ago

Appendfile causes uneven amount of tags (there must always be an opening tag and closing tag). Virtually, this shouldn't happen because implementation is indeed correct as far as it goes. Yet it does. However, writefile works fine. I've mentioned this in your Pull Request.

phoriah commented 3 months ago

@centerepic How often does that happen?

centerepic commented 3 months ago

Happens pretty much every time, regardless of what options I use. Not sure what's causing it, honestly. (as of the last build before fixes were reverted)

centerepic commented 3 months ago

@phoriah has anyone reported these issues on other executors? trying to dig to the root cause so I can try making a definitive patch for celery on my own fork

phoriah commented 3 months ago

@phoriah has anyone reported these issues on other executors? trying to dig to the root cause so I can try making a definitive patch for celery on my own fork

I've given you the most likely root cause of it literally in my first reply here.

phoriah commented 3 months ago

@phoriah has anyone reported these issues on other executors? trying to dig to the root cause so I can try making a definitive patch for celery on my own fork

The answer is no. There have been 0 reports about files not opening ever since saveinstance started using writefile exclusively.

centerepic commented 3 months ago

I fixed it, it was an issue with string.sub in the segmented appendfile version for celery causing a 1 character overlap 😭 Works perfectly for me now. If I open a pull request with a definitive fix for celery, will you accept it?

centerepic commented 3 months ago

Why are part colors/sizes/etc. treated as special properties? None get preserved in the celery patched version. image

phoriah commented 3 months ago

I fixed it, it was an issue with string.sub in the segmented appendfile version for celery causing a 1 character overlap 😭 Works perfectly for me now. If I open a pull request with a definitive fix for celery, will you accept it?

Sure.

phoriah commented 3 months ago

Why are part colors/sizes/etc. treated as special properties? None get preserved in the celery patched version. image

Solara deals with those just fine despite not having gethiddenproperty either. We don't decide what is saved or not, saveinstance strictly follows Studios & Roblox's API. Only properties with CanSave & CanLoad set to true will be saved.

centerepic commented 3 months ago

Alright, I've partially fixed the issue with part sizes, colors still don't get saved... trying to figure out why it improperly ignores certain properties even though they're not hidden, I think I might also be able to push a fix for MeshParts being the wrong size, will probably end up being a temporary patch for exploits that don't support gethiddenproperty.

phoriah commented 3 months ago

It doesn't just save any property, regardless whether it's hidden or not. Like I said - CanSave & CanLoad must be true in the Property's data in the API Dump in order for it to be saved. That's how Studio determines what to save too. Example: BasePart Colors are saved as Color3uint8 but that property is hidden and almost no executor can read it hence why there is a NotScriptableFix for it.

centerepic commented 3 months ago

Figured out a patch for meshes being improperly sized on exploits without gethiddenproperty. From my experience it seems to restore them 100% perfectly to what they were originally.

Before

image

After

image

centerepic commented 3 months ago

Ok, the original issue has been fixed, I'm going to open another issue for properties not being saved that should be savable.