tylercamp / palcalc

MIT License
32 stars 5 forks source link

Implement proper types for SaveReader which match Palworld repr #16

Open wojiaolw322 opened 4 months ago

wojiaolw322 commented 4 months ago

https://github.com/CrystalFerrai/UeSaveGame/ https://github.com/xkp95175333/palworld-sdk/blob/main/SDK/Pal_structs.hpp

tylercamp commented 4 months ago

On my phone and on break so I haven't looked deeply into the links yet, but my initial thoughts:

I considered writing full C# types which would map to the GVAS content, rather than dynamically collecting the structure at runtime, which would make it much easier to work with the parsed data

My only concerns:

  1. The current dynamic approach with visitor pattern helps to minimize memory usage by collecting values instead of parsing the whole structure. However, your suggestion doesn't necessarily require parsing the whole structure and can still just parse what we need, so overall memory usage would still be the same (if not better + reduced runtime due to visitor overhead)
  2. I wanted to keep my save reader impl as close to the reference as possible, and adding proper types would move away from that, but if there are other references I can use which already have proper types then that shouldn't be an issue. (Keeping impls close to reference makes it easier to apply updates from the reference)
  3. Proper typing sort of implies stricter parsing and sensitivity to minor changes in the data format. That only applies if we write purely-binary readers though. The properties in the GVAS are still labeled with strings, so strict binary parsing isn't necessarily required

This broader goal is definitely something to consider and would make save-related bugs easier to troubleshoot. My original concerns seem largely moot.

I'm just not sure about the effort required to bring all of those types in