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

[RDY] Modernizing C++ #1511

Merged
merged 19 commits into from May 28, 2019

Conversation

Projects
None yet
2 participants
@TheCycoONE
Copy link
Member

commented Apr 5, 2019

A number of refactors aimed at modernizing and applying current best practices to the C++ code base.

  • Bump to C++ 14 which is now the default version on a number of compilers.
  • Use constexpr on compile time constants and a conservative set of functions that should be evaluated at compile time.
  • Replace some magic numbers with named constants
  • Greatly reduce the number of macros used
  • Use anonymous namespaces instead of static for controlling linkage in C++ code.

@TheCycoONE TheCycoONE changed the title [RFC] Modernizing C++ [WIP] Modernizing C++ Apr 5, 2019

@TheCycoONE

This comment has been minimized.

Copy link
Member Author

commented Apr 5, 2019

Looks like I'll need to fiddle with this under Visual Studio.

Show resolved Hide resolved CorsixTH/Src/th_sound.cpp Outdated
@TheCycoONE

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2019

The Visual Studio issue was simply a missing header (include what you use.)

I've also added a fix for the fourcc issue, and fixes for a number of other errors and warnings.

@TheCycoONE TheCycoONE changed the title [WIP] Modernizing C++ [RFC] Modernizing C++ Apr 9, 2019

@TheCycoONE

This comment has been minimized.

Copy link
Member Author

commented Apr 9, 2019

git did something weird, and dated my last commit for a week ago.

@TheCycoONE TheCycoONE changed the title [RFC] Modernizing C++ [WIP] Modernizing C++ Apr 17, 2019

@TheCycoONE

This comment has been minimized.

Copy link
Member Author

commented Apr 17, 2019

Working on cleaning up the lua 'class' binding macros which cleans up a lot of the warnings in g++ if -Wpedantic is set.

@TheCycoONE

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2019

There is a regression to the town map view in be953b8

@TheCycoONE TheCycoONE force-pushed the TheCycoONE:cpp14 branch from b334630 to c2f675a Apr 28, 2019

@TheCycoONE

This comment has been minimized.

Copy link
Member Author

commented Apr 28, 2019

Rebased on latest master so that travis can pass.

@TheCycoONE TheCycoONE force-pushed the TheCycoONE:cpp14 branch from c2f675a to b3ceed0 May 5, 2019

@TheCycoONE TheCycoONE force-pushed the TheCycoONE:cpp14 branch from b3ceed0 to 6c5ab65 May 16, 2019

@TheCycoONE TheCycoONE changed the title [WIP] Modernizing C++ [RFC] Modernizing C++ May 16, 2019

@TheCycoONE

This comment has been minimized.

Copy link
Member Author

commented May 16, 2019

That's all the changes I plan to make in this pull. I'd like it merged shortly after 0.63 lands. I have other work based on this branch.

@TheCycoONE TheCycoONE changed the title [RFC] Modernizing C++ [RDY] Modernizing C++ May 25, 2019

@TheCycoONE

This comment has been minimized.

Copy link
Member Author

commented May 25, 2019

Ok, ready for this to land now 😄

TheCycoONE added some commits Mar 28, 2019

Use constexpr for compile time colour handling
Places that use map_colour or pack_argb with literals are now evaluated
at compile time.
Replacing macros
The utf-8 algorithm is substantially rewritten. In most other
cases I stuck close to the original syntax.

The syntax of luaT_new changed from luaT_new(L, Type)(args...) to
luaT_new<Type>(L, args...)
Eliminate static scope from C++
The static keyword is heavily overloaded in C++, so the standard
committee has long recommended using annonymous namespaces instead for
indicating that code only applies to the current translation unit.

As an advantage, anonymous namespaces can also apply to types and
typedefs.

In the case of headers I removed static without replacing it - static
does not make sense in the context of header files.

TheCycoONE added some commits Apr 6, 2019

Fix fourcc
Had implementation defined behaviour because the cast to uint32_t wasn't
happening until after the bitshift; which meant that it was casting to 'int'
instead.

@TheCycoONE TheCycoONE force-pushed the TheCycoONE:cpp14 branch from 6c5ab65 to f4e6ee0 May 27, 2019

@Alberth289346 Alberth289346 merged commit 4ba54f2 into CorsixTH:master May 28, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
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.