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

cata_variant enhancements #33697

Merged
merged 10 commits into from
Sep 1, 2019

Conversation

jbytheway
Copy link
Contributor

@jbytheway jbytheway commented Aug 31, 2019

Summary

SUMMARY: Infrastructure "More cata_variant features, including comparison and serialization"

Purpose of change

Make cata_variant a more "fully-featured" type applicable in more situations.

Motivated by the need to serialize and compare event objects in the forthcoming statistics tracker feature I'm working on for scores and achievements (#4173).

Describe the solution

Add the following to cata_variant:

  • Comparison operators.
  • Hash support.
  • (De)serialization support.
  • Standardised enum <-> string conversions for cata_variant_type.

Incidental changes:

  • string_to_enum now gives a more helpful error message in the event that the given string is not valid.
  • Renamed tuple_hash.h to hash_utils.h because it contains more than just tuple_hash (also hash_combine).

Additional context

These are commits extracted from the branch where I'm adding a class for tracking event statistics. That branch was getting large, so I'm pulling out these features.

The new features are not currently used anywhere, but hopefully they are obviously useful things to have.

The renaming of tuple_hash.h to hash_utils.h probably seems slightly random here, but that's a side-effect of me picking apart commits. Renames are hard to deal with in that context. I have more features to add to that header in a future PR that will make the renaming more logical, but I could remove that aspect from this branch if it seems too unwarranted.

@ZhilkinSerg ZhilkinSerg added Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style [C++] Changes (can be) made in C++. Previously named `Code` labels Aug 31, 2019
Also renamed tuple_hash to hash_utils, because it contains more than
just a tuple hash.
Previously it had bespoke stringification infrastructure.  Switch to the
shared code.
This is needed for deserialization code.
Actually say what the invalid string and the corresponding enum type
were to assist with debugging.
This is supposed to by in the standard library in C++14, but gcc 5.3 is
missing it.
@jbytheway jbytheway force-pushed the cata_variant_enhancements branch from 04d37e1 to e1db9b2 Compare August 31, 2019 18:24
@jbytheway jbytheway changed the title Cata variant enhancements cata_variant enhancements Sep 1, 2019
@ZhilkinSerg ZhilkinSerg self-assigned this Sep 1, 2019
@ZhilkinSerg ZhilkinSerg merged commit 66d59bb into CleverRaven:master Sep 1, 2019
@jbytheway jbytheway deleted the cata_variant_enhancements branch September 1, 2019 15:05
@ZhilkinSerg ZhilkinSerg removed their assignment Sep 1, 2019
misterprimus pushed a commit to misterprimus/Cataclysm-DDA that referenced this pull request Sep 21, 2019
* Comparison and hash support for cata_variant

Also renamed tuple_hash to hash_utils, because it contains more than
just a tuple hash.

* Fix cata_variant construction from const lvalue

* Convert cata_variant to io::enum_to_string

Previously it had bespoke stringification infrastructure.  Switch to the
shared code.

* Add default construction to empty cata_variant

This is needed for deserialization code.

* Serialization support for cata_variant

* Better error on invalid enum strings

Actually say what the invalid string and the corresponding enum type
were to assist with debugging.

* Support for deserializing enums from strings

* Additional cata_variant tests

* Explicit hash specialization for cata_variant_type

This is supposed to by in the standard library in C++14, but gcc 5.3 is
missing it.

* Fix clang-tidy errors
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants