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

[KKS][MainGameOptimizations] Avoid error when saving more than 2 GB in the main game #63

Closed
wants to merge 1 commit into from

Conversation

takahiro0327
Copy link
Contributor

@takahiro0327 takahiro0327 commented Apr 8, 2024

When saving saved data in the main game, there was a problem with not being able to save the data when there were many characters and the size exceeded 2 GB. If it looks good, merge it.

If the maximum number of characters is set and the number of costumes is increased continuously, 2 GB is reached.
Fixed to bypass the process via MemoryStream and write directly to the file.

This problem only occurs with KKS.
KK does not go through MemoryStream.

This modification was made at the request of a commission from BitMagnet.

Copy link
Collaborator

@ManlyMarco ManlyMarco left a comment

Choose a reason for hiding this comment

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

This seems fairly fragile, wouldn't it make more sense to fix the mentioned hook inside ExtendedSave instead? When I wrote it I assumed that save data would never be larger than a few hundred MB at worst so it is not optimized at all.
The proper fix would be to patch the save lambda with a transplier and write extended data there, like it's done for character data.

@takahiro0327
Copy link
Contributor Author

This seems fairly fragile, wouldn't it make more sense to fix the mentioned hook inside ExtendedSave instead?

Is this?
https://github.com/IllusionMods/BepisPlugins/blob/abee07b2392e9d7db88a7f80e68230cf9a7268a8/src/KKS_ExtensibleSaveFormat/KKS.ExtendedSave.SaveData.Hooks.cs#L61

I get an exception on MemoryStream in GetBytes, I can't call GetBytes.

IOException: Stream was too long.
  at System.IO.MemoryStream.Write (System.Byte[] buffer, System.Int32 offset, System.Int32 count) [0x0008c] in <fb001e01371b4adca20013e0ac763896>:0 
  at System.IO.BinaryWriter.Write (System.Byte[] buffer) [0x0000e] in <fb001e01371b4adca20013e0ac763896>:0 
  at (wrapper dynamic-method) SaveData.WorldData.DMD<SaveData.WorldData::GetBytes>(SaveData.WorldData)
  at SaveData.WorldData.<Save>b__140_0 (System.IO.FileStream f) [0x00007] in <6dc6faa1c0ae40368e38f6b0de78ff9c>:0 
  at Illusion.Utils+File.OpenWrite (System.String filePath, System.Boolean isAppend, System.Action`1[T] act) [0x0000b] in <6dc6faa1c0ae40368e38f6b0de78ff9c>:0 
  at (wrapper dynamic-method) SaveData.WorldData.DMD<SaveData.WorldData::Save>(SaveData.WorldData,string,string)
  at SaveData.WorldData.Save (System.String fileName) [0x0000b] in <6dc6faa1c0ae40368e38f6b0de78ff9c>:0 
  at Manager.Game.Save (System.String fileName) [0x00005] in <6dc6faa1c0ae40368e38f6b0de78ff9c>:0 
  at SaveData.SaveLoad+<>c__DisplayClass92_0.<OpenSave>b__1 (SaveData.FileQuestion+Result result) [0x00053] in <6dc6faa1c0ae40368e38f6b0de78ff9c>:0 
  at SaveData.SaveFileQuestion+<LoadAsync>d__11.MoveNext () [0x002cb] in <6dc6faa1c0ae40368e38f6b0de78ff9c>:0 
UnityEngine.DebugLogHandler:Internal_LogException(Exception, Object)
UnityEngine.DebugLogHandler:LogException(Exception, Object)
UnityEngine.Logger:LogException(Exception, Object)
UnityEngine.Debug:LogException(Exception)

When I wrote it I assumed that save data would never be larger than a few hundred MB at worst so it is not optimized at all.

I thought it was awesome when I heard that his saved data exceeded 2GB.

The proper fix would be to patch the save lambda with a transplier and write extended data there,

Is this the lambda of the save?
image

Maybe you haven't seen the KK version?
image

like it's done for character data.

Is this?
Patching GetBytes? Rewrite MemoryStream to FileStream?
https://github.com/IllusionMods/BepisPlugins/blob/abee07b2392e9d7db88a7f80e68230cf9a7268a8/src/Core_ExtensibleSaveFormat/Core.ExtendedSave.Hooks.cs#L300

@takahiro0327
Copy link
Contributor Author

Delete the current ExtendedSave.SaveDataSaveHook patch and add a transpiler to add extended save data behind WorldData.cs:272 in KKS?

272: binaryWriter.Write(WorldData.GetBytes(this));

public void Save(string path, string fileName)
    {
      Utils.File.OpenWrite(path + fileName, false, (Action<FileStream>) (f =>
      {
        using (BinaryWriter binaryWriter = new BinaryWriter((Stream) f))
          binaryWriter.Write(WorldData.GetBytes(this));
          SaveDataSaveHookLikeFunction(binaryWriter);
      }));
    }

@ManlyMarco
Copy link
Collaborator

ManlyMarco commented Apr 8, 2024

Delete the current ExtendedSave.SaveDataSaveHook patch and add a transpiler to add extended save data behind WorldData.cs:272 in KKS?

Yes, that would be the best way to do it I think. The issue I had was that different versions of the game (with or without EX) have different names for this lambda (<Save>b__140_0 and <Save>b__144_0), so there has to be some additional code to find and patch the right one.

Edit: Something like this could be used to see what method is called by Save

var movenext = GetMoveNext(AccessTools.Method(typeof(HSceneProc), nameof(HSceneProc.Start)));
hi.Patch(movenext, transpiler: new HarmonyMethod(typeof(Hooks), nameof(Hooks.ShowerFixTpl)));
}
private static MethodInfo GetMoveNext(MethodInfo targetMethod)
{
var ctx = new ILContext(new DynamicMethodDefinition(targetMethod).Definition);
var il = new ILCursor(ctx);
MethodReference enumeratorCtor = null;
il.GotoNext(instruction => instruction.MatchNewobj(out enumeratorCtor));
if (enumeratorCtor == null) throw new ArgumentNullException(nameof(enumeratorCtor));
if (enumeratorCtor.Name != ".ctor")
throw new ArgumentException($"Unexpected method name {enumeratorCtor.Name}, should be .ctor",
nameof(enumeratorCtor));
var enumeratorType = enumeratorCtor.DeclaringType.ResolveReflection();
var movenext = enumeratorType.GetMethod("MoveNext", AccessTools.all);
if (movenext == null) throw new ArgumentNullException(nameof(movenext));
return movenext;
}

@takahiro0327
Copy link
Contributor Author

No, that method may not extend the data size limit.

I tried it with 500MB of data at hand, and the data stored in the added area was quite small, probably mostly heroine's data.
If I leave GetBytes() as it is, the heroine's data alone reaches 2GB, subject to the MemoryStream-derived 2GB limit.
I think we need to expand the GetBytes() Write code directly into Save() without using the transpiler (GetBytes), what do you think?

image

@ManlyMarco
Copy link
Collaborator

Oh, you're right, the data is added to the characters and not to the save file itself.
In this case there's no way to use GetBytes by patching it I think, it has to be rewritten inside Save as you've suggested.

I believe no other plugins other than ExtendedSave patch or use the GetBytes method, so it should be safe to not use it at all and make it throw an exception (to make sure that no one tries using it since the data it returns is no longer valid).

@takahiro0327
Copy link
Contributor Author

How's that?
IllusionMods/BepisPlugins#197

ManlyMarco pushed a commit to IllusionMods/BepisPlugins that referenced this pull request Apr 9, 2024
#197)

Fixes failing to save the game when used character cards take up more than 2GB in total when serialized.
Discussion at IllusionMods/IllusionFixes#63
@takahiro0327 takahiro0327 deleted the large-save branch May 23, 2024 10:49
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.

None yet

2 participants