Skip to content

Make JsonConvertFactory public sealed#10424

Open
QuackCola wants to merge 2 commits intoFacepunch:masterfrom
QuackCola:make-JsonConvertFactory-public
Open

Make JsonConvertFactory public sealed#10424
QuackCola wants to merge 2 commits intoFacepunch:masterfrom
QuackCola:make-JsonConvertFactory-public

Conversation

@QuackCola
Copy link
Copy Markdown
Contributor

@QuackCola QuackCola commented Apr 9, 2026

Summary

This pr simply makes the class JsonConvertFactory in Utility/Json/IJsonConvert.cs public sealed

Motivation & Context

This change is needed so JsonConvertFactory can be added to custom JsonSerializerOptions. So that you can serialize and deserialize types such as Variant when not using Sandbox.Json.Serialize

Implementation Details

Made the class public sealed so that it can be used externally but not extended.

Checklist

  • Code follows existing style and conventions
  • No unnecessary formatting or unrelated changes
  • Public APIs are documented (if applicable)
  • Unit tests added where applicable and all passing
  • I’m okay with this PR being rejected or requested to change 🙂

@QuackCola QuackCola force-pushed the make-JsonConvertFactory-public branch from 92bf4e9 to f5b2051 Compare April 9, 2026 12:09
@ElmarTalibzade
Copy link
Copy Markdown

I've noticed that there are a lot of classes in the codebase that could be sealed.

I was considering adding those, but I wasn't sure what's Facepunch's stance is on this and whether they want people to override all of the classes.

@DevulTj
Copy link
Copy Markdown
Member

DevulTj commented Apr 13, 2026

Sealing stuff we don't want people inheriting is the preferred solution.

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.

3 participants