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

BG3: Add support for serializing and deserializing modsettings.lsx Load Order file #2149

Merged
merged 3 commits into from
Oct 14, 2024

Conversation

Al12rs
Copy link
Contributor

@Al12rs Al12rs commented Oct 9, 2024

  • Part of Epic - Game Support - Baldurs Gate 3 #1262

  • Add support for serializing a sorted array of ModuleShortName into modsettings.lsx format

  • Added serialization test

  • Add support for deserializing a modsettings.lsx string to sorted array of ModuleShortName structs.

  • Added deserialization test

@Al12rs Al12rs requested a review from a team October 9, 2024 13:03
@Al12rs Al12rs self-assigned this Oct 9, 2024
Copy link
Member

@Sewer56 Sewer56 left a comment

Choose a reason for hiding this comment

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

I think this is mostly fine.
I haven't played BG3, I cannot verify the accuracy of the resulting files; so I'll leave it to a second set of eyes.

It's a bit unusual seeing XML being parsed by hand; the structure present, at least within
the Verify file seems to be representable statically with classes; and a single call to Serialize / Deserialize like you'd do with JSON. I'll assume you either didn't know, think of it, or something else of the kind.

@Sewer56
Copy link
Member

Sewer56 commented Oct 10, 2024

If you want to be really fancy about it though, you could use a struct and IDisposable. Creating the struct does a WriteStartElement, and disposing it does an WriteEndElement.

That would spare you having to label every WriteEndElement and the code structure would closely resemble the XML.

Copy link
Member

@Sewer56 Sewer56 left a comment

Choose a reason for hiding this comment

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

I've marked this as 'you can merge it'; but please give it a while so another set of eyes can give their opinion too.

@halgari
Copy link
Collaborator

halgari commented Oct 11, 2024

C# has some XML formatters for objects as well, probably a few dozen of them, that way we don't need to do the band serialization. That'd require some investigation into what library to use though, so I'm fine with looping back around later to change it.

@Al12rs
Copy link
Contributor Author

Al12rs commented Oct 12, 2024

C# has some XML formatters for objects as well, probably a few dozen of them, that way we don't need to do the band serialization. That'd require some investigation into what library to use though, so I'm fine with looping back around later to change it.

Yeah I avoided that for now since they changed the format of modsetting.lsx a couple of times already, and I found it easier to match whatever they do this way, rather than trying to get a objects of the right shape and formatters for them to do what we need.
E.g. before modsettings.lsx used to have two lists, one with module short names, and another with the load order, containing just a UUID per line. That kind of stuff is easy to change with something like this.

@Al12rs Al12rs merged commit bd7aae4 into main Oct 14, 2024
11 checks passed
@Al12rs Al12rs deleted the feat/bg3-writing-modsettings-lsx branch October 14, 2024 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants