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 generated constants in RPG:: classes #287

Merged
merged 2 commits into from Dec 6, 2018

Conversation

Projects
None yet
4 participants
@fmatthew5876
Copy link
Contributor

fmatthew5876 commented Dec 2, 2018

Towards fixing EasyRPG/Player#1516

Also adds panning constants, so that the default pan values don't need to be duplicated between player and liblcf.

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:constant branch from e24a08c to 858ae47 Dec 2, 2018

@fmatthew5876 fmatthew5876 changed the title Constant Add support for generated constants in RPG:: classes Dec 2, 2018

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:constant branch from 858ae47 to b74cdd1 Dec 2, 2018

@carstene1ns

This comment has been minimized.

Copy link
Member

carstene1ns commented Dec 2, 2018

Thanks! This will also be very useful for #245.

@carstene1ns carstene1ns added this to the 0.6.0 (likely) milestone Dec 2, 2018

@carstene1ns carstene1ns requested a review from Ghabry Dec 6, 2018

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Dec 6, 2018

Lets make a quick style discussion:
I find Hungarian notation really ugly. Is that some common idiom in C++11 code to prefix constexpr-constants with a German "konstant"-k?

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 6, 2018

From my understanding, hungarian notation is where you add a prefix or suffix to say the type. And that to me is horrible. Its useless info and when you change the type you have to change the name too. I hate it.

Using "k" constant is not a C++11 thing. It just a style habit I picked up. It's not because it's german, it's because kSomeConstant stands out more readily than cSomeConstant. Similar to using "e" for eEnumValue.

I don't much care what convention we use. We just have to pick one.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Dec 6, 2018

Ah I thought it is k because c is the prefix for classes.

I dislike using e for enum but for the k we did a quick poll and the result is:

We don't kare

fmatthew5876 added some commits Dec 2, 2018

Add support for constants in rpg_ headers
* Adds State::kDeath
* Adds pan constants so that they don't need to be duplicated in Player

@fmatthew5876 fmatthew5876 force-pushed the fmatthew5876:constant branch from 42aa00e to 6cd8e2d Dec 6, 2018

@fmatthew5876

This comment has been minimized.

Copy link
Contributor Author

fmatthew5876 commented Dec 6, 2018

Fixed the whitespace. I also redefined the panning constants as X * 256

@carstene1ns carstene1ns merged commit 02c6403 into EasyRPG:master Dec 6, 2018

5 checks passed

GNU/Linux Build finished.
Details
OSX Build finished.
Details
Wii Build finished.
Details
Windows Build finished.
Details
web Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.