Require scheduled reducers to have table fields `scheduled_at: ScheduledAt` and `scheduled_id: u64` declared

[kazimuth]

In my experience plus in Ingvar's comments in #1573, it seems slightly challenging to be implicitly adding fields to a table struct. It is also unusual from the user's point of view, since they don't usually expect extra fields to be added by a derive macro. And it seems to break rust-analyzer, which gives "no such field" errors when you access these fields.

We could change codegen to statically generate errors if the fields are not declared. Then the fields are obviously there in the struct and codegen pipelines can be slightly less convoluted.

[kazimuth]

...ah, but this does depend on having a way to statically assert these exist in both C# and Rust. @RReverser , do you think that's feasible? C# doesn't have static assert but we could require that scheduled tables implement an interface with those members.

In Rust we could just throw an error from the derive macro. It would require a syntactic comparison of type names but I think in this case that's pretty harmless.

[RReverser]

In Rust we could just throw an error from the derive macro

Yeah we can do basically the same thing in C#, actually even easier than in Rust because it gives access to fully resolved type names.

I'm not sure whether that's going to be more or less user-friendly though, especially with having to copy-paste same two fields (from docs?) in each such table.

How about having a generic wrapper like Scheduled<TableArgs> instead? Where TableArgs would be a simple SATS type, and the wrapper would always be a table with the scheduling metadata. Although I guess we need to find a way to register all monomorphisations in module then, which can be tricky.

P.S. FWIW I'd like to land #1573 regardless of decision on this - clearly splitting type and table gens makes maintaining them separately easier, as well as allows to remove redundant functionality from Unity-specific DLL, making it faster.

[RReverser]

especially with having to copy-paste same two fields (from docs?) in each such table.

Although I guess we could also combine them into a single struct, so users would only need to add one field.

[gefjon]

Combining them into a single struct is challenging because it prevents the scheduled_id from being auto-inc.

[RReverser]

Combining them into a single struct is challenging because it prevents the scheduled_id from being auto-inc.

Ugh, right...

[RReverser]

Just to list one more alternative I thought of, in C# we could just use inheritance, with those 2 fields living in some base abstract class, but that doesn't help much in Rust.

[kazimuth]

P.S. FWIW I'd like to land #1573 regardless of decision on this - clearly splitting type and table gens makes maintaining them separately easier, as well as allows to remove redundant functionality from Unity-specific DLL, making it faster.

Oh for sure, this issue shouldn't block that. It just was something else that reminded me of this.

[kazimuth]

One other possibility is having field annotations #[scheduled_at] and #[scheduled_id] which gets rid of the type resolution issue maybe. You pass those fields through in the RawModuleDef and then validate that the types are correct during the conversion to ModuleDef.

Come to think of it, I guess that works if we were looking up fields by name as well.

How about having a generic wrapper like Scheduled instead? Where TableArgs would be a simple SATS type, and the wrapper would always be a table with the scheduling metadata. Although I guess we need to find a way to register all monomorphisations in module then, which can be tricky.

Yeah, generics seem like a pain point in multiple places, they're also complicating my validation PR. Right now we have a good amount of special-case code to recognize options. We also can't really handle names for generic types correctly in codegen because sats has no notion of generics, except for array, map, and the slightly kludgey implementation of option

Combining them into a single struct is challenging because it prevents the scheduled_id from being auto-inc.

I'm currently rewriting ModuleDef and could generalize auto-inc to allow it to point to fields of a product type contained in a table, but that seems likely to be hellish

[RReverser]

We also can't really handle names for generic types correctly in codegen because sats has no notion of generics, except for array, map, and the slightly kludgey implementation of option

Yeah but that list is already long enough; like with nominal types, we should support generics properly in SATS at some point anyway.

Doesn't help here for reasons mentioned above though.

[RReverser]

And it seems to break rust-analyzer, which gives "no such field" errors when you access these fields.

Btw, is your Rust Analyzer configured to run proc-macro? I don't remember if it's on by default, but that setting usually fixes these kinds of issues. Although I admit I haven't tested it against those generated fields specifically.