Skip to content

Fix C# + C++ SDKs for making Options filterable and expand sdk-test#5461

Open
JasonAtClockwork wants to merge 5 commits into
phoebe/sdk-test-filterable-optionfrom
jlarabie/sdk-test-filterable-option-expand
Open

Fix C# + C++ SDKs for making Options filterable and expand sdk-test#5461
JasonAtClockwork wants to merge 5 commits into
phoebe/sdk-test-filterable-optionfrom
jlarabie/sdk-test-filterable-option-expand

Conversation

@JasonAtClockwork

@JasonAtClockwork JasonAtClockwork commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Description of Changes

  • Is acompanion to Make Options filterable on clients, and add SDK tests. #5448 expanding the tests to TS/C#/C++
  • Updated sdk-test-ts, sdk-test-cs, and sdk-test-cpp to match Rust version; a couple changes were necessary to allow the codegen to work:
    • Updated C++ table constraint helpers to support std::optional<T> as filterable
    • Updated C# filterability checks to handle nullable values for IsEquatable()
  • Updated C# regression tests to cover nullable lookups, this uncovered more changes required:
    • Updated C# codegen to use the new nullable index base classes instead of removing constraints that Nullable<T> doesn't support
  • Updated C++/Unreal generated bindings to support the new Option<T> index tables and reducers used by the SDK tests

API and ABI breaking changes

  • Adds generated C# nullable index accessor support for indexes that were already intended to be filterable

Expected complexity level and risk

3 - Mostly test coverage and generated binding updates, with small changes to allow this properly in C++ and C#

Testing

  • Ran full sdk-test suite
  • Ran the C# regression tests
  • Manual testing of C# nullable index throwaway e2e test
  • Ran full Unreal SDK tests

@JasonAtClockwork JasonAtClockwork self-assigned this Jun 29, 2026
@JasonAtClockwork JasonAtClockwork marked this pull request as ready for review June 29, 2026 21:37

@gefjon gefjon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't feel entirely qualified to review the implementation, but the tests look reasonable, and that's enough to convince me. It looks like you've missed committing some generated files, as the git tree check part of CI is failing. Otherwise, if you want to wait for @lisandroct 's review, I support that, but if you want to merge, go ahead.

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.

2 participants