Same table names in different namespaces not supported

[jmillan]

As you can see in the following example, having tables with the same name in different namespaces is not supported. They are reduced to one (ie: Check for DumpResponse).

In our case we're using a quite wide schema and having namespaces make perfect sense.

.fbs schema:

union Body {
  FBS.Worker.DumpResponse,
  FBS.Worker.ResourceUsageResponse,
  FBS.WebRtcServer.DumpResponse,
  FBS.Router.DumpResponse,
  FBS.Transport.ProduceResponse,
  FBS.Transport.ConsumeResponse,
  FBS.Transport.RestartIceResponse,
  FBS.PlainTransport.ConnectResponse,
  FBS.PlainTransport.DumpResponse,
  FBS.PlainTransport.GetStatsResponse,
  FBS.PipeTransport.ConnectResponse,
  FBS.PipeTransport.DumpResponse,
  FBS.PipeTransport.GetStatsResponse,
  FBS.DirectTransport.DumpResponse,
  FBS.DirectTransport.GetStatsResponse,
  FBS.WebRtcTransport.ConnectResponse,
  FBS.WebRtcTransport.DumpResponse,
  FBS.WebRtcTransport.GetStatsResponse,
  FBS.Producer.DumpResponse,
  FBS.Producer.GetStatsResponse,
  FBS.Consumer.DumpResponse,
  FBS.Consumer.GetStatsResponse,
  FBS.Consumer.SetPreferredLayersResponse,
  FBS.Consumer.SetPriorityResponse,
  FBS.DataProducer.DumpResponse,
  FBS.DataProducer.GetStatsResponse,
  FBS.DataConsumer.GetBufferedAmountResponse,
  FBS.DataConsumer.DumpResponse,
  FBS.DataConsumer.GetStatsResponse,
}

The following rust code is generated:

            pub enum Body {
                DumpResponse(::planus::alloc::boxed::Box<super::data_consumer::DumpResponse>),
                ResourceUsageResponse(
                    ::planus::alloc::boxed::Box<super::worker::ResourceUsageResponse>,
                ),
                ProduceResponse(::planus::alloc::boxed::Box<super::transport::ProduceResponse>),
                ConsumeResponse(::planus::alloc::boxed::Box<super::transport::ConsumeResponse>),
                RestartIceResponse(
                    ::planus::alloc::boxed::Box<super::transport::RestartIceResponse>,
                ),
                ConnectResponse(
                    ::planus::alloc::boxed::Box<super::web_rtc_transport::ConnectResponse>,
                ),
                GetStatsResponse(
                    ::planus::alloc::boxed::Box<super::data_consumer::GetStatsResponse>,
                ),
                SetPreferredLayersResponse(
                    ::planus::alloc::boxed::Box<super::consumer::SetPreferredLayersResponse>,
                ),
                SetPriorityResponse(
                    ::planus::alloc::boxed::Box<super::consumer::SetPriorityResponse>,
                ),
                GetBufferedAmountResponse(
                    ::planus::alloc::boxed::Box<super::data_consumer::GetBufferedAmountResponse>,
                ),
            }

This is the generated code for c++:

enum class Body : uint8_t {
  NONE = 0,
  FBS_Worker_DumpResponse = 1,
  FBS_Worker_ResourceUsageResponse = 2,
  FBS_WebRtcServer_DumpResponse = 3,
  FBS_Router_DumpResponse = 4,
  FBS_Transport_ProduceResponse = 5,
  FBS_Transport_ConsumeResponse = 6,
  FBS_Transport_RestartIceResponse = 7,
  FBS_PlainTransport_ConnectResponse = 8,
  FBS_PlainTransport_DumpResponse = 9,
  FBS_PlainTransport_GetStatsResponse = 10,
  FBS_PipeTransport_ConnectResponse = 11,
  FBS_PipeTransport_DumpResponse = 12,
  FBS_PipeTransport_GetStatsResponse = 13,
  FBS_DirectTransport_DumpResponse = 14,
  FBS_DirectTransport_GetStatsResponse = 15,
  FBS_WebRtcTransport_ConnectResponse = 16,
  FBS_WebRtcTransport_DumpResponse = 17,
  FBS_WebRtcTransport_GetStatsResponse = 18,
  FBS_Producer_DumpResponse = 19,
  FBS_Producer_GetStatsResponse = 20,
  FBS_Consumer_DumpResponse = 21,
  FBS_Consumer_GetStatsResponse = 22,
  FBS_Consumer_SetPreferredLayersResponse = 23,
  FBS_Consumer_SetPriorityResponse = 24,
  FBS_DataProducer_DumpResponse = 25,
  FBS_DataProducer_GetStatsResponse = 26,
  FBS_DataConsumer_GetBufferedAmountResponse = 27,
  FBS_DataConsumer_DumpResponse = 28,
  FBS_DataConsumer_GetStatsResponse = 29,
  MIN = NONE,
  MAX = FBS_DataConsumer_GetStatsResponse
};

[TethysSvensson]

This is definitely a bug we want to fix, though I'm not quite sure how we want to fix it.

In the meantime you should be able to work around the problem by manually naming the variants. See this test for an example of how to use this syntax.

[TethysSvensson]

My current thoughts on how to fix it:

The current naming scheme doesn't use the namespace at all. As I see it, we have the following options:

• Option 1: Always use the namespaces in variant names.
• Option 1a: Always use the namespaces in variant names when there are variants from different namespaces.
• Option 2: Use namespaces in variant names when ambiguous.
• Option 3: Output an error on ambiguity.
• Option 3a: Output an error on ambiguity, but also add a custom attribute to enable using namespace in variant names.

Currently I think option 1 and 1a are a bad ideas, as it would make the common experience much worse.

I am currently split between options 2, 3 and 3a.

I think I am leaning towards 3, since I currently think that naming the variants is the currently feel that naming the variants is a good enough work-around. However I am definitely willing to be convinced otherwise.

In any case, I think it's a good idea to implement option 3 immediately, since we would need the code to detect ambiguity for 2 and 3a as well.

[jmillan]

I personally think that 3a is a good solution:

1. If there are namespaces but no ambiguity then the current implementation perfectly works and variant naming is kept minimal for better experience.
2. If there are namespaces and there is ambiguity, the generated error allows you to: 2.1. Modify the schema to fix the ambiguity. 2.2. Add a custom attribute to enable using namespace in variant names.

2.1 fits new, small, or not yet in-use schemas. 2.2 fits schemas that are more complex to modify (or which modification would make it less maintainable) and those cases where the schemas are already in use on other languages around a service.

[jmillan]

In the meantime you should be able to work around the problem by manually naming the variants

Thanks for this suggestion @TethysSvensson :+1:. It helps in the meantime.