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

Replace int_id<json_flag> with string_id<json_flag> (housekeeping) #45277

Merged
merged 1 commit into from
Nov 8, 2020

Conversation

Aivean
Copy link
Contributor

@Aivean Aivean commented Nov 6, 2020

Summary

SUMMARY: Infrastructure "Replace int_id<json_flag> with string_id<json_flag> (housekeeping)"

Purpose of change

In #44694 I switched item flags from std::string to int_id<json_flag>, specifically to speedup has_flag lookups.
In a couple of places that resulted in ugly code, when a temporary collection of string_ids was necessary, because it was populated before int_ids are available.

After #45099 string_id is faster* than int_id for lookups in the small collections (see "string_and_int_ids_benchmark").

* faster compared to string_id -> int_id conversion + int_id lookup

So currently there is no need to have the ugly code with temporary collections and all item flags can be represented as string_id<json_flag>.

Describe the solution

  1. Remove all temporary collections
  2. Alias flag_id to flag_str_id:
      using flag_id = string_id<json_flag>;
      using flag_str_id = flag_id;

The intension is to have a follow up PR with mass renaming of flag_str_id to flag_id.

Describe alternatives you've considered

None.

Testing

Game compiles and loads.
Smoke checked following things:

  • clothing modification
  • item flags saving and loading
  • effect flags

Also did a profiling in the item-intensive area to ensure that no performance regressions arised.

Additional context

Profiling results (after the change), filtered by string_id<json_flag>:
image

alias flag_id to flag_str_id (for now)
@Aivean Aivean added [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style labels Nov 6, 2020
@kevingranade kevingranade merged commit 8176f28 into CleverRaven:master Nov 8, 2020
@Aivean Aivean deleted the item-flags-string-id branch November 21, 2020 01:52
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