Skip to content

C#: Expose byte array compress and decompress#106008

Merged
Repiteo merged 1 commit into
godotengine:masterfrom
beicause:cs-byte-array-compress
May 26, 2025
Merged

C#: Expose byte array compress and decompress#106008
Repiteo merged 1 commit into
godotengine:masterfrom
beicause:cs-byte-array-compress

Conversation

@beicause
Copy link
Copy Markdown
Contributor

@beicause beicause commented May 2, 2025

Expose the C# equivalent to PackedByteArray.compress, PackedByteArray.decompress and PackedByteArray.decompress_dynamic.

@beicause beicause requested a review from a team as a code owner May 2, 2025 06:23
@Chaosus Chaosus added this to the 4.5 milestone May 2, 2025
Copy link
Copy Markdown
Member

@paulloz paulloz left a comment

Choose a reason for hiding this comment

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

Hi! Thank you for your contribution.
This is indeed missing from the C# API, and I think some compression methods might not be available through System.IO.Compression. We'd rather have the new methods defined in GD, though, alongside the implementations of IsEmpty(), Join(), and Stringify().

@beicause beicause force-pushed the cs-byte-array-compress branch 2 times, most recently from e276a3c to 75bcf85 Compare May 17, 2025 06:34
Copy link
Copy Markdown
Member

@paulloz paulloz left a comment

Choose a reason for hiding this comment

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

Works fine, and looks good to me!

@raulsntos do we want to make compressionMode optional, and default to FileAccess.CompressionMode.Fastlz like the GDScript API?

@beicause
Copy link
Copy Markdown
Contributor Author

Just my opinion, I think it's less than ideal to make the argument optional. In addition, fastlz is a relatively old compression and doesn't have much advantages compared to modern compression such as zstd or lz4. https://github.com/inikep/lzbench/blob/master/doc/lzbench20_sorted.md.
But if .NET team thinks it's more important to be consistent with GDScript, I have no objection either.

@raulsntos
Copy link
Copy Markdown
Member

I think it makes sense to match unless we had a strong reason not to, but I don't have a strong opinion either way.

@beicause beicause force-pushed the cs-byte-array-compress branch from 75bcf85 to fc8328d Compare May 25, 2025 15:43
@Repiteo Repiteo merged commit 66d20a2 into godotengine:master May 26, 2025
20 checks passed
@Repiteo
Copy link
Copy Markdown
Contributor

Repiteo commented May 26, 2025

Thanks!

@beicause beicause deleted the cs-byte-array-compress branch May 27, 2025 13:22
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.

5 participants