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

Add support for typed and scoped enums #3

Merged
merged 3 commits into from
Jun 28, 2017
Merged

Add support for typed and scoped enums #3

merged 3 commits into from
Jun 28, 2017

Conversation

peterbell10
Copy link
Member

Say I have in the c++ side:

enum class EffectID : Int32
{
    SFX_MOB_GHAST_WARN
};

This will be usable in lua as EffectID.SFX_MOB_GHAST_WARN.

@madmaxoft
Copy link
Member

madmaxoft commented Jun 21, 2017

Make sure that this doesn't lose any of our current API. Compile Cuberite without these changes, run it and issue the api command (from the APIDump built-in plugin); make a backup of the Server/Cuberite_api.lua file generated by this, and then repeat with these changes. Compare the two API files to see if anything changed unexpectedly.

@Seadragon91
Copy link
Contributor

I tested this changes by comparing the generated Bindings.cpp. The only difference is the line with the time stamp. Is that enough as check?

@madmaxoft
Copy link
Member

Yes, that's a good enough check.

@madmaxoft
Copy link
Member

madmaxoft commented Jun 21, 2017

I'm not too sure about the types with names. What is that for? And does the ToLua runtime lib support it?

@peterbell10
Copy link
Member Author

@madmaxoft
Sorry but I'm not sure what you mean by "types with names".

@madmaxoft
Copy link
Member

Sorry, I meant types with spaces.

@peterbell10
Copy link
Member Author

Oh that's simple enough, it just means you can write

enum Foo : unsigned char {};

@madmaxoft
Copy link
Member

Oh, so it's only parser-side, not in the runtime.

-- This code is free software; you can redistribute it and/or modify it.
-- The software provided hereunder is on an "as is" basis, and
-- the author has no obligation to provide maintenance, support, updates,
-- enhancements, or modifications.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's safe to say that this copyright is not what you meant, unless you are secretly Waldemar :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well it was all just copied from enumerate.lua with some minor tweaks. I haven't looked at the license though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the way it is now OK? If not I might need a bit of help for what to put.

Copy link
Member

@madmaxoft madmaxoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good enough for me.

@peterbell10
Copy link
Member Author

Is there anything stopping this from getting merged?

@Seadragon91 Seadragon91 merged commit 988cab3 into cuberite:master Jun 28, 2017
@peterbell10 peterbell10 deleted the ScopedEnums branch June 28, 2017 14:35
@Seadragon91
Copy link
Contributor

Seadragon91 commented Jun 28, 2017

Ah should have done a squash merge...

@madmaxoft
Copy link
Member

Is there any reason why the ScopedEnum is created anew, instead of putting it inside the regular Enum?

I'm trying to fix in-class enum handling and now there's duplicate code.

@peterbell10
Copy link
Member Author

The main differences are the extra namespace qualifier to access the enum, and in classScopedEnum:register where there is an extra module output and the strong typing means the need for extra static_casts. I suppose it could be rewritten so ScopedEnum is just a container for Enumerate and move the static_casts into Enumerate:register.

Though I don't think it's related to where the enum is declared; could you explain the issue with in-class enums?

@madmaxoft
Copy link
Member

I guess I'll rewrite it like that.

The problem with in-class enums is when two classes each have an enum with the same name:

class cClassA
{
  enum eType { ... };
};

class cClassB
{
  enum eType { ... };
},

The parser doesn't use fully-qualified type names for some parts and ends up generating invalid code or mixing up the values.

@Seadragon91
Copy link
Contributor

@peterbell10 If you look into the generated Bindings.cpp for eMaterial. That's the enum for the boat materials and they can be accessed over the class name, e.g. cBoat.btOak. The problem is now that the enum range check is not generated correctly, it's missing the class name.

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.

None yet

3 participants