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

Introduce enum bitset and use it for monster flags and triggers #29368

Merged
merged 14 commits into from
Apr 10, 2019

Conversation

codemime
Copy link
Contributor

@codemime codemime commented Apr 7, 2019

Summary

SUMMARY: Infrastructure "Introduce enum bitset and use it for monster flags and triggers"

Purpose of change

Monster flags/triggers were effectively duplicated in the mtype class: each stored as both std::set and std::bitset of enum entries. There's no point keeping around duplicated data and spend resources for maintaining its consistency, so only bitsets survived: a little benchmark showed that std::bitset is ten times faster than std::set for the purpose of accessing flags. However, plain bitsets aren't perfect for this either: they lack type-safety just like plain enums do.

Describe the solution

  • Introduced a new template class enum_bitset for the purpose of handling enum class-based flag entries in a type-safe manner (essentially it's a wrapper around std::bitset), and applied it initially to monster flags/behavioral triggers;
  • Got rid of sets and string accessors in mtype: working with string identifiers is error-prone due to typos, can be potentially slower, so it should be discouraged;
  • Removed null entries for flags and triggers: they we useless;
  • Slightly improved encapsulation of mtype;
  • Turned string-to-enum maps into static constants (they were member variables);
  • Converted monster triggers into an enum class. Didn't bother with converting flags, mostly out of laziness, but also to prevent oversizing the PR;
  • Slightly adjusted serialization to make it work with the new class.

Describe alternatives you've considered

Leave everything as it is.

Additional context

Shoudn't introduce any changes in observable behavior.

@Night-Pryanik
Copy link
Contributor

Refactoring isn't a valid possible summary line, I think Infrastructure fits best here.

@ZhilkinSerg
Copy link
Contributor

Jenkins rebuild

@codemime codemime force-pushed the enum-bitset-mon branch 2 times, most recently from 6092cc1 to 6088811 Compare April 7, 2019 21:08
Copy link
Contributor

@ifreund ifreund left a comment

Choose a reason for hiding this comment

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

All looks good to me, just found one typo.

src/generic_factory.h Outdated Show resolved Hide resolved
@ifreund ifreund added [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style Monsters Monsters both friendly and unfriendly. labels Apr 7, 2019
@ZhilkinSerg ZhilkinSerg merged commit 9ab8172 into CleverRaven:master Apr 10, 2019
@codemime codemime deleted the enum-bitset-mon branch April 17, 2019 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style Monsters Monsters both friendly and unfriendly.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants