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 include of "bionics.h" from "character.h". #22528

Merged
merged 2 commits into from
Dec 2, 2017

Conversation

BevapDin
Copy link
Contributor

@BevapDin BevapDin commented Dec 1, 2017

Changes Character::my_bionics to be a copyable_uniqe_ptr<bionic_collection>.

bionic_collection is a simple wrapper for std::vector<bionic>, defined in "bionics.h".
It can be forward declared, without defining the bionic class (std::vector<bionic> can't be forward declared without defining the bionic class).

"bionics.h" used to be included (directly/indirectly) by 103 cpp files, no it's only included by 11 cpp files.

… it.

They usually only work the bionic id (a `string_id`), and that does not need a definition from "bionics.h".
Changes `Character::my_bionics` to be a `copyable_uniqe_ptr<bionic_collection>`.

`bionic_collection` is a simple wrapper for `std::vector<bionic>`, defined in "bionics.h".
It can be forward declared, without defining the `bionic` class, `std::vector<bionic>` can't be forward declared without defining the `bionic` class.

Moves the definition of the destructor of `Character` into the cpp file because it implicitly requires the destructor of `bionics_collection` to be visible.
@kevingranade
Copy link
Member

/usr/include/c++/7.2.0/bits/unique_ptr.h:322: typename std::add_lvalue_reference<_Tp>::type std::unique_ptr<_Tp, _Dp>::operator*() const [with _Tp = bionic_collection; _Dp = std::default_delete<bionic_collection>; typename std::add_lvalue_reference<_Tp>::type = bionic_collection&]: Assertion 'get() != pointer()' failed.
Aborted (core dumped)

backtrace from gdb:
Program terminated with signal SIGABRT, Aborted.
#0 0x00007feb97cd68a0 in raise () from /usr/lib/libc.so.6
(gdb) bt
#0 0x00007feb97cd68a0 in raise () from /usr/lib/libc.so.6
#1 0x00007feb97cd7f09 in abort () from /usr/lib/libc.so.6
#2 0x000056488d46f560 in std::__replacement_assert (__condition=0x56488d9231f7 "get() != pointer()",
_function=0x56488da63160 <ZZNKSt10unique_ptrI17bionic_collectionSt14default_deleteIS0_EEdeEvE19__PRETTY_FUNCTION> "typename std::add_lvalue_reference<_Tp>::type std::unique_ptr<_Tp, _Dp>::operator*() const [with _Tp = bionic_collection; _Dp = std::default_delete<bionic_collection>; typename std::add_lvalue_referen"..., __line=322, __file=0x56488d923008 "/usr/include/c++/7.2.0/bits/unique_ptr.h") at /usr/include/c++/7.2.0/x86_64-pc-linux-gnu/bits/c++config.h:472
#3 std::unique_ptr<bionic_collection, std::default_delete<bionic_collection> >::operator* (this=0x56488f540688) at /usr/include/c++/7.2.0/bits/unique_ptr.h:322
#4 Character::has_active_bionic (this=this@entry=0x56488f53fcc0, b=...) at src/character.cpp:630
#5 0x000056488d476f01 in Character::is_blind (this=this@entry=0x56488f53fcc0) at src/character.cpp:2212
#6 0x000056488d476fa1 in Character::recalc_sight_limits (this=this@entry=0x56488f53fcc0) at src/character.cpp:503
#7 0x000056488d433b37 in player::player (this=0x56488f53fcc0) at src/player.cpp:627
#8 0x000056488ca4bc28 in game::game (this=0x56488f4e5ae0) at src/game.cpp:242
#9 0x000056488c95af0f in init_global_game_state (mods=std::__debug::vector of length 1, capacity 1 = {...}) at test_main.cpp:77
#10 0x000056488c95bfd0 in main (argc=, argv=) at test_main.cpp:150

@kevingranade
Copy link
Member

(gdb) f 4
#4 Character::has_active_bionic (this=this@entry=0x56488f53fcc0, b=...) at src/character.cpp:630
630 for (auto &i : *my_bionics) {
(gdb) p my_bionics
$1 = {<std::unique_ptr<bionic_collection, std::default_delete<bionic_collection> >> = std::unique_ptr<bionic_collection> containing 0x0, }

So... the unique ptr isn't initialized.

@@ -602,7 +605,7 @@ class Character : public Creature, public visitable<Character>
item weapon;
item ret_null; // Null item, sometimes returns by weapon() etc

std::vector<bionic> my_bionics;
copyable_unique_ptr<bionic_collection> my_bionics;
Copy link
Member

Choose a reason for hiding this comment

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

This here is the problem I think, the vector was default contructed, but the unique_ptr is constructed empty, it just needs a new default constructed bionic_collection.

Copy link
Member

Choose a reason for hiding this comment

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

all good, added my_bionics.reset( new bionic_collection() ); to the end of the Character constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sorry. I forgot that. Somehow I assumed the wrapper would create the pointer automatically. May need to add something like this.

@kevingranade kevingranade merged commit 50c345b into CleverRaven:master Dec 2, 2017
@BevapDin BevapDin deleted the gec branch December 2, 2017 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants