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

Global getters #41347

Merged
merged 5 commits into from
Jun 16, 2020
Merged

Global getters #41347

merged 5 commits into from
Jun 16, 2020

Conversation

kevingranade
Copy link
Member

Summary

SUMMARY: None

Purpose of change

Continuing to address sources of build slowdown as revealed by #41237
Specifically, over-inclusion of game.h due to g->m, g->u, etc.

Describe the solution

Add getters for the various commonly accessed entities and use them.

Testing

Fairly straightforward refactor.

Additional Information

This depends on #41340 because they would conflict otherwise and I'm lazy.

@kevingranade kevingranade requested a review from KorGgenT as a code owner June 16, 2020 07:18
@jbytheway
Copy link
Contributor

I'm guessing you've only changed the uses of g->u etc. that were relevant to the files you wanted changed.

It wouldn't be hard to write a clang-tidy check to automate this refactoring across all uses. Let me know if that would be useful.

@mlangsdorf mlangsdorf added the Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style label Jun 16, 2020
@ZhilkinSerg ZhilkinSerg merged commit 438774d into CleverRaven:master Jun 16, 2020
@kevingranade
Copy link
Member Author

g->m can definitely be automated, do you think you can automate changing g->u to either get_avatar() or get_player_character()?

I think it's worth doing either way, I just wanted to do a little test run first. I also think it's simple enough to just do a mass find replace, but I'm sure the clang tidy route will be pretty simple too.

A side note, we won't need enforcement since once everything is migrated over m and u and the other affected members will be set to private and the various getters can be made friends of game.

@jbytheway
Copy link
Contributor

Yeah, thinking about it, if we don't need ongoing enforcement then it's probably easier to do mass find & replace. I could write something to choose between get_avatar and get_player_character correctly, but it would be tricky. Probably easier to find-and-replace with get_player_character and then fix the compiler errors. So, I've convinced myself not to bother.

@kevingranade
Copy link
Member Author

kevingranade commented Jun 16, 2020 via email

@jbytheway
Copy link
Contributor

Once you're done with these g->events() might be a good target for similar treatment.

Coolthulhu referenced this pull request in cataclysmbnteam/Cataclysm-BN Feb 7, 2021
* Make Character::stat into a standalone enum class

Enables forward declaration

* Remove player centric overload and unecessary include

* Remove player.h dependency from trap.cpp

* Add singleton-like accessors for avatar, map, weather and event_bus

* use accessors to remove includes of game.h
helariII added a commit to helariII/Cherry-Pick-Move-bionic_data-to-generic_factory that referenced this pull request May 23, 2021
Character::stat into character_stat according to CleverRaven/Cataclysm-DDA#41347.

Removed reference to bullet_protec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants