Skip to content

Comments

Add KVDocument as a root type#94

Merged
xPaw merged 3 commits intomasterfrom
kvfile
Feb 16, 2024
Merged

Add KVDocument as a root type#94
xPaw merged 3 commits intomasterfrom
kvfile

Conversation

@xPaw
Copy link
Member

@xPaw xPaw commented Feb 11, 2024

I made it extend KVObject for backwards compatibility.

KV3 requires a special root object because it has a header that contains format and encoding data. KV2 might have some extra stuff too.

Perhaps there's a better way of doing this?

    public class KVHeader
    {
        public Guid Encoding { get; set; }
        public Guid Format { get; set; }
    }

I think Serialize should have a new overload added to accept this, rather than changing the existing one because this would be a compat break. But how would this work with Serialize<TData>?

@yaakov-h
Copy link
Member

I would avoid using "File" to refer to something that can exist without ever touching disk. Other libraries have gone the route of "Document".

I don't see an obvious "better way".

I would expect that for Serialize() this encoding data should be provided through the KVSerializerOptions object? Ideally with well-known constants/values rather than BYO-GUID (though that should also be allowed).

@xPaw
Copy link
Member Author

xPaw commented Feb 11, 2024

KVDocument sounds fine.

Ideally with well-known constants/values rather than BYO-GUID (though that should also be allowed).

This is covered in the kv3 pr: https://github.com/ValveResourceFormat/ValveKeyValue/blob/3a0c305f14cccc19860a64c93ee76da245586099/ValveKeyValue/ValveKeyValue/KeyValues3/Encoding.cs

For serializing text files, you wouldn't really need to provide these (default to text encoding in generic format).

I'm not sure about adding them to KVSerializerOptions because deserialize gives you a KVDocument that contains these, would make sense for serialize to also take this, which works for generic kv objects, but not the typed variant.

EDIT: KV3 will add KVHeader class, so that could be passed into the serializer options, or as an optional argument.

@xPaw xPaw changed the title Add KVFile as a root type Add KVDocument as a root type Feb 11, 2024
@xPaw xPaw changed the title Add KVDocument as a root type Add KVDocument as a root type Feb 16, 2024
@xPaw xPaw merged commit bb29fd0 into master Feb 16, 2024
@xPaw xPaw deleted the kvfile branch February 16, 2024 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants