Closed amireldor closed 1 year ago
Related to https://github.com/colyseus/schema/issues/69
Hi @amireldor, thanks for the PR! Interesting how you've approached the generated C# to feel more like a "native" enum.
For simplicity's sake, though, I think would be better to use the direct type, which would avoid having to instantiate enums and avoid type casting from object
:
class GameMessageType {
public static int buildSlot = 0;
public static int scrapSlot = 1;
public static int buildShip = 2;
public static int deployMiner = 3;
}
What do you think?
Hi @endel , thanks for the response :)
I use string values for one of my enums in this example:
export enum ShipType {
Transporter = 'transport',
Miner = 'miner',
Colonizer = 'colonizer',
}
The problem I had with this simpler approach you suggested is when using the "enum" as a type itself in an interface, such as BuildShipMessageData
which has aShipType
as one of the parameters:
export interface BuildShipMessageData {
atPlanetId: string;
slotIndex: number;
shipType: ShipType;
}
As C# suprisingly don't have a union type, if using the simper approach then I can't mix strings and ints as the value for the parameter in the message.
I can try to do something more complicated, such as checking if one of the values in the enum I use for ShipType is a string, then use 'string' in the interface and if not, keep is as a number. This would lose some of the type-strictness though.
There might be a slight performance issue for fast games with the "fat enum" approach. Maybe we can start giving CLI flags per generator (i.e. --csharp) for the codegen command to either generate something leaner or more complicated and type-safe. Opinions?
Would it be possible to transform shipType: ShipType
into shipType: string
for C# in this case? 👀
Perhaps we'd need to relate these types in a similar way you've approached the "enrich typeMaps with enums"
Would it be possible to transform shipType: ShipType into shipType: string for C# in this case?
You don't mind losing type-safety here?
If we use shipType: string
, then we can assign ShipType.Transporter
but also MyOtherStringEnum.SomeValue
and the compiler would not complain, potentially breaking logic on the server-side.
Hello @endel, bumping issue up :)
Hi @amireldor, sorry for the delay to reply here!
You don't mind losing type-safety here? If we use shipType: string, then we can assign ShipType.Transporter but also MyOtherStringEnum.SomeValue and the compiler would not complain, potentially breaking logic on the server-side.
I think losing that wouldn't be too bad. The problem I see with the current implementation is:
ShipType.Transporter
in a loop will have the cost of instantiating a new ShipType
at every frame, and having to garbage collect itShipType.Transporter == ShipType.Transporter
will evaluate as false
. (Users could use .Value
here, but another instance of ShipType
will be allocated and needs to be garbage collected again...)Using the raw types directly would allow using ShipType.Transporter == ShipType.Transporter
and avoid unnecessary memory allocations/garbage collection. The type safety is not going to be perfect this way but it won't generate unnecessary memory allocations
Hello @endel, thanks for the message :)
I see your points. The equality problem probably have some C#-ish way that can help with, but the point of not creating new objects that will eventually get garbage collected is interetsing.
As I don't work on that Unity/Colyseus project right now, I will probably not make the changes to the PR soon enough. Anyone in the future who is interetsed in this, please fork my content and change accordingly, I can provide detail on where to look at the code generating the enums and content.
Additionally, @endel, what do you feel about adding a CLI flag to the generator where the developer might choose whether they prefer to use more "typed" enums or simpler values, so the choice between DX and performance is upon the gamedev and not on the library creators :)? I'm talking about flags specific to the chosen generator e.g. --csharp --class-enums
vs --csharp --primitive-enums
(the latter can be the default) etc.
Hi, this seems to work. I'd be happy to discuss and change the solution and code.
Property
's type as an initializer for an enum entry. It's a bit smelly.It works for me on my project. I think. For example, I have these generated:
(ignore the namespace I omitted from the previous snippets)
And I had to use
.ToString()
for the room messages: