c# client generate

[lcodes]

Rust side generating module bindings for clockworklabs/com.clockworklabs.spacetimedbsdk#131

[RReverser]

@lcodes lcodes requested a review from RReverser 18 minutes ago

FWIW I'm going to review this one in more detail after the runtime counterpart.

[RReverser]

@lcodes AddPlayerArgsStruct and such are different from naming in Phoebe's and your own proposal (https://github.com/clockworklabs/SpacetimeDBPrivate/pull/949).

I always wanted to remove that ArgsStruct suffix too, but was punting off as it would be a breaking change, but now we have a perfect excuse for breaking changes - it's in the spec!

[RReverser]

In a scenario like this:

internal static partial class Outer
{
    [SpacetimeDB.Table]
    public partial struct InnerTable
    {
        public string Name;
    }
}

we will currently generate an ITableView implementation with public visibility, which will result in

Assert.Empty() Failure: Collection was not empty
Collection: [SpacetimeDB.Codegen\SpacetimeDB.Codegen.Module\FFI.cs(142,50): error CS0050: Inconsistent accessibility: return type 'IEnumerable<Outer.InnerTable>' is less accessible than method 'InnerTable.Iter()', SpacetimeDB.Codegen\SpacetimeDB.Codegen.Module\FFI.cs(143,37): error CS0050: Inconsistent accessibility: return type 'Outer.InnerTable' is less accessible than method 'InnerTable.Insert(Outer.InnerTable)', SpacetimeDB.Codegen\SpacetimeDB.Codegen.Module\FFI.cs(143,37): error CS0051: Inconsistent accessibility: parameter type 'Outer.InnerTable' is less accessible than method 'InnerTable.Insert(Outer.InnerTable)', SpacetimeDB.Codegen\SpacetimeDB.Codegen.Module\FFI.cs(144,17): error CS0051: Inconsistent accessibility: parameter type 'Outer.InnerTable' is less accessible than method 'InnerTable.Delete(Outer.InnerTable)']
Stack Trace:
at SpacetimeDB.Codegen.Tests.GeneratorSnapshotTests.TypeAndModuleGeneratorsOnServer() in D:\SpacetimeDB\crates\bindings-csharp\Codegen.Tests\Tests.cs:line 152
--- End of stack trace from previous location ---

We should instead generate it as an internal. I think in the visibility loop you need to take the minimum of any visibility you've seen while traversing up.

[RReverser]

Also mentioned this before, but right now Module.verified.txt results in couple of An internal error occurred during codegen: ... due to usage of exceptions for columns and visibility checks.

Could you please convert those to 2 new diagnostics with location context attached?

[jdetter]

I have not done a code review on this but I've tested it against circle game and it works.