Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate "enums" from schema for Unity #139

Merged
merged 5 commits into from
Dec 27, 2022

Conversation

amireldor
Copy link
Contributor

Hi, this seems to work. I'd be happy to discuss and change the solution and code.

It works for me on my project. I think. For example, I have these generated:

public class ShipType {
    private ShipType(object value) { Value = value; }

    public object Value { get; private set; }

    public static ShipType Transporter { get { return new ShipType("transport"); } }
    public static ShipType Miner { get { return new ShipType("miner"); } }
    public static ShipType Colonizer { get { return new ShipType("colonizer"); } }
}
public class BuildShipMessageData {
    public string atPlanetId;
    public float slotIndex;
    public ShipType shipType;  // Note that the "Enum" is used here
}
public class GameMessageType {
    private GameMessageType(object value) { Value = value; }

    public object Value { get; private set; }

    public static GameMessageType buildSlot { get { return new GameMessageType(0); } }
    public static GameMessageType scrapSlot { get { return new GameMessageType(1); } }
    public static GameMessageType buildShip { get { return new GameMessageType(2); } }
    public static GameMessageType deployMiner { get { return new GameMessageType(3); } }
}

(ignore the namespace I omitted from the previous snippets)
image

And I had to use .ToString() for the room messages:
image

@amireldor
Copy link
Contributor Author

Related to #69

@endel
Copy link
Member

endel commented Aug 16, 2022

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?

@amireldor
Copy link
Contributor Author

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?

@endel
Copy link
Member

endel commented Aug 17, 2022

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"

@amireldor
Copy link
Contributor Author

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.

@amireldor
Copy link
Contributor Author

Hello @endel, bumping issue up :)

@endel
Copy link
Member

endel commented Oct 4, 2022

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:

  1. Accessing ShipType.Transporter in a loop will have the cost of instantiating a new ShipType at every frame, and having to garbage collect it
  2. It is not possible to do equality operations - ShipType.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

@amireldor
Copy link
Contributor Author

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.

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