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

Remove craft_command.h and recipe.h from character.h #71183

Merged
merged 2 commits into from
Jan 25, 2024

Conversation

ehughsbaird
Copy link
Contributor

Summary

None

Purpose of change

@Brambor asked a question in Discord, which made me look at this https://github.com/CleverRaven/Cataclysm-DDA/actions/runs/7617238475/job/20745814812, which made me look at this:

 98578 ms: src/recipe.h (included 221 times, avg 446 ms), included via:
  55x: avatar.h character.h craft_command.h 
  44x: character.h craft_command.h 
  18x: game.h character.h craft_command.h 
  17x: activity_actor_definitions.h character.h craft_command.h 
  8x: bionics.h effect_on_condition.h condition.h dialogue.h npc.h character.h craft_command.h 
  8x: vehicle.h clzones.h avatar.h character.h craft_command.h 
  ...

97879 ms: src/craft_command.h (included 220 times, avg 444 ms), included via:
  55x: avatar.h character.h 
  45x: character.h 
  18x: game.h character.h 
  17x: activity_actor_definitions.h character.h 
  8x: bionics.h effect_on_condition.h condition.h dialogue.h npc.h character.h 
  8x: vehicle.h clzones.h avatar.h character.h 

Describe the solution

Remove the headers, which do not need to be included.

Describe alternatives you've considered

Testing

From

% cat obj/*.inc | grep -v '.h:' | grep craft_command.h | wc -l
220
% cat obj/*.inc | grep -v '.h:' | grep recipe.h | wc -l
221

to

% cat obj/*.d | grep -v '.h:' | grep recipe.h | wc -l
142
% cat obj/*.d | grep -v '.h:' | grep craft_command.h | wc -l
130

Additional context

@Brambor If you've got IWYU running, it'd be great to do it on the whole codebase.

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jan 23, 2024
@Brambor
Copy link
Contributor

Brambor commented Jan 23, 2024

As you can see, it is not as trivial.

This is what IWYU says for character.h

/d/GitHub/Cataclysm-DDA/src/character.h should add these lines:
#include <memory>               // for __shared_ptr, __weak_ptr
#include "body_part_set.h"      // for body_part_set
#include "compatibility.h"      // for list_is_noexcept, map_is_noexcept
#include "inventory.h"          // for inventory
#include "pocket_type.h"        // for pocket_type, pocket_type::CONTAINER
#include "safe_reference.h"     // for safe_reference
#include "subbodypart.h"        // for sub_bodypart_id, side, side::BOTH
#include "units.h"              // for energy, mass, volume, temperature_delta
class activity_actor;
class item_pocket;
class profession;
namespace catacurses { class window; }
struct city;
struct field_immunity_data;
struct mutation_category_trait;
struct projectile;

/d/GitHub/Cataclysm-DDA/src/character.h should remove these lines:
- #include <iosfwd>  // lines 10-10
- #include <new>  // lines 14-14
- #include "city.h"  // lines 33-33
- #include "item_pocket.h"  // lines 44-44
- #include "memory_fast.h"  // lines 46-46
- #include "units_fwd.h"  // lines 56-56
- class Character;  // lines 61-61
- class inventory;  // lines 73-73
- class npc;  // lines 79-79
- struct needs_rates;  // lines 97-97
- struct points_left;  // lines 99-99

The full include-list for /d/GitHub/Cataclysm-DDA/src/character.h:
#include <algorithm>            // for max
#include <bitset>               // for bitset
#include <climits>              // for INT_MAX, INT_MIN
#include <cstdint>              // for int64_t, uint64_t
#include <functional>           // for function, reference_wrapper, cref, hash
#include <limits>               // for numeric_limits
#include <list>                 // for list, operator!=, _List_iterator, lis...
#include <map>                  // for map
#include <memory>               // for __shared_ptr, __weak_ptr
#include <optional>             // for optional, nullopt
#include <queue>                // for priority_queue
#include <set>                  // for set
#include <string>               // for string, operator==, allocator, __allo...
#include <type_traits>          // for integral_constant
#include <unordered_map>        // for unordered_map
#include <unordered_set>        // for unordered_set
#include <utility>              // for pair, forward
#include <vector>               // for vector
#include "activity_tracker.h"   // for activity_tracker
#include "activity_type.h"      // for activity_id
#include "addiction.h"          // for addiction
#include "body_part_set.h"      // for body_part_set
#include "bodypart.h"           // for bodypart_id, body_part_type, body_par...
#include "calendar.h"           // for time_point, time_duration, operator""...
#include "cata_utility.h"       // for return_true, return_false
#include "character_attire.h"   // for outfit
#include "character_id.h"       // for character_id
#include "compatibility.h"      // for list_is_noexcept, map_is_noexcept
#include "coordinates.h"        // for tripoint_abs_omt, tripoint_bub_ms
#include "craft_command.h"      // for comp_selection, craft_command (ptr only)
#include "creature.h"           // for creature_size, Creature, creature_siz...
#include "damage.h"             // for damage_unit (ptr only), damage_instance
#include "debug.h"              // for debug_filter
#include "enums.h"              // for social_modifiers, game_message_params
#include "flat_set.h"           // for flat_set
#include "game_constants.h"     // for INVENTORY_HANDLING_PENALTY, PICKUP_RANGE
#include "inventory.h"          // for inventory
#include "item.h"               // for item, faction_id, hint_rating, item::...
#include "item_location.h"      // for item_location, item_location::nowhere
#include "magic_enchantment.h"  // for enchant_cache (ptr only), mod
#include "pimpl.h"              // for pimpl
#include "player_activity.h"    // for player_activity
#include "pocket_type.h"        // for pocket_type, pocket_type::CONTAINER
#include "point.h"              // for tripoint, tripoint_min, tripoint_zero
#include "ranged.h"             // for Target_attributes
#include "recipe.h"             // for recipe (ptr only), recipe_filter_flags
#include "ret_val.h"            // for ret_val
#include "safe_reference.h"     // for safe_reference
#include "stomach.h"            // for stomach_contents, nutrients
#include "string_formatter.h"   // for string_format
#include "subbodypart.h"        // for sub_bodypart_id, side, side::BOTH
#include "type_id.h"            // for trait_id, itype_id, skill_id, flag_id
#include "units.h"              // for energy, mass, volume, temperature_delta
#include "visitable.h"          // for read_only_visitable (ptr only), Visit...
#include "weakpoint.h"          // for weakpoint (ptr only), weakpoint_attack
#include "weighted_list.h"      // for weighted_int_list
class JsonObject;  // lines 62-62
class JsonOut;  // lines 63-63
class SkillLevel;  // lines 64-64
class SkillLevelMap;  // lines 65-65
class activity_actor;
class basecamp;  // lines 66-66
class bionic_collection;  // lines 67-67
class character_martial_arts;  // lines 68-68
class dispersion_sources;  // lines 69-69
class effect;  // lines 70-70
class effect_source;  // lines 71-71
class faction;  // lines 72-72
class item_pocket;
class known_magic;  // lines 74-74
class ma_technique;  // lines 75-75
class map;  // lines 76-76
class monster;  // lines 77-77
class nc_color;  // lines 78-78
class player_morale;  // lines 80-80
class profession;
class proficiency_set;  // lines 81-81
class recipe_subset;  // lines 82-82
class spell;  // lines 83-83
class ui_adaptor;  // lines 84-84
class vehicle;  // lines 86-86
class vpart_reference;  // lines 85-85
namespace catacurses { class window; }
struct Character::bionic_fuels;  // lines 3932-3932
struct bionic;  // lines 87-87
struct city;
struct construction;  // lines 88-88
struct dealt_projectile_attack;  // lines 89-89
struct display_proficiency;  // lines 90-90
struct field_immunity_data;
struct islot_comestible;  // lines 92-92
struct item_comp;  // lines 93-93
struct itype;  // lines 94-94
struct mutation_branch;  // lines 95-95
struct mutation_category_trait;
struct mutation_variant;  // lines 96-96
struct pathfinding_settings;  // lines 98-98
struct projectile;
struct requirement_data;  // lines 100-100
struct tool_comp;  // lines 101-101
struct trait_and_var;  // lines 102-102
struct trap;  // lines 103-103
struct w_point;  // lines 104-104
template <typename E> struct enum_traits;  // lines 105-105
---

This is one of the most expensive headers to include, and there is
literally no reason for it to be here.
Another header that had no reason to be included in character.h, and was
one of the most expensive headers to compile as such.
@github-actions github-actions bot added Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Code: Tests Measurement, self-control, statistics, balancing. labels Jan 23, 2024
@ehughsbaird
Copy link
Contributor Author

I see. Yeah, not as easy as I thought - it wouldn't have caught this without some digging.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 23, 2024
@Maleclypse Maleclypse merged commit 44cf243 into CleverRaven:master Jan 25, 2024
26 checks passed
@ehughsbaird ehughsbaird deleted the easy-header-win branch January 25, 2024 21:09
@Brambor Brambor mentioned this pull request Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. Crafting / Construction / Recipes Includes: Uncrafting / Disassembling json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants